FIX - don't attempt to optimized animated images (#11031)

* FIX - don't attempt to optimized animated images

* ensure_safe_paths before calling ImageMagick

lib/upload_creator.rb

@@ -126,7 +126,7 @@ class UploadCreator
       if is_image
         @upload.thumbnail_width, @upload.thumbnail_height = ImageSizer.resize(*@image_info.size)
         @upload.width, @upload.height = @image_info.size
-        @upload.animated = FastImage.animated?(@file)
+        @upload.animated = animated?(@file)
       end
 
       add_metadata!
@@ -177,6 +177,25 @@ class UploadCreator
     end
   end
 
+  def animated?(file)
+    OptimizedImage.ensure_safe_paths!(file.path)
+
+    # TODO - find out why:
+    #   FastImage.animated?(@file)
+    # doesn't seem to identify all animated gifs
+    @frame_count ||= begin
+      command = [
+        "identify",
+        "-format", "%n\\n",
+        file.path
+      ]
+
+      frames = Discourse::Utils.execute_command(*command).to_i rescue 1
+    end
+
+    @frame_count > 1
+  end
+
   MIN_PIXELS_TO_CONVERT_TO_JPEG ||= 1280 * 720
 
   def convert_png_to_jpeg?
@@ -267,6 +286,8 @@ class UploadCreator
   end
 
   def should_alter_quality?
+    return false if animated?(@file)
+
     @upload.target_image_quality(@file.path, SiteSetting.recompress_original_jpg_quality).present?
   end
 

spec/lib/upload_creator_spec.rb

@@ -134,6 +134,9 @@ RSpec.describe UploadCreator do
       let(:small_filename) { "logo.png" }
       let(:small_file) { file_from_fixtures(small_filename) }
 
+      let(:animated_filename) { "animated.gif" }
+      let(:animated_file) { file_from_fixtures(animated_filename) }
+
       before do
         SiteSetting.png_to_jpg_quality = 1
       end
@@ -174,19 +177,37 @@ RSpec.describe UploadCreator do
         expect(upload.original_filename).to eq('should_be_jpeg.jpg')
       end
 
-      it 'should alter the image quality' do
-        SiteSetting.png_to_jpg_quality = 75
-        SiteSetting.recompress_original_jpg_quality = 40
-        SiteSetting.image_preview_jpg_quality = 10
+      context "jpeg image quality settings" do
+        before do
+          SiteSetting.png_to_jpg_quality = 75
+          SiteSetting.recompress_original_jpg_quality = 40
+          SiteSetting.image_preview_jpg_quality = 10
+        end
 
-        upload = UploadCreator.new(file, filename, force_optimize: true).create_for(user.id)
+        it 'should alter the image quality' do
+          upload = UploadCreator.new(file, filename, force_optimize: true).create_for(user.id)
 
-        expect(image_quality(upload.url)).to eq(SiteSetting.recompress_original_jpg_quality)
+          expect(image_quality(upload.url)).to eq(SiteSetting.recompress_original_jpg_quality)
 
-        upload.create_thumbnail!(100, 100)
-        upload.reload
+          upload.create_thumbnail!(100, 100)
+          upload.reload
 
-        expect(image_quality(upload.optimized_images.first.url)).to eq(SiteSetting.image_preview_jpg_quality)
+          expect(image_quality(upload.optimized_images.first.url)).to eq(SiteSetting.image_preview_jpg_quality)
+        end
+
+        it 'should not convert animated images' do
+          expect do
+            UploadCreator.new(animated_file, animated_filename,
+              force_optimize: true
+            ).create_for(user.id)
+          end.to change { Upload.count }.by(1)
+
+          upload = Upload.last
+
+          expect(upload.extension).to eq('gif')
+          expect(File.extname(upload.url)).to eq('.gif')
+          expect(upload.original_filename).to eq('animated.gif')
+        end
       end
     end