Skip to content

Commit 051c181

Browse files
escape filename in the multipart/form-data Content-Disposition header
1 parent c27adec commit 051c181

File tree

2 files changed

+40
-1
lines changed

2 files changed

+40
-1
lines changed

lib/httparty/request/body.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ def multipart?
3232

3333
private
3434

35+
# https://html.spec.whatwg.org/#multipart-form-data
36+
MULTIPART_FORM_DATA_REPLACEMENT_TABLE = {
37+
'"' => '%22',
38+
"\r" => '%0D',
39+
"\n" => '%0A'
40+
}.freeze
41+
3542
def generate_multipart
3643
normalized_params = params.flat_map { |key, value| HashConversions.normalize_keys(key, value) }
3744

@@ -40,7 +47,7 @@ def generate_multipart
4047
memo << %(Content-Disposition: form-data; name="#{key}")
4148
# value.path is used to support ActionDispatch::Http::UploadedFile
4249
# https://github.com/jnunemaker/httparty/pull/585
43-
memo << %(; filename="#{file_name(value)}") if file?(value)
50+
memo << %(; filename="#{file_name(value).gsub(/["\r\n]/, MULTIPART_FORM_DATA_REPLACEMENT_TABLE)}") if file?(value)
4451
memo << NEWLINE
4552
memo << "Content-Type: #{content_type(value)}#{NEWLINE}" if file?(value)
4653
memo << NEWLINE

spec/httparty/request/body_spec.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,38 @@
105105

106106
it { is_expected.to eq multipart_params }
107107
end
108+
109+
context 'when file name contains [ " \r \n ]' do
110+
let(:options) { { force_multipart: true } }
111+
let(:some_temp_file) { Tempfile.new(['basefile', '.txt']) }
112+
let(:file_content) { 'test' }
113+
let(:raw_filename) { "dummy=tampering.sh\"; \r\ndummy=a.txt" }
114+
let(:expected_file_name) { 'dummy=tampering.sh%22; %0D%0Adummy=a.txt' }
115+
let(:file) { double(:mocked_action_dispatch, path: some_temp_file.path, original_filename: raw_filename, read: file_content) }
116+
let(:params) do
117+
{
118+
user: {
119+
attachment_file: file,
120+
enabled: true
121+
}
122+
}
123+
end
124+
let(:multipart_params) do
125+
"--------------------------c772861a5109d5ef\r\n" \
126+
"Content-Disposition: form-data; name=\"user[attachment_file]\"; filename=\"#{expected_file_name}\"\r\n" \
127+
"Content-Type: text/plain\r\n" \
128+
"\r\n" \
129+
"test\r\n" \
130+
"--------------------------c772861a5109d5ef\r\n" \
131+
"Content-Disposition: form-data; name=\"user[enabled]\"\r\n" \
132+
"\r\n" \
133+
"true\r\n" \
134+
"--------------------------c772861a5109d5ef--\r\n"
135+
end
136+
137+
it { is_expected.to eq multipart_params }
138+
139+
end
108140
end
109141
end
110142
end

0 commit comments

Comments
 (0)