From cdbe027c1c9c773aefedffd7a92df851fc62f79f Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Wed, 24 May 2017 13:42:52 -0400 Subject: [PATCH] Refactor `FileHelper` to use keyword arguments. --- app/controllers/static_controller.rb | 7 ++++++- app/controllers/uploads_controller.rb | 6 +++++- app/controllers/user_avatars_controller.rb | 8 +++++++- app/jobs/regular/pull_hotlinked_images.rb | 7 ++++++- app/models/optimized_image.rb | 7 ++++++- app/models/upload.rb | 7 ++++++- app/models/user_avatar.rb | 13 +++++++++++-- lib/file_helper.rb | 2 +- lib/file_store/base_store.rb | 7 ++++++- lib/tasks/uploads.rake | 14 ++++++++++++-- .../phpbb3/importers/avatar_importer.rb | 6 +++++- spec/components/file_helper_spec.rb | 12 ++++++++++-- 12 files changed, 81 insertions(+), 15 deletions(-) diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index de2988d8a39..1c88edee1ed 100644 --- a/app/controllers/static_controller.rb +++ b/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 diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 31dc0fab1bb..4ea747cedf9 100644 --- a/app/controllers/uploads_controller.rb +++ b/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 diff --git a/app/controllers/user_avatars_controller.rb b/app/controllers/user_avatars_controller.rb index 73c89ac04e6..30a3709c60a 100644 --- a/app/controllers/user_avatars_controller.rb +++ b/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 diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 1044bf60269..c14dae92fec 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/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 diff --git a/app/models/optimized_image.rb b/app/models/optimized_image.rb index 60f7d471769..20e886f987b 100644 --- a/app/models/optimized_image.rb +++ b/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) diff --git a/app/models/upload.rb b/app/models/upload.rb index 15f91fe3912..d6e1916c720 100644 --- a/app/models/upload.rb +++ b/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) diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index 952d66630a3..8005e7d2491 100644 --- a/app/models/user_avatar.rb +++ b/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 diff --git a/lib/file_helper.rb b/lib/file_helper.rb index fedfcc67ed9..4641a200def 100644 --- a/lib/file_helper.rb +++ b/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?:\/\// diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb index 6f7556494d8..b79282fe681 100644 --- a/lib/file_store/base_store.rb +++ b/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 diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index 24ac08179ca..c55722e2fee 100644 --- a/lib/tasks/uploads.rake +++ b/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) } diff --git a/script/import_scripts/phpbb3/importers/avatar_importer.rb b/script/import_scripts/phpbb3/importers/avatar_importer.rb index 4c237e76868..8a5e3a0c6f0 100644 --- a/script/import_scripts/phpbb3/importers/avatar_importer.rb +++ b/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 diff --git a/spec/components/file_helper_spec.rb b/spec/components/file_helper_spec.rb index 7b10d1a510f..a3ec7b913a3 100644 --- a/spec/components/file_helper_spec.rb +++ b/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