From fc3ae1343df5adb83a3733958a4436981feb380f Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 29 Sep 2021 23:52:36 +0200 Subject: Switch from unmaintained paperclip to kt-paperclip (#16724) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Switch from unmaintained paperclip to kt-paperclip * Drop some compatibility monkey-patches not required by kt-paperclip * Drop media spoof check monkey-patching It's broken with kt-paperclip and hopefully it won't be needed anymore * Fix regression introduced by paperclip 6.1.0 * Do not rely on pathname to call FastImage * Add test for ogg vorbis file with cover art * Add audio/vorbis to the accepted content-types This seems erroneous as this would be the content-type for a vorbis stream without an ogg container, but that's what the `marcel` gem outputs, so… * Restore missing for_as_default method * Refactor Attachmentable concern and delay Paperclip's content-type spoof check Check for content-type spoofing *after* setting the extension ourselves, this fixes a regression with kt-paperclip's validations being more strict than paperclip 6.0.0 and rejecting some Pleroma uploads because of unknown extensions. * Please CodeClimate * Add audio/vorbis to the unreliable set It doesn't correspond to a file format and thus has no extension associated. --- app/lib/fast_geometry_parser.rb | 2 +- app/models/account.rb | 2 +- app/models/concerns/attachmentable.rb | 61 ++++++++++++++++------------------- app/models/custom_emoji.rb | 6 ++-- app/models/media_attachment.rb | 6 ++-- app/models/preview_card.rb | 6 ++-- 6 files changed, 38 insertions(+), 45 deletions(-) (limited to 'app') diff --git a/app/lib/fast_geometry_parser.rb b/app/lib/fast_geometry_parser.rb index 5209c2bc5..f3395a833 100644 --- a/app/lib/fast_geometry_parser.rb +++ b/app/lib/fast_geometry_parser.rb @@ -2,7 +2,7 @@ class FastGeometryParser def self.from_file(file) - width, height = FastImage.size(file.path) + width, height = FastImage.size(file) raise Paperclip::Errors::NotIdentifiedByImageMagickError if width.nil? diff --git a/app/models/account.rb b/app/models/account.rb index 2f2a55b55..291d3e571 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -62,12 +62,12 @@ class Account < ApplicationRecord MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[[:word:]\.\-]+[[:word:]]+)?)/i URL_PREFIX_RE = /\Ahttp(s?):\/\/[^\/]+/ + include Attachmentable include AccountAssociations include AccountAvatar include AccountFinderConcern include AccountHeader include AccountInteractions - include Attachmentable include Paginable include AccountCounters include DomainNormalizable diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb index c5febb828..01fae4236 100644 --- a/app/models/concerns/attachmentable.rb +++ b/app/models/concerns/attachmentable.rb @@ -15,50 +15,47 @@ module Attachmentable # those files, it is necessary to use the output of the # `file` utility instead INCORRECT_CONTENT_TYPES = %w( + audio/vorbis video/ogg video/webm ).freeze included do - before_post_process :obfuscate_file_name - before_post_process :set_file_extensions - before_post_process :check_image_dimensions - before_post_process :set_file_content_type + def self.has_attached_file(name, options = {}) # rubocop:disable Naming/PredicateName + options = { validate_media_type: false }.merge(options) + super(name, options) + send(:"before_#{name}_post_process") do + attachment = send(name) + check_image_dimension(attachment) + set_file_content_type(attachment) + obfuscate_file_name(attachment) + set_file_extension(attachment) + Paperclip::Validators::MediaTypeSpoofDetectionValidator.new(attributes: [name]).validate(self) + end + end end private - def set_file_content_type - self.class.attachment_definitions.each_key do |attachment_name| - attachment = send(attachment_name) - - next if attachment.blank? || attachment.queued_for_write[:original].blank? || !INCORRECT_CONTENT_TYPES.include?(attachment.instance_read(:content_type)) + def set_file_content_type(attachment) # rubocop:disable Naming/AccessorMethodName + return if attachment.blank? || attachment.queued_for_write[:original].blank? || !INCORRECT_CONTENT_TYPES.include?(attachment.instance_read(:content_type)) - attachment.instance_write :content_type, calculated_content_type(attachment) - end + attachment.instance_write :content_type, calculated_content_type(attachment) end - def set_file_extensions - self.class.attachment_definitions.each_key do |attachment_name| - attachment = send(attachment_name) + def set_file_extension(attachment) # rubocop:disable Naming/AccessorMethodName + return if attachment.blank? - next if attachment.blank? - - attachment.instance_write :file_name, [Paperclip::Interpolations.basename(attachment, :original), appropriate_extension(attachment)].delete_if(&:blank?).join('.') - end + attachment.instance_write :file_name, [Paperclip::Interpolations.basename(attachment, :original), appropriate_extension(attachment)].delete_if(&:blank?).join('.') end - def check_image_dimensions - self.class.attachment_definitions.each_key do |attachment_name| - attachment = send(attachment_name) + def check_image_dimension(attachment) + return if attachment.blank? || !/image.*/.match?(attachment.content_type) || attachment.queued_for_write[:original].blank? - next if attachment.blank? || !/image.*/.match?(attachment.content_type) || attachment.queued_for_write[:original].blank? + width, height = FastImage.size(attachment.queued_for_write[:original].path) + matrix_limit = attachment.content_type == 'image/gif' ? GIF_MATRIX_LIMIT : MAX_MATRIX_LIMIT - width, height = FastImage.size(attachment.queued_for_write[:original].path) - matrix_limit = attachment.content_type == 'image/gif' ? GIF_MATRIX_LIMIT : MAX_MATRIX_LIMIT - - raise Mastodon::DimensionsValidationError, "#{width}x#{height} images are not supported" if width.present? && height.present? && (width * height > matrix_limit) - end + raise Mastodon::DimensionsValidationError, "#{width}x#{height} images are not supported" if width.present? && height.present? && (width * height > matrix_limit) end def appropriate_extension(attachment) @@ -79,13 +76,9 @@ module Attachmentable '' end - def obfuscate_file_name - self.class.attachment_definitions.each_key do |attachment_name| - attachment = send(attachment_name) + def obfuscate_file_name(attachment) + return if attachment.blank? || attachment.queued_for_write[:original].blank? || attachment.options[:preserve_files] - next if attachment.blank? || attachment.queued_for_write[:original].blank? || attachment.options[:preserve_files] - - attachment.instance_write :file_name, SecureRandom.hex(8) + File.extname(attachment.instance_read(:file_name)) - end + attachment.instance_write :file_name, SecureRandom.hex(8) + File.extname(attachment.instance_read(:file_name)) end end diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb index 7cb03b819..a85feb73a 100644 --- a/app/models/custom_emoji.rb +++ b/app/models/custom_emoji.rb @@ -21,6 +21,8 @@ # class CustomEmoji < ApplicationRecord + include Attachmentable + LIMIT = 50.kilobytes SHORTCODE_RE_FRAGMENT = '[a-zA-Z0-9_]{2,}' @@ -34,7 +36,7 @@ class CustomEmoji < ApplicationRecord belongs_to :category, class_name: 'CustomEmojiCategory', optional: true has_one :local_counterpart, -> { where(domain: nil) }, class_name: 'CustomEmoji', primary_key: :shortcode, foreign_key: :shortcode - has_attached_file :image, styles: { static: { format: 'png', convert_options: '-coalesce -strip' } } + has_attached_file :image, styles: { static: { format: 'png', convert_options: '-coalesce -strip' } }, validate_media_type: false before_validation :downcase_domain @@ -49,8 +51,6 @@ class CustomEmoji < ApplicationRecord remotable_attachment :image, LIMIT - include Attachmentable - after_commit :remove_entity_cache def local? diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 66d800b7b..cc48f65ed 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -31,6 +31,8 @@ class MediaAttachment < ApplicationRecord self.inheritance_column = nil + include Attachmentable + enum type: [:image, :gifv, :video, :unknown, :audio] enum processing: [:queued, :in_progress, :complete, :failed], _prefix: true @@ -50,7 +52,7 @@ class MediaAttachment < ApplicationRecord IMAGE_MIME_TYPES = %w(image/jpeg image/png image/gif).freeze VIDEO_MIME_TYPES = %w(video/webm video/mp4 video/quicktime video/ogg).freeze VIDEO_CONVERTIBLE_MIME_TYPES = %w(video/webm video/quicktime).freeze - AUDIO_MIME_TYPES = %w(audio/wave audio/wav audio/x-wav audio/x-pn-wave audio/ogg audio/mpeg audio/mp3 audio/webm audio/flac audio/aac audio/m4a audio/x-m4a audio/mp4 audio/3gpp video/x-ms-asf).freeze + AUDIO_MIME_TYPES = %w(audio/wave audio/wav audio/x-wav audio/x-pn-wave audio/ogg audio/vorbis audio/mpeg audio/mp3 audio/webm audio/flac audio/aac audio/m4a audio/x-m4a audio/mp4 audio/3gpp video/x-ms-asf).freeze BLURHASH_OPTIONS = { x_comp: 4, @@ -182,8 +184,6 @@ class MediaAttachment < ApplicationRecord validates_attachment_size :thumbnail, less_than: IMAGE_LIMIT remotable_attachment :thumbnail, IMAGE_LIMIT, suppress_errors: true, download_on_assign: false - include Attachmentable - validates :account, presence: true validates :description, length: { maximum: MAX_DESCRIPTION_LENGTH }, if: :local? validates :file, presence: true, if: :local? diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index a6ec839f8..bca3a3ce8 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -27,6 +27,8 @@ # class PreviewCard < ApplicationRecord + include Attachmentable + IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif'].freeze LIMIT = 1.megabytes @@ -41,9 +43,7 @@ class PreviewCard < ApplicationRecord has_and_belongs_to_many :statuses - has_attached_file :image, processors: [:thumbnail, :blurhash_transcoder], styles: ->(f) { image_styles(f) }, convert_options: { all: '-quality 80 -strip' } - - include Attachmentable + has_attached_file :image, processors: [:thumbnail, :blurhash_transcoder], styles: ->(f) { image_styles(f) }, convert_options: { all: '-quality 80 -strip' }, validate_media_type: false validates :url, presence: true, uniqueness: true validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES -- cgit