From f76da3614deb09bf8c9f8de9eab2c054b043b9eb Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 24 Mar 2026 20:26:47 -0700 Subject: [PATCH] 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 --- lib/webrick/httprequest.rb | 47 ++++++++++++++++++++++++++------ test/webrick/test_httprequest.rb | 22 +++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/lib/webrick/httprequest.rb b/lib/webrick/httprequest.rb index c6efaf5..e283608 100644 --- a/lib/webrick/httprequest.rb +++ b/lib/webrick/httprequest.rb @@ -308,7 +308,9 @@ def query # The content-length header def content_length - return Integer(self['content-length']) + if length = self['content-length'] + Integer(length, 10) + end end ## @@ -474,10 +476,21 @@ def read_request_line(socket) end end + DISALLOWED_TRAILERS = %w[content-encoding content-type content-range + trailer authorization set-cookie transfer-encoding content-length + host cache-control max-forwards te].freeze + private_constant :DISALLOWED_TRAILERS + def read_header(socket) if socket end_of_headers = false + raw = if @header + trailer_lines = [] + else + @raw_header + end + while line = read_line(socket) if line == CRLF end_of_headers = true @@ -489,19 +502,35 @@ def read_header(socket) if line.include?("\x00") raise HTTPStatus::BadRequest, 'null byte in header' end - @raw_header << line + raw << line end # Allow if @header already set to support chunked trailers - raise HTTPStatus::EOFError unless end_of_headers || @header + raise HTTPStatus::EOFError unless end_of_headers || trailer_lines end - @header = HTTPUtils::parse_header(@raw_header.join) - if (content_length = @header['content-length']) && content_length.length != 0 - if content_length.length > 1 - raise HTTPStatus::BadRequest, "multiple content-length request headers" - elsif !/\A\d+\z/.match?(content_length[0]) - raise HTTPStatus::BadRequest, "invalid content-length request header" + if trailer_lines + if requested_trailers = self["trailer"] + requested_trailers = requested_trailers.downcase.split + requested_trailers -= DISALLOWED_TRAILERS + unless requested_trailers.empty? + trailers = HTTPUtils::parse_header(trailer_lines.join) + requested_trailers.each do |key| + if value = trailers[key] + @header[key] = value + end + end + end + end + else + @header = HTTPUtils::parse_header(@raw_header.join) + + if (content_length = @header['content-length']) && content_length.length != 0 + if content_length.length > 1 + raise HTTPStatus::BadRequest, "multiple content-length request headers" + elsif !/\A\d+\z/.match?(content_length[0]) + raise HTTPStatus::BadRequest, "invalid content-length request header" + end end end end diff --git a/test/webrick/test_httprequest.rb b/test/webrick/test_httprequest.rb index 3d02573..e84f614 100644 --- a/test/webrick/test_httprequest.rb +++ b/test/webrick/test_httprequest.rb @@ -441,6 +441,28 @@ def test_bad_chunked end end + def test_chunked_trailers + req = WEBrick::HTTPRequest.new(WEBrick::Config::HTTP) + req.parse(StringIO.new(<<~HTTP)) + POST /path HTTP/1.1\r + host: foo.example.com\r + Transfer-Encoding: chunked\r + Trailer: host Foo\r + \r + 1\r + 2\r + 0\r + Content-Length: 2\r + Foo: a\r + Host: bar.example.com\r + \r + HTTP + assert_nil(req.content_length) + assert_equal("2", req.body) + assert_equal("a", req["foo"]) + assert_equal("foo.example.com", req["host"]) + end + def test_bad_chunked_extra_data msg = <<~HTTP POST /path HTTP/1.1\r