FIX: Blurry onebox favicon images (#15258)

This is a fix to address blurry onebox favicon images if the site you
are linking to happens to have a favicon.ico file that contains multiple
images.

This fix detects of we are trying to create an upload for a favicon.ico
file. We then convert it to a png and not a jpeg like we were doing. We
want a png because it will preserve transparency, otherwise if we
convert it to a jpeg we lose that and it looks bad on dark themed sites.

This fix also addresses the fact that .ico files can include multiple
images. The blurry images we were producing was caused by the
ImageMagick `-flatten` option when the .ico file had multiple images
which then squishes them all together. So for .ico files we are no
longer flattening them and instead we are grabbing the last image in the
.ico bundle and converting that single image to a png.

lib/upload_creator.rb

@@ -72,8 +72,10 @@ class UploadCreator
         extract_image_info!
         return @upload if @upload.errors.present?
 
-        if @image_info.type.to_s == "svg"
+        if @image_info.type == :svg
           clean_svg!
+        elsif @image_info.type == :ico
+          convert_favicon_to_png!
         elsif !Rails.env.test? || @opts[:force_optimize]
           convert_to_jpeg! if convert_png_to_jpeg? || should_alter_quality?
           fix_orientation! if should_fix_orientation?
@@ -261,6 +263,33 @@ class UploadCreator
   MIN_CONVERT_TO_JPEG_BYTES_SAVED = 75_000
   MIN_CONVERT_TO_JPEG_SAVING_RATIO = 0.70
 
+  def convert_favicon_to_png!
+    png_tempfile = Tempfile.new(["image", ".png"])
+
+    from = @file.path
+    to = png_tempfile.path
+
+    OptimizedImage.ensure_safe_paths!(from, to)
+
+    from = OptimizedImage.prepend_decoder!(from, nil, filename: "image.#{@image_info.type}")
+    to = OptimizedImage.prepend_decoder!(to)
+
+    from = "#{from}[-1]" # We only want the last(largest) image of the .ico file
+
+    opts = { flatten: false } # Preserve transparency
+
+    begin
+      execute_convert(from, to, opts)
+    rescue
+      # retry with debugging enabled
+      execute_convert(from, to, opts.merge(debug: true))
+    end
+
+    @file.respond_to?(:close!) ? @file.close! : @file.close
+    @file = png_tempfile
+    extract_image_info!
+  end
+
   def convert_to_jpeg!
     return if @opts[:for_site_setting]
     return if filesize < MIN_CONVERT_TO_JPEG_BYTES_SAVED
@@ -331,8 +360,8 @@ class UploadCreator
       "-auto-orient",
       "-background", "white",
       "-interlace", "none",
-      "-flatten"
     ]
+    command << "-flatten" unless opts[:flatten] == false
     command << "-debug" << "all" if opts[:debug]
     command << "-quality" << opts[:quality].to_s if opts[:quality]
     command << to

spec/lib/upload_creator_spec.rb

@@ -547,6 +547,23 @@ RSpec.describe UploadCreator do
     end
   end
 
+  describe '#convert_favicon_to_png!' do
+    let(:filename) { "smallest.ico" }
+    let(:file) { file_from_fixtures(filename, "images") }
+
+    before do
+      SiteSetting.authorized_extensions = 'png|jpg|ico'
+    end
+
+    it 'converts to png' do
+      upload = UploadCreator.new(file, filename).create_for(user.id)
+
+      expect(upload.persisted?).to eq(true)
+      expect(upload.extension).to eq('png')
+    end
+
+  end
+
   describe '#clean_svg!' do
     let(:b64) do
       Base64.encode64('<svg onmouseover="alert(alert)" />')