FIX: Converting PNG to JPEG does not set the correct extension.

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

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

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