[WiP] Make libvips optionalfeatures/optional-libvips

13 files changed, 285 insertions, 142 deletions

diff --git a/Gemfile b/Gemfile
index 283b56bf5f4..ca32d0cca18 100644
--- a/Gemfile
+++ b/Gemfile
@@ -23,7 +23,7 @@ gem 'fog-core', '<= 2.4.0'
 gem 'fog-openstack', '~> 1.0', require: false
 gem 'kt-paperclip', '~> 7.2'
 gem 'md-paperclip-azure', '~> 2.2', require: false
-gem 'ruby-vips', '~> 2.2'
+gem 'ruby-vips', '~> 2.2', require: false
 
 gem 'active_model_serializers', '~> 0.10'
 gem 'addressable', '~> 2.8'
diff --git a/app/models/concerns/account/avatar.rb b/app/models/concerns/account/avatar.rb
index ebf8b976926..39f599db182 100644
--- a/app/models/concerns/account/avatar.rb
+++ b/app/models/concerns/account/avatar.rb
@@ -9,7 +9,7 @@ module Account::Avatar
   class_methods do
     def avatar_styles(file)
       styles = { original: { geometry: '400x400#', file_geometry_parser: FastGeometryParser } }
-      styles[:static] = { geometry: '400x400#', format: 'png', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif'
+      styles[:static] = { geometry: '400x400#', format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif'
       styles
     end
 
@@ -18,7 +18,7 @@ module Account::Avatar
 
   included do
     # Avatar upload
-    has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, processors: [:lazy_thumbnail]
+    has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '+profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, processors: [:lazy_thumbnail]
     validates_attachment_content_type :avatar, content_type: IMAGE_MIME_TYPES
     validates_attachment_size :avatar, less_than: LIMIT
     remotable_attachment :avatar, LIMIT, suppress_errors: false
diff --git a/app/models/concerns/account/header.rb b/app/models/concerns/account/header.rb
index aae19abbf3e..44ae774e94d 100644
--- a/app/models/concerns/account/header.rb
+++ b/app/models/concerns/account/header.rb
@@ -10,7 +10,7 @@ module Account::Header
   class_methods do
     def header_styles(file)
       styles = { original: { pixels: MAX_PIXELS, file_geometry_parser: FastGeometryParser } }
-      styles[:static] = { format: 'png', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif'
+      styles[:static] = { format: 'png', convert_options: '-coalesce', file_geometry_parser: FastGeometryParser } if file.content_type == 'image/gif'
       styles
     end
 
@@ -19,7 +19,7 @@ module Account::Header
 
   included do
     # Header upload
-    has_attached_file :header, styles: ->(f) { header_styles(f) }, processors: [:lazy_thumbnail]
+    has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '+profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, processors: [:lazy_thumbnail]
     validates_attachment_content_type :header, content_type: IMAGE_MIME_TYPES
     validates_attachment_size :header, less_than: LIMIT
     remotable_attachment :header, LIMIT, suppress_errors: false
diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index d573ed40a6d..f53da04a975 100644
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -170,13 +170,18 @@ class MediaAttachment < ApplicationRecord
 
   DEFAULT_STYLES = [:original].freeze
 
+  GLOBAL_CONVERT_OPTIONS = {
+    all: '-quality 90 +profile "!icc,*" +set date:modify +set date:create +set date:timestamp -define jpeg:dct-method=float',
+  }.freeze
+
   belongs_to :account,          inverse_of: :media_attachments, optional: true
   belongs_to :status,           inverse_of: :media_attachments, optional: true
   belongs_to :scheduled_status, inverse_of: :media_attachments, optional: true
 
   has_attached_file :file,
                     styles: ->(f) { file_styles f },
-                    processors: ->(f) { file_processors f }
+                    processors: ->(f) { file_processors f },
+                    convert_options: GLOBAL_CONVERT_OPTIONS
 
   before_file_validate :set_type_and_extension
   before_file_validate :check_video_dimensions
@@ -187,7 +192,8 @@ class MediaAttachment < ApplicationRecord
 
   has_attached_file :thumbnail,
                     styles: THUMBNAIL_STYLES,
-                    processors: [:lazy_thumbnail, :blurhash_transcoder, :color_extractor]
+                    processors: [:lazy_thumbnail, :blurhash_transcoder, :color_extractor],
+                    convert_options: GLOBAL_CONVERT_OPTIONS
 
   validates_attachment_content_type :thumbnail, content_type: IMAGE_MIME_TYPES
   validates_attachment_size :thumbnail, less_than: IMAGE_LIMIT
diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb
index af412f3c75a..491e20f3c10 100644
--- a/app/models/preview_card.rb
+++ b/app/models/preview_card.rb
@@ -55,7 +55,9 @@ class PreviewCard < ApplicationRecord
 
   has_one :trend, class_name: 'PreviewCardTrend', inverse_of: :preview_card, dependent: :destroy
 
-  has_attached_file :image, processors: [:lazy_thumbnail, :blurhash_transcoder], styles: ->(f) { image_styles(f) }, validate_media_type: false
+  has_attached_file :image, processors: [ENV['MASTODON_USE_LIBVIPS'] == true ? :lazy_thumbnail : :thumbnail, :blurhash_transcoder], styles: lambda { |f|
+                                                                                                                                              image_styles(f)
+                                                                                                                                            }, convert_options: { all: '-quality 90 +profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, validate_media_type: false
 
   validates :url, presence: true, uniqueness: true, url: true
   validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES
diff --git a/app/models/site_upload.rb b/app/models/site_upload.rb
index d47b25c9462..6431d1007d1 100644
--- a/app/models/site_upload.rb
+++ b/app/models/site_upload.rb
@@ -64,7 +64,7 @@ class SiteUpload < ApplicationRecord
     mascot: {}.freeze,
   }.freeze
 
-  has_attached_file :file, styles: ->(file) { STYLES[file.instance.var.to_sym] }, processors: [:lazy_thumbnail, :blurhash_transcoder, :type_corrector]
+  has_attached_file :file, styles: ->(file) { STYLES[file.instance.var.to_sym] }, convert_options: { all: '-coalesce +profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, processors: [:lazy_thumbnail, :blurhash_transcoder, :type_corrector]
 
   validates_attachment_content_type :file, content_type: %r{\Aimage/.*\z}
   validates :file, presence: true
diff --git a/config/application.rb b/config/application.rb
index 07b50ca036b..b2916024501 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -27,7 +27,13 @@ require_relative '../lib/sanitize_ext/sanitize_config'
 require_relative '../lib/redis/namespace_extensions'
 require_relative '../lib/paperclip/url_generator_extensions'
 require_relative '../lib/paperclip/attachment_extensions'
-require_relative '../lib/paperclip/lazy_thumbnail'
+
+if ENV['MASTODON_USE_LIBVIPS'] == 'true'
+  require_relative '../lib/paperclip/vips_lazy_thumbnail'
+else
+  require_relative '../lib/paperclip/lazy_thumbnail'
+end
+
 require_relative '../lib/paperclip/gif_transcoder'
 require_relative '../lib/paperclip/media_type_spoof_detector_extensions'
 require_relative '../lib/paperclip/transcoder'
diff --git a/config/imagemagick/policy.xml b/config/imagemagick/policy.xml
new file mode 100644
index 00000000000..2730a9f84e3
--- /dev/null
+++ b/config/imagemagick/policy.xml
@@ -0,0 +1,27 @@
+<policymap>
+  <!-- Set some basic system resource limits -->
+  <policy domain="resource" name="time" value="60" />
+
+  <policy domain="module" rights="none" pattern="URL" />
+
+  <policy domain="filter" rights="none" pattern="*" />
+
+  <!--
+    Ideally, we would restrict ImageMagick to only accessing its own
+    disk-backed pixel cache as well as Mastodon-created Tempfiles.
+
+    However, those paths depend on the operating system and environment
+    variables, so they can only be known at runtime.
+
+    Furthermore, those paths are not necessarily shared across Mastodon
+    processes, so even creating a policy.xml at runtime is impractical.
+
+    For the time being, only disable indirect reads.
+  -->
+  <policy domain="path" rights="none" pattern="@*" />
+
+  <!-- Disallow any coder by default, and only enable ones required by Mastodon -->
+  <policy domain="coder" rights="none" pattern="*" />
+  <policy domain="coder" rights="read | write" pattern="{JPEG,PNG,GIF,WEBP,HEIC,AVIF}" />
+  <policy domain="coder" rights="write" pattern="{HISTOGRAM,RGB,INFO,ICO}" />
+</policymap>
diff --git a/config/initializers/vips.rb b/config/initializers/vips.rb
index 578c5b99efd..839a6254b65 100644
--- a/config/initializers/vips.rb
+++ b/config/initializers/vips.rb
@@ -1,3 +1,27 @@
 # frozen_string_literal: true
 
-Vips.block_untrusted(true) if Vips.at_least_libvips?(8, 13)
+if ENV['MASTODON_USE_LIBVIPS'] == 'true'
+  ENV['VIPS_BLOCK_UNTRUSTED'] = 'true'
+
+  require 'vips'
+
+  abort('Incompatible libvips version, please install libvips >= 8.13') unless Vips.at_least_libvips?(8, 13)
+
+  Vips.block('VipsForeign', true)
+
+  %w(
+    VipsForeignLoadNsgif
+    VipsForeignLoadJpeg
+    VipsForeignLoadPng
+    VipsForeignLoadWebp
+    VipsForeignLoadHeif
+    VipsForeignSavePng
+    VipsForeignSaveSpng
+    VipsForeignSaveJpeg
+    VipsForeignSaveWebp
+  ).each do |operation|
+    Vips.block(operation, false)
+  end
+
+  Vips.block_untrusted(true)
+end
diff --git a/lib/paperclip/blurhash_transcoder.rb b/lib/paperclip/blurhash_transcoder.rb
index 3f5a397fb6e..c3980183bd5 100644
--- a/lib/paperclip/blurhash_transcoder.rb
+++ b/lib/paperclip/blurhash_transcoder.rb
@@ -5,11 +5,22 @@ module Paperclip
     def make
       return @file unless options[:style] == :small || options[:blurhash]
 
-      image = Vips::Image.thumbnail(@file.path, 100)
-
-      attachment.instance.blurhash = Blurhash.encode(image.width, image.height, image.to_a.flatten, **(options[:blurhash] || {}))
+      attachment.instance.blurhash = Blurhash.encode(*blurhash_params, **(options[:blurhash] || {}))
 
       @file
     end
+
+    private
+
+    def blurhash_params
+      if ENV['MASTODON_USE_LIBVIPS'] == 'true'
+        image = Vips::Image.thumbnail(@file.path, 100)
+        [image.width, image.height, image.extract_band(0, n: 3).to_a.flatten]
+      else
+        pixels   = convert(':source -depth 8 RGB:-', source: File.expand_path(@file.path)).unpack('C*')
+        geometry = options.fetch(:file_geometry_parser).from_file(@file)
+        [geometry.width, geometry.height, pixels]
+      end
+    end
   end
 end
diff --git a/lib/paperclip/color_extractor.rb b/lib/paperclip/color_extractor.rb
index 95f42fcede7..f5129e4288b 100644
--- a/lib/paperclip/color_extractor.rb
+++ b/lib/paperclip/color_extractor.rb
@@ -10,20 +10,7 @@ module Paperclip
     BINS = 10
 
     def make
-      image = downscaled_image
-      block_edge_dim = (image.height * 0.25).floor
-      line_edge_dim = (image.width * 0.25).floor
-
-      edge_image = begin
-        top = image.crop(0, 0, image.width, block_edge_dim)
-        bottom = image.crop(0, image.height - block_edge_dim, image.width, block_edge_dim)
-        left = image.crop(0, block_edge_dim, line_edge_dim, image.height - (block_edge_dim * 2))
-        right = image.crop(image.width - line_edge_dim, block_edge_dim, line_edge_dim, image.height - (block_edge_dim * 2))
-        top.join(bottom, :vertical).join(left, :horizontal).join(right, :horizontal)
-      end
-
-      background_palette = palette_from_image(edge_image)
-      foreground_palette = palette_from_image(image)
+      background_palette, foreground_palette = ENV['MASTODON_USE_LIBVIPS'] == 'true' ? palettes_from_libvips : palettes_from_imagemagick
 
       background_color   = background_palette.first || foreground_palette.first
       foreground_colors  = []
@@ -86,6 +73,35 @@ module Paperclip
 
     private
 
+    def palettes_from_vips
+      image = downscaled_image
+      block_edge_dim = (image.height * 0.25).floor
+      line_edge_dim = (image.width * 0.25).floor
+
+      edge_image = begin
+        top = image.crop(0, 0, image.width, block_edge_dim)
+        bottom = image.crop(0, image.height - block_edge_dim, image.width, block_edge_dim)
+        left = image.crop(0, block_edge_dim, line_edge_dim, image.height - (block_edge_dim * 2))
+        right = image.crop(image.width - line_edge_dim, block_edge_dim, line_edge_dim, image.height - (block_edge_dim * 2))
+        top.join(bottom, :vertical).join(left, :horizontal).join(right, :horizontal)
+      end
+
+      background_palette = palette_from_image(edge_image)
+      foreground_palette = palette_from_image(image)
+      [background_palette, foreground_palette]
+    end
+
+    def palettes_from_imagemagick
+      depth = 8
+
+      # Determine background palette by getting colors close to the image's edge only
+      background_palette = palette_from_im_histogram(convert(':source -alpha set -gravity Center -region 75%x75% -fill None -colorize 100% -alpha transparent +region -format %c -colors :quantity -depth :depth histogram:info:', source: File.expand_path(@file.path), quantity: 10, depth: depth), 10)
+
+      # Determine foreground palette from the whole image
+      foreground_palette = palette_from_im_histogram(convert(':source -format %c -colors :quantity -depth :depth histogram:info:', source: File.expand_path(@file.path), quantity: 10, depth: depth), 10)
+      [background_palette, foreground_palette]
+    end
+
     def downscaled_image
       image = Vips::Image.new_from_file(@file.path, access: :random).thumbnail_image(100)
 
@@ -209,6 +225,18 @@ module Paperclip
       ColorDiff::Color::RGB.new(*hsl_to_rgb(hue, saturation, light))
     end
 
+    def palette_from_im_histogram(result, quantity)
+      frequencies       = result.scan(/([0-9]+):/).flatten.map(&:to_f)
+      hex_values        = result.scan(/\#([0-9A-Fa-f]{6,8})/).flatten
+      total_frequencies = frequencies.sum.to_f
+
+      frequencies.map.with_index { |f, i| [f / total_frequencies, hex_values[i]] }
+                 .sort_by { |r| -r[0] }
+                 .reject { |r| r[1].size == 8 && r[1].end_with?('00') }
+                 .map { |r| ColorDiff::Color::RGB.new(*r[1][0..5].scan(/../).map { |c| c.to_i(16) }) }
+                 .slice(0, quantity)
+    end
+
     def rgb_to_hex(rgb)
       format('#%02x%02x%02x', rgb.r, rgb.g, rgb.b)
     end
diff --git a/lib/paperclip/lazy_thumbnail.rb b/lib/paperclip/lazy_thumbnail.rb
index d799f8ac6d0..10b14860c4a 100644
--- a/lib/paperclip/lazy_thumbnail.rb
+++ b/lib/paperclip/lazy_thumbnail.rb
@@ -1,128 +1,24 @@
 # frozen_string_literal: true
 
 module Paperclip
-  class LazyThumbnail < Paperclip::Processor
-    GIF_FPS = 12
-
-    GIF_PALETTE_COLORS = 32
-
-    ALLOWED_FIELDS = %w(
-      icc-profile-data
-    ).freeze
-
-    class PixelGeometryParser
-      def self.parse(current_geometry, pixels)
-        width  = Math.sqrt(pixels * (current_geometry.width.to_f / current_geometry.height)).round.to_i
-        height = Math.sqrt(pixels * (current_geometry.height.to_f / current_geometry.width)).round.to_i
-
-        Paperclip::Geometry.new(width, height)
-      end
-    end
-
-    def initialize(file, options = {}, attachment = nil)
-      super
-
-      @crop = options[:geometry].to_s[-1, 1] == '#'
-      @current_geometry = options.fetch(:file_geometry_parser, Geometry).from_file(@file)
-      @target_geometry = options[:pixels] ? PixelGeometryParser.parse(@current_geometry, options[:pixels]) : options.fetch(:string_geometry_parser, Geometry).parse(options[:geometry].to_s)
-      @format = options[:format]
-      @current_format = File.extname(@file.path)
-      @basename = File.basename(@file.path, @current_format)
-
-      correct_current_format!
-    end
-
+  class LazyThumbnail < Paperclip::Thumbnail
     def make
       return File.open(@file.path) unless needs_convert?
 
-      dst = TempfileFactory.new.generate([@basename, @format ? ".#{@format}" : @current_format].join)
-
-      if preserve_animation?
-        if @target_geometry.nil? || (@current_geometry.width <= @target_geometry.width && @current_geometry.height <= @target_geometry.height)
-          target_width = 'iw'
-          target_height = 'ih'
-        else
-          scale = [@target_geometry.width.to_f / @current_geometry.width, @target_geometry.height.to_f / @target_geometry.height].min
-          target_width = (@current_geometry.width * scale).round
-          target_height = (@current_geometry.height * scale).round
-        end
-
-        # The only situation where we use crop on GIFs is cropping them to a square
-        # aspect ratio, so this is the only special case we implement. If cropping
-        # ever becomes necessary for other situations, this will need to be expanded.
-        if @target_geometry&.square?
-          crop_width = [target_width, target_height].min
-          crop_height = [target_width, target_height].min
-        end
-
-        filter = begin
-          if @crop
-            "scale=#{target_width}:#{target_height}:force_original_aspect_ratio=increase,crop=#{crop_width}:#{crop_height}"
-          else
-            "scale=#{target_width}:#{target_height}:force_original_aspect_ratio=decrease"
-          end
-        end
-
-        command = Terrapin::CommandLine.new(Rails.configuration.x.ffmpeg_binary, '-nostdin -i :source -map_metadata -1 -filter_complex :filter -y :destination', logger: Paperclip.logger)
-        command.run({ source: @file.path, filter: "fps=#{GIF_FPS},#{filter},split[a][b];[a]palettegen=max_colors=#{GIF_PALETTE_COLORS}[p];[b][p]paletteuse=dither=bayer", destination: dst.path })
-      else
-        transformed_image.write_to_file(dst.path, **save_options)
+      if options[:geometry]
+        min_side = [@current_geometry.width, @current_geometry.height].min.to_i
+        options[:geometry] = "#{min_side}x#{min_side}#" if @target_geometry.square? && min_side < @target_geometry.width
+      elsif options[:pixels]
+        width  = Math.sqrt(options[:pixels] * (@current_geometry.width.to_f / @current_geometry.height)).round.to_i
+        height = Math.sqrt(options[:pixels] * (@current_geometry.height.to_f / @current_geometry.width)).round.to_i
+        options[:geometry] = "#{width}x#{height}>"
       end
 
-      dst
-    rescue Terrapin::ExitStatusError => e
-      raise Paperclip::Error, "Error while optimizing #{@basename}: #{e}"
-    rescue Terrapin::CommandNotFoundError
-      raise Paperclip::Errors::CommandNotFoundError, 'Could not run the `ffmpeg` command. Please install ffmpeg.'
+      Paperclip::Thumbnail.make(file, options, attachment)
     end
 
     private
 
-    def correct_current_format!
-      # If the attachment was uploaded through a base64 payload, the tempfile
-      # will not have a file extension. It could also have the wrong file extension,
-      # depending on what the uploaded file was named. We correct for this in the final
-      # file name, which is however not yet physically in place on the temp file, so we
-      # need to use it here. Mind that this only reliably works if this processor is
-      # the first in line and we're working with the original, unmodified file.
-      @current_format = File.extname(attachment.instance_read(:file_name))
-    end
-
-    def transformed_image
-      # libvips has some optimizations for resizing an image on load. If we don't need to
-      # resize the image, we have to load it a different way.
-      if @target_geometry.nil?
-        Vips::Image.new_from_file(preserve_animation? ? "#{@file.path}[n=-1]" : @file.path, access: :sequential).copy.mutate do |mutable|
-          (mutable.get_fields - ALLOWED_FIELDS).each do |field|
-            mutable.remove!(field)
-          end
-        end
-      else
-        Vips::Image.thumbnail(@file.path, @target_geometry.width, height: @target_geometry.height, **thumbnail_options).mutate do |mutable|
-