Skip to content

Commit

Permalink
Fix media attachment size validation not correctly accounting for fil…
Browse files Browse the repository at this point in the history
…e type (mastodon#16819)

* Fix media attachment size validation not correctly accounting for file type

Fixes a regression introduced in mastodon#16724 caused by the fact that kt-paperclip
now correctly runs validations before processing, meaning that file size
verification could not rely on our before_post_processing hook.

Moved the `before_post_processing` hooks to `before_validate` to make sure
the media attachment type is set correctly before the file gets validated.

* Add tests
  • Loading branch information
ClearlyClaire authored Oct 6, 2021
1 parent 900481b commit 84ceebe
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
7 changes: 3 additions & 4 deletions app/models/media_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,11 @@ class MediaAttachment < ApplicationRecord
processors: ->(f) { file_processors f },
convert_options: GLOBAL_CONVERT_OPTIONS

before_file_post_process :set_type_and_extension
before_file_post_process :check_video_dimensions
before_file_validate :set_type_and_extension
before_file_validate :check_video_dimensions

validates_attachment_content_type :file, content_type: IMAGE_MIME_TYPES + VIDEO_MIME_TYPES + AUDIO_MIME_TYPES
validates_attachment_size :file, less_than: IMAGE_LIMIT, unless: :larger_media_format?
validates_attachment_size :file, less_than: VIDEO_LIMIT, if: :larger_media_format?
validates_attachment_size :file, less_than: ->(m) { m.larger_media_format? ? VIDEO_LIMIT : IMAGE_LIMIT }
remotable_attachment :file, VIDEO_LIMIT, suppress_errors: false, download_on_assign: false, attribute_name: :remote_url

has_attached_file :thumbnail,
Expand Down
28 changes: 28 additions & 0 deletions spec/models/media_attachment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,32 @@
expect(media.description.size).to be <= 1_500
end
end

describe 'size limit validation' do
it 'rejects video files that are too large' do
stub_const 'MediaAttachment::IMAGE_LIMIT', 100.megabytes
stub_const 'MediaAttachment::VIDEO_LIMIT', 1.kilobyte
expect { MediaAttachment.create!(account: Fabricate(:account), file: attachment_fixture('attachment.webm')) }.to raise_error(ActiveRecord::RecordInvalid)
end

it 'accepts video files that are small enough' do
stub_const 'MediaAttachment::IMAGE_LIMIT', 1.kilobyte
stub_const 'MediaAttachment::VIDEO_LIMIT', 100.megabytes
media = MediaAttachment.create!(account: Fabricate(:account), file: attachment_fixture('attachment.webm'))
expect(media.valid?).to be true
end

it 'rejects image files that are too large' do
stub_const 'MediaAttachment::IMAGE_LIMIT', 1.kilobyte
stub_const 'MediaAttachment::VIDEO_LIMIT', 100.megabytes
expect { MediaAttachment.create!(account: Fabricate(:account), file: attachment_fixture('attachment.jpg')) }.to raise_error(ActiveRecord::RecordInvalid)
end

it 'accepts image files that are small enough' do
stub_const 'MediaAttachment::IMAGE_LIMIT', 100.megabytes
stub_const 'MediaAttachment::VIDEO_LIMIT', 1.kilobyte
media = MediaAttachment.create!(account: Fabricate(:account), file: attachment_fixture('attachment.jpg'))
expect(media.valid?).to be true
end
end
end

0 comments on commit 84ceebe

Please sign in to comment.