From 319dd6290c569e0519adc9b0dc290346833fa378 Mon Sep 17 00:00:00 2001 From: Gary Grossman Date: Sun, 6 Sep 2015 00:20:30 -0700 Subject: [PATCH 1/2] Fix bug in parsing of Content-Disposition header where an unquoted name at end-of-line sucked in the trailing newline. --- lib/rack/multipart.rb | 2 +- lib/rack/multipart/parser.rb | 1 + test/spec_multipart.rb | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/rack/multipart.rb b/lib/rack/multipart.rb index 12c3917b6..cc2d6ebbf 100644 --- a/lib/rack/multipart.rb +++ b/lib/rack/multipart.rb @@ -17,7 +17,7 @@ module Multipart BROKEN_QUOTED = /^#{CONDISP}.*;\sfilename="(.*?)"(?:\s*$|\s*;\s*#{TOKEN}=)/i BROKEN_UNQUOTED = /^#{CONDISP}.*;\sfilename=(#{TOKEN})/i MULTIPART_CONTENT_TYPE = /Content-Type: (.*)#{EOL}/ni - MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*\s+name="?([^\";]*)"?/ni + MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*\s+name=(#{TOKEN}|"[^\";]*")/ni MULTIPART_CONTENT_ID = /Content-ID:\s*([^#{EOL}]*)/ni # Updated definitions from RFC 2231 ATTRIBUTE_CHAR = %r{[^ \t\v\n\r)(><@,;:\\"/\[\]?='*%]} diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index 8196207c4..bc0162b4e 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -246,6 +246,7 @@ def handle_mime_head content_type = head[MULTIPART_CONTENT_TYPE, 1] name = head[MULTIPART_CONTENT_DISPOSITION, 1] || head[MULTIPART_CONTENT_ID, 1] + name.gsub!(/\A"|"\Z/, '') if name filename = get_filename(head) diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index cfe508c12..7f7e68564 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -606,6 +606,26 @@ def initialize(*) params["file"][:filename].must_equal 'long' * 100 end + it "parse unquoted parameter values at end of line" do + data = <<-EOF +--AaB03x\r +Content-Type: text/plain\r +Content-Disposition: attachment; name=inline\r +\r +true\r +--AaB03x--\r + EOF + + options = { + "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x", + "CONTENT_LENGTH" => data.length.to_s, + :input => StringIO.new(data) + } + env = Rack::MockRequest.env_for("/", options) + params = Rack::Multipart.parse_multipart(env) + params["inline"].must_equal 'true' + end + it "support mixed case metadata" do file = multipart_file(:text) data = File.open(file, 'rb') { |io| io.read } From e4242514473b8208e54d71925f3de6609d30f2fe Mon Sep 17 00:00:00 2001 From: Gary Grossman Date: Tue, 8 Sep 2015 15:27:07 -0700 Subject: [PATCH 2/2] When parsing the name parameter of Content-Disposition, support quoted chars in the quoted-string case. --- lib/rack/multipart.rb | 2 +- lib/rack/multipart/parser.rb | 7 +++++-- test/spec_multipart.rb | 20 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/rack/multipart.rb b/lib/rack/multipart.rb index cc2d6ebbf..db59ee59a 100644 --- a/lib/rack/multipart.rb +++ b/lib/rack/multipart.rb @@ -17,7 +17,7 @@ module Multipart BROKEN_QUOTED = /^#{CONDISP}.*;\sfilename="(.*?)"(?:\s*$|\s*;\s*#{TOKEN}=)/i BROKEN_UNQUOTED = /^#{CONDISP}.*;\sfilename=(#{TOKEN})/i MULTIPART_CONTENT_TYPE = /Content-Type: (.*)#{EOL}/ni - MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*\s+name=(#{TOKEN}|"[^\";]*")/ni + MULTIPART_CONTENT_DISPOSITION = /Content-Disposition:.*\s+name=(#{VALUE})/ni MULTIPART_CONTENT_ID = /Content-ID:\s*([^#{EOL}]*)/ni # Updated definitions from RFC 2231 ATTRIBUTE_CHAR = %r{[^ \t\v\n\r)(><@,;:\\"/\[\]?='*%]} diff --git a/lib/rack/multipart/parser.rb b/lib/rack/multipart/parser.rb index bc0162b4e..fe3381b9a 100644 --- a/lib/rack/multipart/parser.rb +++ b/lib/rack/multipart/parser.rb @@ -245,8 +245,11 @@ def handle_mime_head @buf.slice!(0, 2) # Second \r\n content_type = head[MULTIPART_CONTENT_TYPE, 1] - name = head[MULTIPART_CONTENT_DISPOSITION, 1] || head[MULTIPART_CONTENT_ID, 1] - name.gsub!(/\A"|"\Z/, '') if name + if name = head[MULTIPART_CONTENT_DISPOSITION, 1] + name = Rack::Auth::Digest::Params::dequote(name) + else + name = head[MULTIPART_CONTENT_ID, 1] + end filename = get_filename(head) diff --git a/test/spec_multipart.rb b/test/spec_multipart.rb index 7f7e68564..f7eca3005 100644 --- a/test/spec_multipart.rb +++ b/test/spec_multipart.rb @@ -626,6 +626,26 @@ def initialize(*) params["inline"].must_equal 'true' end + it "parse quoted chars in name parameter" do + data = <<-EOF +--AaB03x\r +Content-Type: text/plain\r +Content-Disposition: attachment; name="quoted\\\\chars\\"in\rname"\r +\r +true\r +--AaB03x--\r + EOF + + options = { + "CONTENT_TYPE" => "multipart/form-data; boundary=AaB03x", + "CONTENT_LENGTH" => data.length.to_s, + :input => StringIO.new(data) + } + env = Rack::MockRequest.env_for("/", options) + params = Rack::Multipart.parse_multipart(env) + params["quoted\\chars\"in\rname"].must_equal 'true' + end + it "support mixed case metadata" do file = multipart_file(:text) data = File.open(file, 'rb') { |io| io.read }