From b94633e8447e2d1b82d2a3ec9a29bf23f46904f4 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Mon, 30 Jul 2018 10:48:44 +0800
Subject: [PATCH] FIX: `FileHelper` should prioritize response content-type.

Request to a URL with `.png` extension may return a jpg
instead causing us to attach the wrong extension to an
upload.
---
 app/models/user_avatar.rb           | 10 +++++++++-
 lib/file_helper.rb                  |  6 ++----
 spec/components/file_helper_spec.rb | 19 ++++++++++++++++++-
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb
index 4774df14f32..731c15981a6 100644
--- a/app/models/user_avatar.rb
+++ b/app/models/user_avatar.rb
@@ -32,7 +32,15 @@ class UserAvatar < ActiveRecord::Base
         )
 
         if tempfile
-          upload = UploadCreator.new(tempfile, 'gravatar.png', origin: gravatar_url, type: "avatar").create_for(user_id)
+          ext = File.extname(tempfile)
+          ext = '.png' if ext.blank?
+
+          upload = UploadCreator.new(
+            tempfile,
+            "gravatar#{ext}",
+            origin: gravatar_url,
+            type: "avatar"
+          ).create_for(user_id)
 
           if gravatar_upload_id != upload.id
             gravatar_upload&.destroy!
diff --git a/lib/file_helper.rb b/lib/file_helper.rb
index 4fc4df68e48..9f859ff335a 100644
--- a/lib/file_helper.rb
+++ b/lib/file_helper.rb
@@ -54,15 +54,13 @@ class FileHelper
           end
         end
 
-        # first run
-        tmp_file_ext = File.extname(uri.path)
-
-        if tmp_file_ext.blank? && response.content_type.present?
+        if response.content_type.present?
           ext = MiniMime.lookup_by_content_type(response.content_type)&.extension
           ext = "jpg" if ext == "jpe"
           tmp_file_ext = "." + ext if ext.present?
         end
 
+        tmp_file_ext ||= File.extname(uri.path)
         tmp = Tempfile.new([tmp_file_name, tmp_file_ext])
         tmp.binmode
       end
diff --git a/spec/components/file_helper_spec.rb b/spec/components/file_helper_spec.rb
index 2ba6cb405ec..b481260ed77 100644
--- a/spec/components/file_helper_spec.rb
+++ b/spec/components/file_helper_spec.rb
@@ -12,7 +12,6 @@ describe FileHelper do
   end
 
   describe "download" do
-
     it "correctly raises an OpenURI HTTP error if it gets a 404 even with redirect" do
       url = "http://fourohfour.com/404"
       stub_request(:get, url).to_return(status: 404, body: "404")
@@ -69,6 +68,24 @@ describe FileHelper do
       )
       expect(tmpfile.read[0..5]).to eq("GIF89a")
     end
+
+    describe 'when url is a jpeg' do
+      let(:url) { "https://eviltrout.com/trout.jpg" }
+
+      it "should prioritize the content type returned by the response" do
+        stub_request(:get, url).to_return(body: png, headers: {
+          "content-type": "image/png"
+        })
+
+        tmpfile = FileHelper.download(
+          url,
+          max_file_size: 10000,
+          tmp_file_name: 'trouttmp'
+        )
+
+        expect(File.extname(tmpfile)).to eq('.png')
+      end
+    end
   end
 
 end