Skip to content

Commit

Permalink
Merge pull request #332 from igorkasyanchuk/331-validator-optimise-co…
Browse files Browse the repository at this point in the history
…ntent_type-validator

[Validator] Optimise content_type validator check when not using spoofing protection (#331)
  • Loading branch information
Mth0158 authored Dec 21, 2024
2 parents 1b2d5dc + de1ec27 commit 3d5ea37
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def content_type_from_analyzer
end

def open3_mime_type_for_io
return nil if io.blank?
return nil if io.bytesize == 0

Tempfile.create do |tempfile|
tempfile.binmode
Expand Down
16 changes: 11 additions & 5 deletions lib/active_storage_validations/content_type_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ def validate_each(record, attribute, _value)
@authorized_content_types = authorized_content_types_from_options(record)
return if @authorized_content_types.empty?

attachables_from_changes(record, attribute).each do |attachable|
set_attachable_cached_values(attachable)
is_valid?(record, attribute, attachable)
checked_files = disable_spoofing_protection? ? attached_files(record, attribute) : attachables_from_changes(record, attribute)

checked_files.each do |file|
set_attachable_cached_values(file)
is_valid?(record, attribute, file)
end
end

Expand All @@ -54,8 +56,8 @@ def authorized_content_types_from_options(record)
end

def set_attachable_cached_values(attachable)
@attachable_content_type = attachable_content_type_rails_like(attachable)
@attachable_filename = attachable_filename(attachable).to_s
@attachable_content_type = disable_spoofing_protection? ? attachable.blob.content_type : attachable_content_type_rails_like(attachable)
@attachable_filename = disable_spoofing_protection? ? attachable.blob.filename.to_s : attachable_filename(attachable).to_s
end

# Check if the provided content_type is authorized and not spoofed against
Expand Down Expand Up @@ -116,6 +118,10 @@ def marcel_attachable_content_type(attachable)
Marcel::MimeType.for(declared_type: @attachable_content_type, name: @attachable_filename)
end

def disable_spoofing_protection?
!enable_spoofing_protection?
end

def enable_spoofing_protection?
options[:spoofing_protection] == true
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ module ASVActiveStorageable

private

# Retrieve either an ActiveStorage::Attached::One or an
# ActiveStorage::Attached::Many instance depending on attribute definition
# Retrieve either an `ActiveStorage::Attached::One` or an
# `ActiveStorage::Attached::Many` instance depending on attribute definition
def attached_files(record, attribute)
Array.wrap(record.send(attribute))
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
class ContentType::Validator::UsingAttachable < ApplicationRecord
has_one_attached :using_attachable
has_many_attached :using_attachables
validates :using_attachable, content_type: :png
validates :using_attachables, content_type: :png
validates :using_attachable, content_type: { with: :png, spoofing_protection: true }
validates :using_attachables, content_type: { with: :png, spoofing_protection: true }
end
2 changes: 1 addition & 1 deletion test/validators/content_type_validator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
end

describe 'Validator checks' do
include WorksFineWithAttachables
include WorksFineWithAttachables # when using spoofing_protection

let(:model) { validator_test_class::Check.new(params) }

Expand Down

0 comments on commit 3d5ea37

Please sign in to comment.