FIX: pull hotlinked images even when they have no extension

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"]

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

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)

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
 

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

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)

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: "<img src='http://wiki.mozilla.org/images/2/2e/Longcat1.png'>")
 
     Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
@@ -22,7 +22,7 @@ describe Jobs::PullHotlinkedImages do
     expect(post.raw).to match(/^<img src='\/uploads/)
   end
 
-  it 'replaces image src without protocol' do
+  it 'replaces images without protocol' do
     post = Fabricate(:post, raw: "<img src='//wiki.mozilla.org/images/2/2e/Longcat1.png'>")
 
     Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
@@ -31,6 +31,18 @@ describe Jobs::PullHotlinkedImages do
     expect(post.raw).to match(/^<img src='\/uploads/)
   end
 
+  it 'replaces images without extension' do
+    extensionless_url = "http://wiki.mozilla.org/images/2/2e/Longcat1"
+    stub_request(:get, extensionless_url).to_return(body: png, headers: { "Content-Type" => "image/png" })
+    stub_request(:head, extensionless_url)
+    post = Fabricate(:post, raw: "<img src='#{extensionless_url}'>")
+
+    Jobs::PullHotlinkedImages.new.execute(post_id: post.id)
+    post.reload
+
+    expect(post.raw).to match(/^<img src='\/uploads/)
+  end
+
   describe 'onebox' do
 
     let(:media) { "File:Brisbane_May_2013201.jpg" }