From ee801d62223e482f9ef89f81d64743cdbe79183d Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 5 Jul 2024 10:36:09 -0700 Subject: [PATCH 1/2] Only strip space and horizontal tab in headers Previously, all whitespace was stripped, but that goes against the related RFCs. Fixes #139 --- lib/webrick/httputils.rb | 19 +++++++++++-------- test/webrick/test_httprequest.rb | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/lib/webrick/httputils.rb b/lib/webrick/httputils.rb index ea67fdb..92f3044 100644 --- a/lib/webrick/httputils.rb +++ b/lib/webrick/httputils.rb @@ -178,12 +178,12 @@ def parse_header(raw) field.downcase! header[field] = HEADER_CLASSES[field].new unless header.has_key?(field) header[field] << value - when /^\s+([^\r\n\0]*?)\r\n/om + when /^[ \t]+([^\r\n\0]*?)\r\n/om unless field raise HTTPStatus::BadRequest, "bad header '#{line}'." end value = line - value.lstrip! + value.gsub!(/\A[ \t]+/, '') value.slice!(-2..-1) header[field][-1] << " " << value else @@ -191,7 +191,10 @@ def parse_header(raw) end } header.each{|key, values| - values.each(&:strip!) + values.each{|value| + value.gsub!(/\A[ \t]+/, '') + value.gsub!(/[ \t]+\z/, '') + } } header end @@ -202,7 +205,7 @@ def parse_header(raw) def split_header_value(str) str.scan(%r'\G((?:"(?:\\.|[^"])+?"|[^",]++)+) - (?:,\s*|\Z)'xn).flatten + (?:,[ \t]*|\Z)'xn).flatten end module_function :split_header_value @@ -230,9 +233,9 @@ def parse_range_header(ranges_specifier) def parse_qvalues(value) tmp = [] if value - parts = value.split(/,\s*/) + parts = value.split(/,[ \t]*/) parts.each {|part| - if m = %r{^([^\s,]+?)(?:;\s*q=(\d+(?:\.\d+)?))?$}.match(part) + if m = %r{^([^ \t,]+?)(?:;[ \t]*q=(\d+(?:\.\d+)?))?$}.match(part) val = m[1] q = (m[2] or 1).to_f tmp.push([val, q]) @@ -331,8 +334,8 @@ def <<(str) elsif str == CRLF @header = HTTPUtils::parse_header(@raw_header.join) if cd = self['content-disposition'] - if /\s+name="(.*?)"/ =~ cd then @name = $1 end - if /\s+filename="(.*?)"/ =~ cd then @filename = $1 end + if /[ \t]+name="(.*?)"/ =~ cd then @name = $1 end + if /[ \t]+filename="(.*?)"/ =~ cd then @filename = $1 end end else @raw_header << str diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 6088f18..ad00d60 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -118,6 +118,27 @@ def test_bare_lf_header } end + def test_header_vt_ff_whitespace + msg = <<~HTTP + GET / HTTP/1.1\r + Foo: \x0b1\x0c\r + \r + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + assert_equal("\x0b1\x0c", req["Foo"]) + + msg = <<~HTTP + GET / HTTP/1.1\r + Foo: \x0b1\x0c\r + \x0b2\x0c\r + \r + HTTP + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + assert_equal("\x0b1\x0c \x0b2\x0c", req["Foo"]) + end + def test_bare_cr_request_line msg = <<~HTTP.gsub("\n", "\r\n") GET / HTTP/1.1\r\r From 3cbbacfc8bfcf926376c79a9445d142a75501d1c Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Thu, 11 Jul 2024 15:02:21 -0700 Subject: [PATCH 2/2] Remove unnecessary gsub calls in test_httprequest.rb --- test/webrick/test_httprequest.rb | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index ad00d60..84faefc 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -89,7 +89,7 @@ def test_invalid_content_length_header HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {8}/, "").gsub("\n", "\r\n"))) + req.parse(StringIO.new(msg)) } end end @@ -102,7 +102,7 @@ def test_bare_lf_request_line HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -114,7 +114,7 @@ def test_bare_lf_header HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -125,7 +125,7 @@ def test_header_vt_ff_whitespace \r HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) assert_equal("\x0b1\x0c", req["Foo"]) msg = <<~HTTP @@ -135,7 +135,7 @@ def test_header_vt_ff_whitespace \r HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) assert_equal("\x0b1\x0c \x0b2\x0c", req["Foo"]) end @@ -147,7 +147,7 @@ def test_bare_cr_request_line HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -159,7 +159,7 @@ def test_bare_cr_header HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -171,7 +171,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } msg = <<~HTTP.gsub("\n", "\r\n") @@ -181,7 +181,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } msg = <<~HTTP.gsub("\n", "\r\n") @@ -191,7 +191,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } msg = <<~HTTP.gsub("\n", "\r\n") @@ -201,7 +201,7 @@ def test_invalid_request_lines HTTP req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ - req.parse(StringIO.new(msg.gsub(/^ {6}/, ""))) + req.parse(StringIO.new(msg)) } end @@ -427,7 +427,6 @@ def test_null_byte_in_header Evil: evil\x00\r \r HTTP - msg.gsub!(/^ {6}/, "") req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) assert_raise(WEBrick::HTTPStatus::BadRequest){ req.parse(StringIO.new(msg)) } end