Skip to content

Commit

Permalink
[Validator] Remove extension check on content_type validator (#282/#291/
Browse files Browse the repository at this point in the history 
#292/#293)
  • Loading branch information
@Mth0158
Mth0158 committed Nov 24, 2024
1 parent 5722abe commit a5ac5bd
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 105 deletions.
35 changes: 22 additions & 13 deletions lib/active_storage_validations/content_type_validator.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,23 +56,32 @@ def set_attachable_cached_values(attachable)
@attachable_filename = attachable_filename(attachable).to_s
end

# Check if the provided content_type is authorized and not spoofed against
# the file io.
def is_valid?(record, attribute, attachable)
extension_matches_content_type?(record, attribute, attachable) &&
authorized_content_type?(record, attribute, attachable) &&
authorized_content_type?(record, attribute, attachable) &&
not_spoofing_content_type?(record, attribute, attachable)
end

def extension_matches_content_type?(record, attribute, attachable)
return true if !@attachable_filename || !@attachable_content_type

extension = @attachable_filename.split('.').last
possible_extensions = Marcel::TYPE_EXTS[@attachable_content_type]
return true if possible_extensions && extension.downcase.in?(possible_extensions)

errors_options = initialize_and_populate_error_options(options, attachable)
add_error(record, attribute, ERROR_TYPES.first, **errors_options)
false
end
# Dead code that we keep here for some time, maybe we will find a solution
# to this check later? (November 2024)
#
# We do not perform any validations against the extension because it is an
# unreliable source of truth. For example, a `.csv` file could have its
# `text/csv` content_type changed to `application/vnd.ms-excel` because
# it had been opened by Excel at some point, making the file extension vs
# file content_type check invalid.
# def extension_matches_content_type?(record, attribute, attachable)
# return true if !@attachable_filename || !@attachable_content_type

# extension = @attachable_filename.split('.').last
# possible_extensions = Marcel::TYPE_EXTS[@attachable_content_type]
# return true if possible_extensions && extension.downcase.in?(possible_extensions)

# errors_options = initialize_and_populate_error_options(options, attachable)
# add_error(record, attribute, ERROR_TYPES.first, **errors_options)
# false
# end

def authorized_content_type?(record, attribute, attachable)
attachable_content_type_is_authorized = @authorized_content_types.any? do |authorized_content_type|
Expand Down
187 changes: 95 additions & 92 deletions test/validators/content_type_validator_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,98 +70,101 @@

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

describe "#extension_matches_content_type?" do
describe "when the attachable content_type and extension cannot match (e.g. 'text/plain' and .png)" do
subject { model.public_send(attribute).attach(file_with_issue) and model }

let(:attribute) { :extension_content_type_mismatch }
let(:file_with_issue) { extension_content_type_mismatch_file }
let(:error_options) do
{
authorized_types: "PNG",
content_type: file_with_issue[:content_type],
filename: file_with_issue[:filename]
}
end

it { is_expected_not_to_be_valid }
it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
it { is_expected_to_have_error_options(error_options) }
end

describe "when the file has 2 extensions (e.g. hello.docx.pdf)" do
describe "and we only allow the first extension content_type (e.g. 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' (docx))" do
subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }

let(:attribute) { :extension_two_extensions_docx }
let(:docx_file_with_two_extensions) do
docx_file.tap do |file|
file[:filename] += ".pdf"
end
end
let(:error_options) do
{
authorized_types: "DOCX",
content_type: docx_file_with_two_extensions[:content_type],
filename: docx_file_with_two_extensions[:filename]
}
end

it { is_expected_not_to_be_valid }
it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
it { is_expected_to_have_error_options(error_options) }
end

describe "and we allow the last extension content_type (e.g. 'application/pdf')" do
subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }

let(:attribute) { :extension_two_extensions_pdf }
let(:docx_file_with_two_extensions) do
docx_file.tap do |file|
file[:filename] += ".pdf"
file[:content_type] = "application/pdf"
end
end

it { is_expected_to_be_valid }
end
end

describe "when the extension is in uppercase" do
subject { model.public_send(attribute).attach(pdf_file_with_extension_in_uppercase) and model }

let(:attribute) { :extension_upcase_extension }
let(:pdf_file_with_extension_in_uppercase) do
pdf_file.tap do |file|
file[:filename][".pdf"] = ".PDF"
end
end

it { is_expected_to_be_valid }
end

describe "when the extension is missing" do
subject { model.public_send(attribute).attach(pdf_file_without_extension) and model }

let(:attribute) { :extension_missing_extension }
let(:pdf_file_without_extension) do
pdf_file.tap do |file|
file[:filename][".pdf"] = ""
end
end
let(:error_options) do
{
authorized_types: "PDF",
content_type: pdf_file_without_extension[:content_type],
filename: pdf_file_without_extension[:filename]
}
end

it { is_expected_not_to_be_valid }
it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
it { is_expected_to_have_error_options(error_options) }
end
end
# Dead code that we keep here for some time, maybe we will find a solution
# to this check later? (November 2024)
#
# describe "#extension_matches_content_type?" do
# describe "when the attachable content_type and extension cannot match (e.g. 'text/plain' and .png)" do
# subject { model.public_send(attribute).attach(file_with_issue) and model }

# let(:attribute) { :extension_content_type_mismatch }
# let(:file_with_issue) { extension_content_type_mismatch_file }
# let(:error_options) do
# {
# authorized_types: "PNG",
# content_type: file_with_issue[:content_type],
# filename: file_with_issue[:filename]
# }
# end

# it { is_expected_not_to_be_valid }
# it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
# it { is_expected_to_have_error_options(error_options) }
# end

# describe "when the file has 2 extensions (e.g. hello.docx.pdf)" do
# describe "and we only allow the first extension content_type (e.g. 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' (docx))" do
# subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }

# let(:attribute) { :extension_two_extensions_docx }
# let(:docx_file_with_two_extensions) do
# docx_file.tap do |file|
# file[:filename] += ".pdf"
# end
# end
# let(:error_options) do
# {
# authorized_types: "DOCX",
# content_type: docx_file_with_two_extensions[:content_type],
# filename: docx_file_with_two_extensions[:filename]
# }
# end

# it { is_expected_not_to_be_valid }
# it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
# it { is_expected_to_have_error_options(error_options) }
# end

# describe "and we allow the last extension content_type (e.g. 'application/pdf')" do
# subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }

# let(:attribute) { :extension_two_extensions_pdf }
# let(:docx_file_with_two_extensions) do
# docx_file.tap do |file|
# file[:filename] += ".pdf"
# file[:content_type] = "application/pdf"
# end
# end

# it { is_expected_to_be_valid }
# end
# end

# describe "when the extension is in uppercase" do
# subject { model.public_send(attribute).attach(pdf_file_with_extension_in_uppercase) and model }

# let(:attribute) { :extension_upcase_extension }
# let(:pdf_file_with_extension_in_uppercase) do
# pdf_file.tap do |file|
# file[:filename][".pdf"] = ".PDF"
# end
# end

# it { is_expected_to_be_valid }
# end

# describe "when the extension is missing" do
# subject { model.public_send(attribute).attach(pdf_file_without_extension) and model }

# let(:attribute) { :extension_missing_extension }
# let(:pdf_file_without_extension) do
# pdf_file.tap do |file|
# file[:filename][".pdf"] = ""
# end
# end
# let(:error_options) do
# {
# authorized_types: "PDF",
# content_type: pdf_file_without_extension[:content_type],
# filename: pdf_file_without_extension[:filename]
# }
# end

# it { is_expected_not_to_be_valid }
# it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
# it { is_expected_to_have_error_options(error_options) }
# end
# end

describe ":with" do
# validates :with_string, content_type: 'png'
Expand Down

0 comments on commit a5ac5bd

Please sign in to comment.