Switch from unmaintained paperclip to kt-paperclip (#16724)

* 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.
pull/16798/head
Claire 2021-09-29 23:52:36 +02:00 committed by GitHub
parent 0e4a4db141
commit fc3ae1343d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 99 additions and 199 deletions

View File

@ -20,7 +20,7 @@ gem 'dotenv-rails', '~> 2.7'
gem 'aws-sdk-s3', '~> 1.103', require: false gem 'aws-sdk-s3', '~> 1.103', require: false
gem 'fog-core', '<= 2.1.0' gem 'fog-core', '<= 2.1.0'
gem 'fog-openstack', '~> 0.3', require: false gem 'fog-openstack', '~> 0.3', require: false
gem 'paperclip', '~> 6.0' gem 'kt-paperclip', '~> 7.0'
gem 'blurhash', '~> 0.1' gem 'blurhash', '~> 0.1'
gem 'active_model_serializers', '~> 0.10' gem 'active_model_serializers', '~> 0.10'

View File

@ -316,6 +316,12 @@ GEM
activerecord activerecord
kaminari-core (= 1.2.1) kaminari-core (= 1.2.1)
kaminari-core (1.2.1) kaminari-core (1.2.1)
kt-paperclip (7.0.0)
activemodel (>= 4.2.0)
activesupport (>= 4.2.0)
marcel (~> 1.0.1)
mime-types
terrapin (~> 0.6.0)
launchy (2.5.0) launchy (2.5.0)
addressable (~> 2.7) addressable (~> 2.7)
letter_opener (1.7.0) letter_opener (1.7.0)
@ -351,9 +357,6 @@ GEM
mime-types (3.3.1) mime-types (3.3.1)
mime-types-data (~> 3.2015) mime-types-data (~> 3.2015)
mime-types-data (3.2020.0512) mime-types-data (3.2020.0512)
mimemagic (0.3.10)
nokogiri (~> 1)
rake
mini_mime (1.1.1) mini_mime (1.1.1)
mini_portile2 (2.6.1) mini_portile2 (2.6.1)
minitest (5.14.4) minitest (5.14.4)
@ -391,12 +394,6 @@ GEM
openssl-signature_algorithm (0.4.0) openssl-signature_algorithm (0.4.0)
orm_adapter (0.5.0) orm_adapter (0.5.0)
ox (2.14.5) ox (2.14.5)
paperclip (6.0.0)
activemodel (>= 4.2.0)
activesupport (>= 4.2.0)
mime-types
mimemagic (~> 0.3.0)
terrapin (~> 0.6.0)
parallel (1.21.0) parallel (1.21.0)
parallel_tests (3.7.3) parallel_tests (3.7.3)
parallel parallel
@ -720,6 +717,7 @@ DEPENDENCIES
json-ld json-ld
json-ld-preloaded (~> 3.1) json-ld-preloaded (~> 3.1)
kaminari (~> 1.2) kaminari (~> 1.2)
kt-paperclip (~> 7.0)
letter_opener (~> 1.7) letter_opener (~> 1.7)
letter_opener_web (~> 1.4) letter_opener_web (~> 1.4)
link_header (~> 0.0) link_header (~> 0.0)
@ -738,7 +736,6 @@ DEPENDENCIES
omniauth-rails_csrf_protection (~> 0.1) omniauth-rails_csrf_protection (~> 0.1)
omniauth-saml (~> 1.10) omniauth-saml (~> 1.10)
ox (~> 2.14) ox (~> 2.14)
paperclip (~> 6.0)
parallel (~> 1.21) parallel (~> 1.21)
parallel_tests (~> 3.7) parallel_tests (~> 3.7)
parslet parslet

View File

@ -2,7 +2,7 @@
class FastGeometryParser class FastGeometryParser
def self.from_file(file) def self.from_file(file)
width, height = FastImage.size(file.path) width, height = FastImage.size(file)
raise Paperclip::Errors::NotIdentifiedByImageMagickError if width.nil? raise Paperclip::Errors::NotIdentifiedByImageMagickError if width.nil?

View File

@ -62,12 +62,12 @@ class Account < ApplicationRecord
MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[[:word:]\.\-]+[[:word:]]+)?)/i MENTION_RE = /(?<=^|[^\/[:word:]])@((#{USERNAME_RE})(?:@[[:word:]\.\-]+[[:word:]]+)?)/i
URL_PREFIX_RE = /\Ahttp(s?):\/\/[^\/]+/ URL_PREFIX_RE = /\Ahttp(s?):\/\/[^\/]+/
include Attachmentable
include AccountAssociations include AccountAssociations
include AccountAvatar include AccountAvatar
include AccountFinderConcern include AccountFinderConcern
include AccountHeader include AccountHeader
include AccountInteractions include AccountInteractions
include Attachmentable
include Paginable include Paginable
include AccountCounters include AccountCounters
include DomainNormalizable include DomainNormalizable

View File

@ -15,51 +15,48 @@ module Attachmentable
# those files, it is necessary to use the output of the # those files, it is necessary to use the output of the
# `file` utility instead # `file` utility instead
INCORRECT_CONTENT_TYPES = %w( INCORRECT_CONTENT_TYPES = %w(
audio/vorbis
video/ogg video/ogg
video/webm video/webm
).freeze ).freeze
included do included do
before_post_process :obfuscate_file_name def self.has_attached_file(name, options = {}) # rubocop:disable Naming/PredicateName
before_post_process :set_file_extensions options = { validate_media_type: false }.merge(options)
before_post_process :check_image_dimensions super(name, options)
before_post_process :set_file_content_type 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 end
private private
def set_file_content_type def set_file_content_type(attachment) # rubocop:disable Naming/AccessorMethodName
self.class.attachment_definitions.each_key do |attachment_name| return if attachment.blank? || attachment.queued_for_write[:original].blank? || !INCORRECT_CONTENT_TYPES.include?(attachment.instance_read(:content_type))
attachment = send(attachment_name)
next 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) attachment.instance_write :content_type, calculated_content_type(attachment)
end end
end
def set_file_extensions def set_file_extension(attachment) # rubocop:disable Naming/AccessorMethodName
self.class.attachment_definitions.each_key do |attachment_name| return if attachment.blank?
attachment = send(attachment_name)
next if attachment.blank?
attachment.instance_write :file_name, [Paperclip::Interpolations.basename(attachment, :original), appropriate_extension(attachment)].delete_if(&:blank?).join('.') attachment.instance_write :file_name, [Paperclip::Interpolations.basename(attachment, :original), appropriate_extension(attachment)].delete_if(&:blank?).join('.')
end end
end
def check_image_dimensions def check_image_dimension(attachment)
self.class.attachment_definitions.each_key do |attachment_name| return if attachment.blank? || !/image.*/.match?(attachment.content_type) || attachment.queued_for_write[:original].blank?
attachment = send(attachment_name)
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) width, height = FastImage.size(attachment.queued_for_write[:original].path)
matrix_limit = attachment.content_type == 'image/gif' ? GIF_MATRIX_LIMIT : MAX_MATRIX_LIMIT 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) raise Mastodon::DimensionsValidationError, "#{width}x#{height} images are not supported" if width.present? && height.present? && (width * height > matrix_limit)
end end
end
def appropriate_extension(attachment) def appropriate_extension(attachment)
mime_type = MIME::Types[attachment.content_type] mime_type = MIME::Types[attachment.content_type]
@ -79,13 +76,9 @@ module Attachmentable
'' ''
end end
def obfuscate_file_name def obfuscate_file_name(attachment)
self.class.attachment_definitions.each_key do |attachment_name| return if attachment.blank? || attachment.queued_for_write[:original].blank? || attachment.options[:preserve_files]
attachment = send(attachment_name)
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)) attachment.instance_write :file_name, SecureRandom.hex(8) + File.extname(attachment.instance_read(:file_name))
end end
end
end end

View File

@ -21,6 +21,8 @@
# #
class CustomEmoji < ApplicationRecord class CustomEmoji < ApplicationRecord
include Attachmentable
LIMIT = 50.kilobytes LIMIT = 50.kilobytes
SHORTCODE_RE_FRAGMENT = '[a-zA-Z0-9_]{2,}' SHORTCODE_RE_FRAGMENT = '[a-zA-Z0-9_]{2,}'
@ -34,7 +36,7 @@ class CustomEmoji < ApplicationRecord
belongs_to :category, class_name: 'CustomEmojiCategory', optional: true 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_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 before_validation :downcase_domain
@ -49,8 +51,6 @@ class CustomEmoji < ApplicationRecord
remotable_attachment :image, LIMIT remotable_attachment :image, LIMIT
include Attachmentable
after_commit :remove_entity_cache after_commit :remove_entity_cache
def local? def local?

View File

@ -31,6 +31,8 @@
class MediaAttachment < ApplicationRecord class MediaAttachment < ApplicationRecord
self.inheritance_column = nil self.inheritance_column = nil
include Attachmentable
enum type: [:image, :gifv, :video, :unknown, :audio] enum type: [:image, :gifv, :video, :unknown, :audio]
enum processing: [:queued, :in_progress, :complete, :failed], _prefix: true 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 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_MIME_TYPES = %w(video/webm video/mp4 video/quicktime video/ogg).freeze
VIDEO_CONVERTIBLE_MIME_TYPES = %w(video/webm video/quicktime).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 = { BLURHASH_OPTIONS = {
x_comp: 4, x_comp: 4,
@ -182,8 +184,6 @@ class MediaAttachment < ApplicationRecord
validates_attachment_size :thumbnail, less_than: IMAGE_LIMIT validates_attachment_size :thumbnail, less_than: IMAGE_LIMIT
remotable_attachment :thumbnail, IMAGE_LIMIT, suppress_errors: true, download_on_assign: false remotable_attachment :thumbnail, IMAGE_LIMIT, suppress_errors: true, download_on_assign: false
include Attachmentable
validates :account, presence: true validates :account, presence: true
validates :description, length: { maximum: MAX_DESCRIPTION_LENGTH }, if: :local? validates :description, length: { maximum: MAX_DESCRIPTION_LENGTH }, if: :local?
validates :file, presence: true, if: :local? validates :file, presence: true, if: :local?

View File

@ -27,6 +27,8 @@
# #
class PreviewCard < ApplicationRecord class PreviewCard < ApplicationRecord
include Attachmentable
IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif'].freeze IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif'].freeze
LIMIT = 1.megabytes LIMIT = 1.megabytes
@ -41,9 +43,7 @@ class PreviewCard < ApplicationRecord
has_and_belongs_to_many :statuses 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' } has_attached_file :image, processors: [:thumbnail, :blurhash_transcoder], styles: ->(f) { image_styles(f) }, convert_options: { all: '-quality 80 -strip' }, validate_media_type: false
include Attachmentable
validates :url, presence: true, uniqueness: true validates :url, presence: true, uniqueness: true
validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES

View File

@ -25,11 +25,8 @@ require_relative '../lib/exceptions'
require_relative '../lib/enumerable' require_relative '../lib/enumerable'
require_relative '../lib/sanitize_ext/sanitize_config' require_relative '../lib/sanitize_ext/sanitize_config'
require_relative '../lib/redis/namespace_extensions' require_relative '../lib/redis/namespace_extensions'
require_relative '../lib/paperclip/schema_extensions'
require_relative '../lib/paperclip/validation_extensions'
require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/url_generator_extensions'
require_relative '../lib/paperclip/attachment_extensions' require_relative '../lib/paperclip/attachment_extensions'
require_relative '../lib/paperclip/media_type_spoof_detector_extensions'
require_relative '../lib/paperclip/lazy_thumbnail' require_relative '../lib/paperclip/lazy_thumbnail'
require_relative '../lib/paperclip/gif_transcoder' require_relative '../lib/paperclip/gif_transcoder'
require_relative '../lib/paperclip/transcoder' require_relative '../lib/paperclip/transcoder'

View File

@ -6,6 +6,35 @@ module Paperclip
instance_read(:meta) instance_read(:meta)
end end
# monkey-patch to avoid unlinking too avoid unlinking source file too early
# see https://github.com/kreeti/kt-paperclip/issues/64
def post_process_style(name, style) #:nodoc:
raise "Style #{name} has no processors defined." if style.processors.blank?
intermediate_files = []
original = @queued_for_write[:original]
# if we're processing the original, close + unlink the source tempfile
intermediate_files << original if name == :original
@queued_for_write[name] = style.processors.
inject(original) do |file, processor|
file = Paperclip.processor(processor).make(file, style.processor_options, self)
intermediate_files << file unless file == original
file
end
unadapted_file = @queued_for_write[name]
@queued_for_write[name] = Paperclip.io_adapters.
for(@queued_for_write[name], @options[:adapter_options])
unadapted_file.close if unadapted_file.respond_to?(:close)
@queued_for_write[name]
rescue Paperclip::Errors::NotIdentifiedByImageMagickError => e
log("An error was received while processing: #{e.inspect}")
(@errors[:processing] ||= []) << e.message if @options[:whiny]
ensure
unlink_files(intermediate_files)
end
# We overwrite this method to support delayed processing in # We overwrite this method to support delayed processing in
# Sidekiq. Since we process the original file to reduce disk # Sidekiq. Since we process the original file to reduce disk
# usage, and we still want to generate thumbnails straight # usage, and we still want to generate thumbnails straight

View File

@ -1,35 +0,0 @@
# frozen_string_literal: true
module Paperclip
module MediaTypeSpoofDetectorExtensions
def mapping_override_mismatch?
!Array(mapped_content_type).include?(calculated_content_type) && !Array(mapped_content_type).include?(type_from_mime_magic)
end
def calculated_media_type_from_mime_magic
@calculated_media_type_from_mime_magic ||= type_from_mime_magic.split('/').first
end
def calculated_type_mismatch?
!media_types_from_name.include?(calculated_media_type) && !media_types_from_name.include?(calculated_media_type_from_mime_magic)
end
def type_from_mime_magic
@type_from_mime_magic ||= begin
begin
File.open(@file.path) do |file|
MimeMagic.by_magic(file)&.type || ''
end
rescue Errno::ENOENT
''
end
end
end
def type_from_file_command
@type_from_file_command ||= FileCommandContentTypeDetector.new(@file.path).detect
end
end
end
Paperclip::MediaTypeSpoofDetector.prepend(Paperclip::MediaTypeSpoofDetectorExtensions)

View File

@ -1,37 +0,0 @@
# frozen_string_literal: true
# Monkey-patch various Paperclip methods for Ruby 3.0 compatibility
module Paperclip
module Schema
module StatementsExtensions
def add_attachment(table_name, *attachment_names)
raise ArgumentError, 'Please specify attachment name in your add_attachment call in your migration.' if attachment_names.empty?
options = attachment_names.extract_options!
attachment_names.each do |attachment_name|
COLUMNS.each_pair do |column_name, column_type|
column_options = options.merge(options[column_name.to_sym] || {})
add_column(table_name, "#{attachment_name}_#{column_name}", column_type, **column_options)
end
end
end
end
module TableDefinitionExtensions
def attachment(*attachment_names)
options = attachment_names.extract_options!
attachment_names.each do |attachment_name|
COLUMNS.each_pair do |column_name, column_type|
column_options = options.merge(options[column_name.to_sym] || {})
column("#{attachment_name}_#{column_name}", column_type, **column_options)
end
end
end
end
end
end
Paperclip::Schema::Statements.prepend(Paperclip::Schema::StatementsExtensions)
Paperclip::Schema::TableDefinition.prepend(Paperclip::Schema::TableDefinitionExtensions)

View File

@ -2,16 +2,6 @@
module Paperclip module Paperclip
module UrlGeneratorExtensions module UrlGeneratorExtensions
# Monkey-patch Paperclip to use Addressable::URI's normalization instead
# of the long-deprecated URI.esacpe
def escape_url(url)
if url.respond_to?(:escape)
url.escape
else
Addressable::URI.parse(url).normalize.to_str.gsub(escape_regex) { |m| "%#{m.ord.to_s(16).upcase}" }
end
end
def for_as_default(style_name) def for_as_default(style_name)
attachment_options[:interpolator].interpolate(default_url, @attachment, style_name) attachment_options[:interpolator].interpolate(default_url, @attachment, style_name)
end end

View File

@ -1,58 +0,0 @@
# frozen_string_literal: true
# Monkey-patch various Paperclip validators for Ruby 3.0 compatibility
module Paperclip
module Validators
module AttachmentSizeValidatorExtensions
def validate_each(record, attr_name, _value)
base_attr_name = attr_name
attr_name = "#{attr_name}_file_size".to_sym
value = record.send(:read_attribute_for_validation, attr_name)
if value.present?
options.slice(*Paperclip::Validators::AttachmentSizeValidator::AVAILABLE_CHECKS).each do |option, option_value|
option_value = option_value.call(record) if option_value.is_a?(Proc)
option_value = extract_option_value(option, option_value)
next if value.send(Paperclip::Validators::AttachmentSizeValidator::CHECKS[option], option_value)
error_message_key = options[:in] ? :in_between : option
[attr_name, base_attr_name].each do |error_attr_name|
record.errors.add(error_attr_name, error_message_key, **filtered_options(value).merge(
min: min_value_in_human_size(record),
max: max_value_in_human_size(record),
count: human_size(option_value)
))
end
end
end
end
end
module AttachmentContentTypeValidatorExtensions
def mark_invalid(record, attribute, types)
record.errors.add attribute, :invalid, **options.merge({ types: types.join(', ') })
end
end
module AttachmentPresenceValidatorExtensions
def validate_each(record, attribute, _value)
if record.send("#{attribute}_file_name").blank?
record.errors.add(attribute, :blank, **options)
end
end
end
module AttachmentFileNameValidatorExtensions
def mark_invalid(record, attribute, patterns)
record.errors.add attribute, :invalid, options.merge({ names: patterns.join(', ') })
end
end
end
end
Paperclip::Validators::AttachmentSizeValidator.prepend(Paperclip::Validators::AttachmentSizeValidatorExtensions)
Paperclip::Validators::AttachmentContentTypeValidator.prepend(Paperclip::Validators::AttachmentContentTypeValidatorExtensions)
Paperclip::Validators::AttachmentPresenceValidator.prepend(Paperclip::Validators::AttachmentPresenceValidatorExtensions)
Paperclip::Validators::AttachmentFileNameValidator.prepend(Paperclip::Validators::AttachmentFileNameValidatorExtensions)

BIN
spec/fixtures/files/boop.ogg vendored Normal file

Binary file not shown.

View File

@ -114,6 +114,30 @@ RSpec.describe MediaAttachment, type: :model do
end end
end end
describe 'ogg with cover art' do
let(:media) { MediaAttachment.create(account: Fabricate(:account), file: attachment_fixture('boop.ogg')) }
it 'detects it as an audio file' do
expect(media.type).to eq 'audio'
end
it 'sets meta for the duration' do
expect(media.file.meta['original']['duration']).to be_within(0.05).of(0.235102)
end
it 'extracts thumbnail' do
expect(media.thumbnail.present?).to eq true
end
it 'extracts colors from thumbnail' do
expect(media.file.meta['colors']['background']).to eq '#3088d4'
end
it 'gives the file a random name' do
expect(media.file_file_name).to_not eq 'boop.ogg'
end
end
describe 'jpeg' do describe 'jpeg' do
let(:media) { MediaAttachment.create(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) } let(:media) { MediaAttachment.create(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) }