From 5d63a7f4a68b0dbde481601d31fa33cf9c18556b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Tue, 13 Jun 2017 13:27:05 +0200 Subject: [PATCH] FIX: pull hotlinked images even when they have no extension --- Gemfile | 1 + Gemfile.lock | 4 +++- app/controllers/uploads_controller.rb | 3 ++- app/jobs/regular/pull_hotlinked_images.rb | 10 +++++++--- lib/file_helper.rb | 16 ++++++++++++---- lib/file_store/s3_store.rb | 3 ++- spec/jobs/pull_hotlinked_images_spec.rb | 20 ++++++++++++++++---- 7 files changed, 43 insertions(+), 14 deletions(-) diff --git a/Gemfile b/Gemfile index 5bf31ec3329..53d34033ff6 100644 --- a/Gemfile +++ b/Gemfile @@ -36,6 +36,7 @@ end gem 'mail' gem 'mime-types', require: 'mime/types/columnar' +gem 'mini_mime' gem 'hiredis' gem 'redis', require: ["redis", "redis/connection/hiredis"] diff --git a/Gemfile.lock b/Gemfile.lock index 1214cbfb3a7..721a2c2670c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -160,6 +160,7 @@ GEM metaclass (0.0.4) method_source (0.8.2) mime-types (2.99.3) + mini_mime (0.1.3) mini_portile2 (2.2.0) mini_racer (0.1.9) libv8 (~> 5.3) @@ -432,6 +433,7 @@ DEPENDENCIES memory_profiler message_bus mime-types + mini_mime mini_racer minitest mocha @@ -491,4 +493,4 @@ DEPENDENCIES webmock BUNDLED WITH - 1.14.6 + 1.15.1 diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 7313165d353..295671c8345 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -1,3 +1,4 @@ +require "mini_mime" require_dependency 'upload_creator' class UploadsController < ApplicationController @@ -42,7 +43,7 @@ class UploadsController < ApplicationController if upload = Upload.find_by(sha1: params[:sha]) || Upload.find_by(id: params[:id], url: request.env["PATH_INFO"]) opts = { filename: upload.original_filename, - content_type: Rack::Mime.mime_type(File.extname(upload.original_filename)), + content_type: MiniMime.lookup_by_filename(upload.original_filename)&.content_type, } opts[:disposition] = "inline" if params[:inline] opts[:disposition] ||= "attachment" unless FileHelper.is_image?(upload.original_filename) diff --git a/app/jobs/regular/pull_hotlinked_images.rb b/app/jobs/regular/pull_hotlinked_images.rb index 316dfda9fcb..10dffa6a0ee 100644 --- a/app/jobs/regular/pull_hotlinked_images.rb +++ b/app/jobs/regular/pull_hotlinked_images.rb @@ -47,8 +47,13 @@ module Jobs if hotlinked if File.size(hotlinked.path) <= @max_size filename = File.basename(URI.parse(src).path) + filename << File.extname(hotlinked.path) unless filename["."] upload = UploadCreator.new(hotlinked, filename, origin: src).create_for(post.user_id) - downloaded_urls[src] = upload.url + if upload.persisted? + downloaded_urls[src] = upload.url + else + Rails.logger.info("Failed to pull hotlinked image for post: #{post_id}: #{src} - #{upload.errors.join("\n")}") + end else Rails.logger.info("Failed to pull hotlinked image for post: #{post_id}: #{src} - Image is bigger than #{@max_size}") end @@ -81,8 +86,7 @@ module Jobs rescue => e Rails.logger.info("Failed to pull hotlinked image: #{src} post:#{post_id}\n" + e.message + "\n" + e.backtrace.join("\n")) ensure - # close & delete the temp file - hotlinked && hotlinked.close! + hotlinked&.close! rescue nil end end diff --git a/lib/file_helper.rb b/lib/file_helper.rb index d87db228bbd..da1962b1be0 100644 --- a/lib/file_helper.rb +++ b/lib/file_helper.rb @@ -1,5 +1,6 @@ -require "open-uri" require "final_destination" +require "mini_mime" +require "open-uri" class FileHelper @@ -24,19 +25,26 @@ class FileHelper ).resolve return unless uri.present? + downloaded = uri.open("rb", read_timeout: read_timeout) + extension = File.extname(uri.path) + + if extension.blank? && downloaded.content_type.present? + ext = MiniMime.lookup_by_content_type(downloaded.content_type)&.extension + extension = "." + ext if ext.present? + end + tmp = Tempfile.new([tmp_file_name, extension]) File.open(tmp.path, "wb") do |f| - downloaded = uri.open("rb", read_timeout: read_timeout) while f.size <= max_file_size && data = downloaded.read(512.kilobytes) f.write(data) end - # tiny files are StringIO, no close! on them - downloaded.try(:close!) rescue nil end tmp + ensure + downloaded&.close! rescue nil end private diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index b19963bb65d..265c7d53a83 100644 --- a/lib/file_store/s3_store.rb +++ b/lib/file_store/s3_store.rb @@ -1,4 +1,5 @@ require "uri" +require "mini_mime" require_dependency "file_store/base_store" require_dependency "s3_helper" require_dependency "file_helper" @@ -33,7 +34,7 @@ module FileStore # stored uploaded are public by default options = { acl: "public-read", - content_type: opts[:content_type].presence || Rack::Mime.mime_type(File.extname(filename)) + content_type: opts[:content_type].presence || MiniMime.lookup_by_filename(filename)&.content_type } # add a "content disposition" header for "attachments" options[:content_disposition] = "attachment; filename=\"#{filename}\"" unless FileHelper.is_image?(filename) diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index 220ed4e79cf..e1f5cb16d3e 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -4,16 +4,16 @@ require 'jobs/regular/pull_hotlinked_images' describe Jobs::PullHotlinkedImages do let(:image_url) { "http://wiki.mozilla.org/images/2/2e/Longcat1.png" } + let(:png) { Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") } before do - png = Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") - stub_request(:get, image_url).to_return(body: png) + stub_request(:get, image_url).to_return(body: png, headers: { "Content-Type" => "image/png" }) stub_request(:head, image_url) SiteSetting.download_remote_images_to_local = true FastImage.expects(:size).returns([100, 100]).at_least_once end - it 'replaces image src' do + it 'replaces images' do post = Fabricate(:post, raw: "") Jobs::PullHotlinkedImages.new.execute(post_id: post.id) @@ -22,7 +22,7 @@ describe Jobs::PullHotlinkedImages do expect(post.raw).to match(/^") Jobs::PullHotlinkedImages.new.execute(post_id: post.id) @@ -31,6 +31,18 @@ describe Jobs::PullHotlinkedImages do expect(post.raw).to match(/^ "image/png" }) + stub_request(:head, extensionless_url) + post = Fabricate(:post, raw: "") + + Jobs::PullHotlinkedImages.new.execute(post_id: post.id) + post.reload + + expect(post.raw).to match(/^