Skip to content

Commit ecb3de2

Browse files
Mth0158Mth0158
authored
Merge pull request #296 from igorkasyanchuk/293-video-content-types-not-working-since-131
[Validator] Remove extension check on content_type validator (#282/#291/#292/#293)
2 parents b726c6d + 83a8a54 commit ecb3de2

File tree

2 files changed

+117
-105
lines changed

2 files changed

+117
-105
lines changed

‎lib/active_storage_validations/content_type_validator.rb

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,23 +56,32 @@ def set_attachable_cached_values(attachable)
5656
@attachable_filename = attachable_filename(attachable).to_s
5757
end
5858

59+
# Check if the provided content_type is authorized and not spoofed against
60+
# the file io.
5961
def is_valid?(record, attribute, attachable)
60-
extension_matches_content_type?(record, attribute, attachable) &&
61-
authorized_content_type?(record, attribute, attachable) &&
62+
authorized_content_type?(record, attribute, attachable) &&
6263
not_spoofing_content_type?(record, attribute, attachable)
6364
end
6465

65-
def extension_matches_content_type?(record, attribute, attachable)
66-
return true if !@attachable_filename || !@attachable_content_type
67-
68-
extension = @attachable_filename.split('.').last
69-
possible_extensions = Marcel::TYPE_EXTS[@attachable_content_type]
70-
return true if possible_extensions && extension.downcase.in?(possible_extensions)
71-
72-
errors_options = initialize_and_populate_error_options(options, attachable)
73-
add_error(record, attribute, ERROR_TYPES.first, **errors_options)
74-
false
75-
end
66+
# Dead code that we keep here for some time, maybe we will find a solution
67+
# to this check later? (November 2024)
68+
#
69+
# We do not perform any validations against the extension because it is an
70+
# unreliable source of truth. For example, a `.csv` file could have its
71+
# `text/csv` content_type changed to `application/vnd.ms-excel` because
72+
# it had been opened by Excel at some point, making the file extension vs
73+
# file content_type check invalid.
74+
# def extension_matches_content_type?(record, attribute, attachable)
75+
# return true if !@attachable_filename || !@attachable_content_type
76+
77+
# extension = @attachable_filename.split('.').last
78+
# possible_extensions = Marcel::TYPE_EXTS[@attachable_content_type]
79+
# return true if possible_extensions && extension.downcase.in?(possible_extensions)
80+
81+
# errors_options = initialize_and_populate_error_options(options, attachable)
82+
# add_error(record, attribute, ERROR_TYPES.first, **errors_options)
83+
# false
84+
# end
7685

7786
def authorized_content_type?(record, attribute, attachable)
7887
attachable_content_type_is_authorized = @authorized_content_types.any? do |authorized_content_type|

‎test/validators/content_type_validator_test.rb

Lines changed: 95 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -70,98 +70,101 @@
7070

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

73-
describe "#extension_matches_content_type?" do
74-
describe "when the attachable content_type and extension cannot match (e.g. 'text/plain' and .png)" do
75-
subject { model.public_send(attribute).attach(file_with_issue) and model }
76-
77-
let(:attribute) { :extension_content_type_mismatch }
78-
let(:file_with_issue) { extension_content_type_mismatch_file }
79-
let(:error_options) do
80-
{
81-
authorized_types: "PNG",
82-
content_type: file_with_issue[:content_type],
83-
filename: file_with_issue[:filename]
84-
}
85-
end
86-
87-
it { is_expected_not_to_be_valid }
88-
it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
89-
it { is_expected_to_have_error_options(error_options) }
90-
end
91-
92-
describe "when the file has 2 extensions (e.g. hello.docx.pdf)" do
93-
describe "and we only allow the first extension content_type (e.g. 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' (docx))" do
94-
subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }
95-
96-
let(:attribute) { :extension_two_extensions_docx }
97-
let(:docx_file_with_two_extensions) do
98-
docx_file.tap do |file|
99-
file[:filename] += ".pdf"
100-
end
101-
end
102-
let(:error_options) do
103-
{
104-
authorized_types: "DOCX",
105-
content_type: docx_file_with_two_extensions[:content_type],
106-
filename: docx_file_with_two_extensions[:filename]
107-
}
108-
end
109-
110-
it { is_expected_not_to_be_valid }
111-
it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
112-
it { is_expected_to_have_error_options(error_options) }
113-
end
114-
115-
describe "and we allow the last extension content_type (e.g. 'application/pdf')" do
116-
subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }
117-
118-
let(:attribute) { :extension_two_extensions_pdf }
119-
let(:docx_file_with_two_extensions) do
120-
docx_file.tap do |file|
121-
file[:filename] += ".pdf"
122-
file[:content_type] = "application/pdf"
123-
end
124-
end
125-
126-
it { is_expected_to_be_valid }
127-
end
128-
end
129-
130-
describe "when the extension is in uppercase" do
131-
subject { model.public_send(attribute).attach(pdf_file_with_extension_in_uppercase) and model }
132-
133-
let(:attribute) { :extension_upcase_extension }
134-
let(:pdf_file_with_extension_in_uppercase) do
135-
pdf_file.tap do |file|
136-
file[:filename][".pdf"] = ".PDF"
137-
end
138-
end
139-
140-
it { is_expected_to_be_valid }
141-
end
142-
143-
describe "when the extension is missing" do
144-
subject { model.public_send(attribute).attach(pdf_file_without_extension) and model }
145-
146-
let(:attribute) { :extension_missing_extension }
147-
let(:pdf_file_without_extension) do
148-
pdf_file.tap do |file|
149-
file[:filename][".pdf"] = ""
150-
end
151-
end
152-
let(:error_options) do
153-
{
154-
authorized_types: "PDF",
155-
content_type: pdf_file_without_extension[:content_type],
156-
filename: pdf_file_without_extension[:filename]
157-
}
158-
end
159-
160-
it { is_expected_not_to_be_valid }
161-
it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
162-
it { is_expected_to_have_error_options(error_options) }
163-
end
164-
end
73+
# Dead code that we keep here for some time, maybe we will find a solution
74+
# to this check later? (November 2024)
75+
#
76+
# describe "#extension_matches_content_type?" do
77+
# describe "when the attachable content_type and extension cannot match (e.g. 'text/plain' and .png)" do
78+
# subject { model.public_send(attribute).attach(file_with_issue) and model }
79+
80+
# let(:attribute) { :extension_content_type_mismatch }
81+
# let(:file_with_issue) { extension_content_type_mismatch_file }
82+
# let(:error_options) do
83+
# {
84+
# authorized_types: "PNG",
85+
# content_type: file_with_issue[:content_type],
86+
# filename: file_with_issue[:filename]
87+
# }
88+
# end
89+
90+
# it { is_expected_not_to_be_valid }
91+
# it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
92+
# it { is_expected_to_have_error_options(error_options) }
93+
# end
94+
95+
# describe "when the file has 2 extensions (e.g. hello.docx.pdf)" do
96+
# describe "and we only allow the first extension content_type (e.g. 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' (docx))" do
97+
# subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }
98+
99+
# let(:attribute) { :extension_two_extensions_docx }
100+
# let(:docx_file_with_two_extensions) do
101+
# docx_file.tap do |file|
102+
# file[:filename] += ".pdf"
103+
# end
104+
# end
105+
# let(:error_options) do
106+
# {
107+
# authorized_types: "DOCX",
108+
# content_type: docx_file_with_two_extensions[:content_type],
109+
# filename: docx_file_with_two_extensions[:filename]
110+
# }
111+
# end
112+
113+
# it { is_expected_not_to_be_valid }
114+
# it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
115+
# it { is_expected_to_have_error_options(error_options) }
116+
# end
117+
118+
# describe "and we allow the last extension content_type (e.g. 'application/pdf')" do
119+
# subject { model.public_send(attribute).attach(docx_file_with_two_extensions) and model }
120+
121+
# let(:attribute) { :extension_two_extensions_pdf }
122+
# let(:docx_file_with_two_extensions) do
123+
# docx_file.tap do |file|
124+
# file[:filename] += ".pdf"
125+
# file[:content_type] = "application/pdf"
126+
# end
127+
# end
128+
129+
# it { is_expected_to_be_valid }
130+
# end
131+
# end
132+
133+
# describe "when the extension is in uppercase" do
134+
# subject { model.public_send(attribute).attach(pdf_file_with_extension_in_uppercase) and model }
135+
136+
# let(:attribute) { :extension_upcase_extension }
137+
# let(:pdf_file_with_extension_in_uppercase) do
138+
# pdf_file.tap do |file|
139+
# file[:filename][".pdf"] = ".PDF"
140+
# end
141+
# end
142+
143+
# it { is_expected_to_be_valid }
144+
# end
145+
146+
# describe "when the extension is missing" do
147+
# subject { model.public_send(attribute).attach(pdf_file_without_extension) and model }
148+
149+
# let(:attribute) { :extension_missing_extension }
150+
# let(:pdf_file_without_extension) do
151+
# pdf_file.tap do |file|
152+
# file[:filename][".pdf"] = ""
153+
# end
154+
# end
155+
# let(:error_options) do
156+
# {
157+
# authorized_types: "PDF",
158+
# content_type: pdf_file_without_extension[:content_type],
159+
# filename: pdf_file_without_extension[:filename]
160+
# }
161+
# end
162+
163+
# it { is_expected_not_to_be_valid }
164+
# it { is_expected_to_have_error_message("content_type_invalid", error_options: error_options) }
165+
# it { is_expected_to_have_error_options(error_options) }
166+
# end
167+
# end
165168

166169
describe ":with" do
167170
# validates :with_string, content_type: 'png'

0 commit comments

Comments
 (0)