Fix media attachment size validation not correctly accounting for file type (#16819)
* Fix media attachment size validation not correctly accounting for file type Fixes a regression introduced in #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
This commit is contained in:
		
							parent
							
								
									900481b7fa
								
							
						
					
					
						commit
						84ceebe1c4
					
				@ -167,12 +167,11 @@ class MediaAttachment < ApplicationRecord
 | 
				
			|||||||
                    processors: ->(f) { file_processors f },
 | 
					                    processors: ->(f) { file_processors f },
 | 
				
			||||||
                    convert_options: GLOBAL_CONVERT_OPTIONS
 | 
					                    convert_options: GLOBAL_CONVERT_OPTIONS
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  before_file_post_process :set_type_and_extension
 | 
					  before_file_validate :set_type_and_extension
 | 
				
			||||||
  before_file_post_process :check_video_dimensions
 | 
					  before_file_validate :check_video_dimensions
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  validates_attachment_content_type :file, content_type: IMAGE_MIME_TYPES + VIDEO_MIME_TYPES + AUDIO_MIME_TYPES
 | 
					  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: ->(m) { m.larger_media_format? ? VIDEO_LIMIT : IMAGE_LIMIT }
 | 
				
			||||||
  validates_attachment_size :file, less_than: VIDEO_LIMIT, if: :larger_media_format?
 | 
					 | 
				
			||||||
  remotable_attachment :file, VIDEO_LIMIT, suppress_errors: false, download_on_assign: false, attribute_name: :remote_url
 | 
					  remotable_attachment :file, VIDEO_LIMIT, suppress_errors: false, download_on_assign: false, attribute_name: :remote_url
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  has_attached_file :thumbnail,
 | 
					  has_attached_file :thumbnail,
 | 
				
			||||||
 | 
				
			|||||||
@ -181,4 +181,32 @@ RSpec.describe MediaAttachment, type: :model do
 | 
				
			|||||||
      expect(media.description.size).to be <= 1_500
 | 
					      expect(media.description.size).to be <= 1_500
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  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
 | 
					end
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user