Skip to content

Commit f76da36

Browse files
committed
Only allow specific trailers
Only allow trailers listed in the `Trailer` header. Even if listed there, disallow the following names, based on Mozilla recommendations: ``` content-encoding content-type content-range trailer authorization set-cookie transfer-encoding content-length host cache-control max-forwards te ``` There are probably additional ones we should disallow, but this is a decent start. Do not merge the header and trailer data. Parse the trailers into a separate hash, and for allowed names, copy the value into the headers hash. This ignores invalid trailers instead of raising an exception, which is preferable for backwards compatibility. In order to get the new test to pass, make content_length return nil instead of of raising a TypeError if no content length was provided. Also, parse the content length as decimal instead of trying to autodetect the radix. Fixes #198
1 parent c3aeef0 commit f76da36

2 files changed

Lines changed: 60 additions & 9 deletions

File tree

lib/webrick/httprequest.rb

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,9 @@ def query
308308
# The content-length header
309309

310310
def content_length
311-
return Integer(self['content-length'])
311+
if length = self['content-length']
312+
Integer(length, 10)
313+
end
312314
end
313315

314316
##
@@ -474,10 +476,21 @@ def read_request_line(socket)
474476
end
475477
end
476478

479+
DISALLOWED_TRAILERS = %w[content-encoding content-type content-range
480+
trailer authorization set-cookie transfer-encoding content-length
481+
host cache-control max-forwards te].freeze
482+
private_constant :DISALLOWED_TRAILERS
483+
477484
def read_header(socket)
478485
if socket
479486
end_of_headers = false
480487

488+
raw = if @header
489+
trailer_lines = []
490+
else
491+
@raw_header
492+
end
493+
481494
while line = read_line(socket)
482495
if line == CRLF
483496
end_of_headers = true
@@ -489,19 +502,35 @@ def read_header(socket)
489502
if line.include?("\x00")
490503
raise HTTPStatus::BadRequest, 'null byte in header'
491504
end
492-
@raw_header << line
505+
raw << line
493506
end
494507

495508
# Allow if @header already set to support chunked trailers
496-
raise HTTPStatus::EOFError unless end_of_headers || @header
509+
raise HTTPStatus::EOFError unless end_of_headers || trailer_lines
497510
end
498-
@header = HTTPUtils::parse_header(@raw_header.join)
499511

500-
if (content_length = @header['content-length']) && content_length.length != 0
501-
if content_length.length > 1
502-
raise HTTPStatus::BadRequest, "multiple content-length request headers"
503-
elsif !/\A\d+\z/.match?(content_length[0])
504-
raise HTTPStatus::BadRequest, "invalid content-length request header"
512+
if trailer_lines
513+
if requested_trailers = self["trailer"]
514+
requested_trailers = requested_trailers.downcase.split
515+
requested_trailers -= DISALLOWED_TRAILERS
516+
unless requested_trailers.empty?
517+
trailers = HTTPUtils::parse_header(trailer_lines.join)
518+
requested_trailers.each do |key|
519+
if value = trailers[key]
520+
@header[key] = value
521+
end
522+
end
523+
end
524+
end
525+
else
526+
@header = HTTPUtils::parse_header(@raw_header.join)
527+
528+
if (content_length = @header['content-length']) && content_length.length != 0
529+
if content_length.length > 1
530+
raise HTTPStatus::BadRequest, "multiple content-length request headers"
531+
elsif !/\A\d+\z/.match?(content_length[0])
532+
raise HTTPStatus::BadRequest, "invalid content-length request header"
533+
end
505534
end
506535
end
507536
end

test/webrick/test_httprequest.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,28 @@ def test_bad_chunked
441441
end
442442
end
443443

444+
def test_chunked_trailers
445+
req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP)
446+
req.parse(StringIO.new(<<~HTTP))
447+
POST /path HTTP/1.1\r
448+
host: foo.example.com\r
449+
Transfer-Encoding: chunked\r
450+
Trailer: host Foo\r
451+
\r
452+
1\r
453+
2\r
454+
0\r
455+
Content-Length: 2\r
456+
Foo: a\r
457+
Host: bar.example.com\r
458+
\r
459+
HTTP
460+
assert_nil(req.content_length)
461+
assert_equal("2", req.body)
462+
assert_equal("a", req["foo"])
463+
assert_equal("foo.example.com", req["host"])
464+
end
465+
444466
def test_bad_chunked_extra_data
445467
msg = <<~HTTP
446468
POST /path HTTP/1.1\r

0 commit comments

Comments
 (0)