Refactor FileHelper to use keyword arguments.

app/controllers/static_controller.rb

@@ -102,7 +102,12 @@ class StaticController < ApplicationController
 
     data = DistributedMemoizer.memoize('favicon' + SiteSetting.favicon_url, 60*30) do
       begin
-        file = FileHelper.download(SiteSetting.favicon_url, 50.kilobytes, "favicon.png", true)
+        file = FileHelper.download(
+          SiteSetting.favicon_url,
+          max_file_size: 50.kilobytes,
+          tmp_file_name: "favicon.png",
+          follow_redirect: true
+        )
         data = file.read
         file.unlink
         data

app/controllers/uploads_controller.rb

@@ -62,7 +62,11 @@ class UploadsController < ApplicationController
     if file.nil?
       if url.present? && is_api?
         maximum_upload_size = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes
-        tempfile = FileHelper.download(url, maximum_upload_size, "discourse-upload-#{type}") rescue nil
+        tempfile = FileHelper.download(
+          url,
+          max_file_size: maximum_upload_size,
+          tmp_file_name: "discourse-upload-#{type}"
+        ) rescue nil
         filename = File.basename(URI.parse(url).path)
       end
     else

app/controllers/user_avatars_controller.rb

@@ -134,7 +134,13 @@ class UserAvatarsController < ApplicationController
 
     unless File.exist? path
       FileUtils.mkdir_p PROXY_PATH
-      tmp = FileHelper.download(url, 1.megabyte, filename, true, 10)
+      tmp = FileHelper.download(
+        url,
+        max_file_size: 1.megabyte,
+        tmp_file_name: filename,
+        follow_redirect: true,
+        read_timeout: 10
+      )
       FileUtils.mv tmp.path, path
     end
 

app/jobs/regular/pull_hotlinked_images.rb

@@ -36,7 +36,12 @@ module Jobs
             # have we already downloaded that file?
             unless downloaded_urls.include?(src)
               begin
-                hotlinked = FileHelper.download(src, @max_size, "discourse-hotlinked", true)
+                hotlinked = FileHelper.download(
+                  src,
+                  max_file_size: @max_size,
+                  tmp_file_name: "discourse-hotlinked",
+                  follow_redirect: true
+                )
               rescue Discourse::InvalidParameters
               end
               if hotlinked

app/models/optimized_image.rb

@@ -246,7 +246,12 @@ class OptimizedImage < ActiveRecord::Base
           # download if external
           if external
             url = SiteSetting.scheme + ":" + previous_url
-            file = FileHelper.download(url, max_file_size_kb, "discourse", true) rescue nil
+            file = FileHelper.download(
+              url,
+              max_file_size: max_file_size_kb,
+              tmp_file_name: "discourse",
+              follow_redirect: true
+            ) rescue nil
             path = file.path
           else
             path = local_store.path_for(optimized_image)

app/models/upload.rb

@@ -93,7 +93,12 @@ class Upload < ActiveRecord::Base
           # download if external
           if external
             url = SiteSetting.scheme + ":" + previous_url
-            file = FileHelper.download(url, max_file_size_kb, "discourse", true) rescue nil
+            file = FileHelper.download(
+              url,
+              max_file_size: max_file_size_kb,
+              tmp_file_name: "discourse",
+              follow_redirect: true
+            ) rescue nil
             path = file.path
           else
             path = local_store.path_for(upload)

app/models/user_avatar.rb

@@ -20,7 +20,11 @@ class UserAvatar < ActiveRecord::Base
 
         max = Discourse.avatar_sizes.max
         gravatar_url = "http://www.gravatar.com/avatar/#{email_hash}.png?s=#{max}&d=404"
-        tempfile = FileHelper.download(gravatar_url, SiteSetting.max_image_size_kb.kilobytes, "gravatar")
+        tempfile = FileHelper.download(
+          gravatar_url,
+          max_file_size: SiteSetting.max_image_size_kb.kilobytes,
+          tmp_file_name: "gravatar"
+        )
         if tempfile
           upload = UploadCreator.new(tempfile, 'gravatar.png', origin: gravatar_url, type: "avatar").create_for(user_id)
 
@@ -63,7 +67,12 @@ class UserAvatar < ActiveRecord::Base
   end
 
   def self.import_url_for_user(avatar_url, user, options=nil)
-    tempfile = FileHelper.download(avatar_url, SiteSetting.max_image_size_kb.kilobytes, "sso-avatar", true)
+    tempfile = FileHelper.download(
+      avatar_url,
+      max_file_size: SiteSetting.max_image_size_kb.kilobytes,
+      tmp_file_name: "sso-avatar",
+      follow_redirect: true
+    )
 
     ext = FastImage.type(tempfile).to_s
     tempfile.rewind

lib/file_helper.rb

@@ -7,7 +7,7 @@ class FileHelper
     filename =~ images_regexp
   end
 
-  def self.download(url, max_file_size, tmp_file_name, follow_redirect=false, read_timeout=5)
+  def self.download(url, max_file_size:, tmp_file_name:, follow_redirect: false, read_timeout: 5)
     url = "https:" + url if url.start_with?("//")
     raise Discourse::InvalidParameters.new(:url) unless url =~ /^https?:\/\//
 

lib/file_store/base_store.rb

@@ -68,7 +68,12 @@ module FileStore
         if !file
           max_file_size_kb = [SiteSetting.max_image_size_kb, SiteSetting.max_attachment_size_kb].max.kilobytes
           url = SiteSetting.scheme + ":" + upload.url
-          file = FileHelper.download(url, max_file_size_kb, "discourse-download", true)
+          file = FileHelper.download(
+            url,
+            max_file_size: max_file_size_kb,
+            tmp_file_name: "discourse-download",
+            follow_redirect: true
+          )
           cache_file(file, filename)
         end
 

lib/tasks/uploads.rake

@@ -142,7 +142,12 @@ def migrate_from_s3
           puts "UPLOAD URL: #{url}"
           if filename = guess_filename(url, post.raw)
             puts "FILENAME: #{filename}"
-            file = FileHelper.download("http:#{url}", 20.megabytes, "from_s3", true)
+            file = FileHelper.download(
+              "http:#{url}",
+              max_file_size: 20.megabytes,
+              tmp_file_name: "from_s3",
+              follow_redirect: true
+            )
             if upload = UploadCreator.new(file, filename, File.size(file)).create_for(post.user_id || -1)
               post.raw = post.raw.gsub(/(https?:)?#{Regexp.escape(url)}/, upload.url)
               post.save
@@ -510,7 +515,12 @@ def regenerate_missing_optimized
         if (!File.exists?(original) || File.size(original) <= 0) && upload.origin.present?
           # try to fix it by redownloading it
           begin
-            downloaded = FileHelper.download(upload.origin, SiteSetting.max_image_size_kb.kilobytes, "discourse-missing", true) rescue nil
+            downloaded = FileHelper.download(
+              upload.origin,
+              max_file_size: SiteSetting.max_image_size_kb.kilobytes,
+              tmp_file_name: "discourse-missing",
+              follow_redirect: true
+            ) rescue nil
             if downloaded && downloaded.size > 0
               FileUtils.mkdir_p(File.dirname(original))
               File.open(original, "wb") { |f| f.write(downloaded.read) }

script/import_scripts/phpbb3/importers/avatar_importer.rb

@@ -65,7 +65,11 @@ module ImportScripts::PhpBB3
       max_image_size_kb = SiteSetting.max_image_size_kb.kilobytes
 
       begin
-        avatar_file = FileHelper.download(url, max_image_size_kb, 'discourse-avatar')
+        avatar_file = FileHelper.download(
+          url,
+          max_file_size: max_image_size_kb,
+          tmp_file_name: 'discourse-avatar'
+        )
       rescue StandardError => err
         warn "Error downloading avatar: #{err.message}. Skipping..."
         return nil

spec/components/file_helper_spec.rb

@@ -13,12 +13,20 @@ describe FileHelper do
 
   describe "download" do
     it "returns a file with the image" do
-      tmpfile = FileHelper.download(url, 10000, 'trouttmp')
+      tmpfile = FileHelper.download(
+        url,
+        max_file_size: 10000,
+        tmp_file_name: 'trouttmp'
+      )
       expect(tmpfile.read[0..5]).to eq("GIF89a")
     end
 
     it "works with a protocol relative url" do
-      tmpfile = FileHelper.download("//eviltrout.com/trout.png", 10000, 'trouttmp')
+      tmpfile = FileHelper.download(
+        "//eviltrout.com/trout.png",
+        max_file_size: 10000,
+        tmp_file_name: 'trouttmp'
+      )
       expect(tmpfile.read[0..5]).to eq("GIF89a")
     end
   end