From a9e502936fb87bf480eb2f38f76de408b3556408 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Fri, 17 Aug 2018 11:41:30 +0800
Subject: [PATCH] FIX: Converting PNG to JPEG does not set the correct
 extension.

---
 lib/upload_creator.rb                    |  9 ++--
 spec/lib/upload_creator_spec.rb          | 69 ++++++++++++++++++++++++
 spec/requests/uploads_controller_spec.rb | 40 --------------
 3 files changed, 72 insertions(+), 46 deletions(-)
 create mode 100644 spec/lib/upload_creator_spec.rb

diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb
index f3c0bcd9f22..f25f7ae677e 100644
--- a/lib/upload_creator.rb
+++ b/lib/upload_creator.rb
@@ -38,13 +38,9 @@ class UploadCreator
         extract_image_info!
         return @upload if @upload.errors.present?
 
-        image_type = @image_info.type.to_s
-        basename = File.basename(@filename, File.extname(@filename))
-        @filename = "#{basename}.#{image_type}"
-
         if @filename[/\.svg$/i]
           whitelist_svg!
-        elsif !Rails.env.test?
+        elsif !Rails.env.test? || @opts[:force_optimize]
           convert_to_jpeg! if should_convert_to_jpeg?
           downsize!        if should_downsize?
 
@@ -54,6 +50,8 @@ class UploadCreator
           crop!            if should_crop?
           optimize!        if should_optimize?
         end
+
+        image_type = @image_info.type.to_s
       end
 
       # compute the sha of the file
@@ -156,7 +154,6 @@ class UploadCreator
     # keep the JPEG if it's at least 15% smaller
     if File.size(jpeg_tempfile.path) < filesize * 0.85
       @file = jpeg_tempfile
-      @filename = (File.basename(@filename, ".*").presence || I18n.t("image").presence || "image") + ".jpg"
       extract_image_info!
     else
       jpeg_tempfile&.close
diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb
new file mode 100644
index 00000000000..02861221a39
--- /dev/null
+++ b/spec/lib/upload_creator_spec.rb
@@ -0,0 +1,69 @@
+require 'rails_helper'
+
+RSpec.describe UploadCreator do
+  let(:user) { Fabricate(:user) }
+
+  describe '#create_for' do
+    describe 'when upload is not an image' do
+      before do
+        SiteSetting.authorized_extensions = 'txt'
+      end
+
+      let(:filename) { "utf-8.txt" }
+      let(:file) { file_from_fixtures(filename, "encodings") }
+
+      it 'should store the upload with the right extension' do
+        expect do
+          UploadCreator.new(file, filename).create_for(user.id)
+        end.to change { Upload.count }.by(1)
+
+        upload = Upload.last
+
+        expect(upload.extension).to eq('txt')
+        expect(File.extname(upload.url)).to eq('.txt')
+        expect(upload.original_filename).to eq('utf-8.txt')
+      end
+    end
+
+    describe 'when image has the wrong extension' do
+      let(:filename) { "png_as.jpg" }
+      let(:file) { file_from_fixtures(filename) }
+
+      it 'should store the upload with the right extension' do
+        expect do
+          UploadCreator.new(file, filename).create_for(user.id)
+        end.to change { Upload.count }.by(1)
+
+        upload = Upload.last
+
+        expect(upload.extension).to eq('png')
+        expect(File.extname(upload.url)).to eq('.png')
+        expect(upload.original_filename).to eq('png_as.jpg')
+      end
+    end
+
+    describe 'converting to jpeg' do
+      let(:filename) { "logo.png" }
+      let(:file) { file_from_fixtures(filename) }
+
+      before do
+        SiteSetting.png_to_jpg_quality = 1
+      end
+
+      it 'should store the upload with the right extension' do
+        expect do
+          UploadCreator.new(file, filename,
+            pasted: true,
+            force_optimize: true
+          ).create_for(user.id)
+        end.to change { Upload.count }.by(1)
+
+        upload = Upload.last
+
+        expect(upload.extension).to eq('jpeg')
+        expect(File.extname(upload.url)).to eq('.jpeg')
+        expect(upload.original_filename).to eq('logo.png')
+      end
+    end
+  end
+end
diff --git a/spec/requests/uploads_controller_spec.rb b/spec/requests/uploads_controller_spec.rb
index 62ba2562435..fac1a98d8d2 100644
--- a/spec/requests/uploads_controller_spec.rb
+++ b/spec/requests/uploads_controller_spec.rb
@@ -163,46 +163,6 @@ describe UploadsController do
         message = JSON.parse(response.body)["errors"]
         expect(message).to contain_exactly(I18n.t("upload.images.size_not_found"))
       end
-
-      describe 'when upload is not an image' do
-        before do
-          SiteSetting.authorized_extensions = 'txt'
-        end
-
-        let(:file) do
-          Rack::Test::UploadedFile.new(file_from_fixtures("utf-8.txt", "encodings"))
-        end
-
-        it 'should store the upload with the right extension' do
-          expect do
-            post "/uploads.json", params: { file: file, type: "composer" }
-          end.to change { Upload.count }.by(1)
-
-          upload = Upload.last
-
-          expect(upload.extension).to eq('txt')
-          expect(File.extname(upload.url)).to eq('.txt')
-          expect(upload.original_filename).to eq('utf-8.txt')
-        end
-      end
-
-      describe 'when image has the wrong extension' do
-        let(:file) do
-          Rack::Test::UploadedFile.new(file_from_fixtures("png_as.jpg"))
-        end
-
-        it 'should store the upload with the right extension' do
-          expect do
-            post "/uploads.json", params: { file: file, type: "avatar" }
-          end.to change { Upload.count }.by(1)
-
-          upload = Upload.last
-
-          expect(upload.extension).to eq('png')
-          expect(File.extname(upload.url)).to eq('.png')
-          expect(upload.original_filename).to eq('png_as.png')
-        end
-      end
     end
   end