From 499a5947549581178957e359f50005c2a74188f1 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Tue, 12 Jan 2021 17:07:07 +0200
Subject: [PATCH] FIX: Do not downsize or crop GIF images (#10989)

It was a problem because during this operation only the first frame
is kept. This commit removes the alternative solution to check if a GIF
image is animated.
---
 Gemfile.lock                    |  2 +-
 lib/upload_creator.rb           | 30 +++++-------------------------
 spec/lib/upload_creator_spec.rb | 16 ++++++++++++++++
 3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 6c568cf76d0..b8054929988 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -134,7 +134,7 @@ GEM
     faraday-net_http (1.0.0)
     fast_blank (1.0.0)
     fast_xs (0.8.0)
-    fastimage (2.2.0)
+    fastimage (2.2.1)
     ffi (1.14.2)
     fspath (3.1.2)
     gc_tracer (1.5.1)
diff --git a/lib/upload_creator.rb b/lib/upload_creator.rb
index 5bd0eace14b..9c2f4abbda1 100644
--- a/lib/upload_creator.rb
+++ b/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 = animated?(@file)
+        @upload.animated = FastImage.animated?(@file)
       end
 
       add_metadata!
@@ -177,25 +177,6 @@ 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?
@@ -286,13 +267,13 @@ class UploadCreator
   end
 
   def should_alter_quality?
-    return false if animated?(@file)
+    return false if FastImage.animated?(@file)
 
     @upload.target_image_quality(@file.path, SiteSetting.recompress_original_jpg_quality).present?
   end
 
   def should_downsize?
-    max_image_size > 0 && filesize >= max_image_size
+    max_image_size > 0 && filesize >= max_image_size && !FastImage.animated?(@file)
   end
 
   def downsize!
@@ -302,14 +283,13 @@ class UploadCreator
 
       from = @file.path
       to = down_tempfile.path
-      scale = (from =~ /\.GIF$/i) ? "0.5" : "50%"
 
       OptimizedImage.ensure_safe_paths!(from, to)
 
       OptimizedImage.downsize(
         from,
         to,
-        scale,
+        "50%",
         scale_image: true,
         raise_on_error: true
       )
@@ -371,7 +351,7 @@ class UploadCreator
   end
 
   def should_crop?
-    return false if @opts[:type] == 'custom_emoji' && animated?(@file)
+    return false if ['profile_background', 'card_background', 'custom_emoji'].include?(@opts[:type]) && FastImage.animated?(@file)
 
     TYPES_TO_CROP.include?(@opts[:type])
   end
diff --git a/spec/lib/upload_creator_spec.rb b/spec/lib/upload_creator_spec.rb
index f1f0787737f..23442bcdfac 100644
--- a/spec/lib/upload_creator_spec.rb
+++ b/spec/lib/upload_creator_spec.rb
@@ -497,4 +497,20 @@ RSpec.describe UploadCreator do
       end
     end
   end
+
+  describe '#should_downsize?' do
+    context "GIF image" do
+      let(:gif_file) { file_from_fixtures("animated.gif") }
+
+      before do
+        SiteSetting.max_image_size_kb = 1
+      end
+
+      it "is not downsized" do
+        creator = UploadCreator.new(gif_file, "animated.gif")
+        creator.extract_image_info!
+        expect(creator.should_downsize?).to eq(false)
+      end
+    end
+  end
 end