From 03ef44c31f1aed654696d049b70713fbc51f6dde Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 12:35:37 +1300 Subject: [PATCH 1/4] Keep received chunked trailers in the trailer section rather than merging them into the header section. --- lib/protocol/http1/body/chunked.rb | 4 ++++ releases.md | 4 ++++ test/protocol/http1/body/chunked.rb | 35 +++++++++++++++++++++++++++++ test/protocol/http1/trailer.rb | 19 ++++++++++++++++ 4 files changed, 62 insertions(+) diff --git a/lib/protocol/http1/body/chunked.rb b/lib/protocol/http1/body/chunked.rb index 2c8b19e..0ad044d 100644 --- a/lib/protocol/http1/body/chunked.rb +++ b/lib/protocol/http1/body/chunked.rb @@ -148,7 +148,11 @@ def as_json(...) private # Read the trailer from the connection, and add any headers to the trailer. + # + # The trailer section is marked before any fields are added, so that `headers.header` and `headers.trailer` remain distinct. A recipient must not merge trailer fields into the header section (RFC 9110, Section 6.5.2). If the trailer section was already marked, this is a no-op. def read_trailer + @headers.trailer! + while line = @connection.read_line # Empty line indicates end of trailer: break if line.empty? diff --git a/releases.md b/releases.md index db77727..24cf828 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - Keep received chunked trailers in the trailer section of the headers, rather than merging them into the header section. + ## v0.41.0 - Map remote socket failures to `Protocol::HTTP::RemoteError` and prevent connection reuse. diff --git a/test/protocol/http1/body/chunked.rb b/test/protocol/http1/body/chunked.rb index 3d4580f..0914f9e 100644 --- a/test/protocol/http1/body/chunked.rb +++ b/test/protocol/http1/body/chunked.rb @@ -126,6 +126,41 @@ expect(connection).to be(:half_closed_remote?) end + + it "does not merge trailers into the header section" do + headers.add("trailer", "etag") + + expect(headers).not.to be(:trailer?) + + body.join + + expect(headers).to be(:trailer?) + expect(headers.header.to_a).to be == [["trailer", "etag"]] + expect(headers.trailer.to_a).to be == [["ETag", "abcd"]] + end + + it "preserves the trailer section if it was already marked" do + headers.add("trailer", "etag") + headers.trailer! + + body.join + + expect(headers.header.to_a).to be == [["trailer", "etag"]] + expect(headers.trailer.to_a).to be == [["ETag", "abcd"]] + end + end + + with "trailer not announced in header section" do + let(:postfix) {"x-forwarded-host: evil\r\n"} + + it "does not merge trailers into the header section" do + headers.add("etag", "abcd") + + body.join + + expect(headers.header.to_a).to be == [["etag", "abcd"]] + expect(headers.trailer.to_a).to be == [["x-forwarded-host", "evil"]] + end end with "incomplete trailer" do diff --git a/test/protocol/http1/trailer.rb b/test/protocol/http1/trailer.rb index ad08c4d..c080646 100644 --- a/test/protocol/http1/trailer.rb +++ b/test/protocol/http1/trailer.rb @@ -51,5 +51,24 @@ # Headers are updated: expect(headers).to be == {"foo" => ["bar"]} end + + it "keeps received trailers separate from the header section" do + expect(body).to receive(:length).and_return(nil) + trailer["foo"] = "bar" + + server.write_response("HTTP/1.1", 200, {"etag" => "abcd"}) + server.write_body("HTTP/1.1", body, false, trailer) + + version, status, reason, headers, body = client.read_response("GET") + + expect(headers).not.to be(:trailer?) + expect(headers.header.to_a).to be == [["etag", "abcd"]] + + body.join + + expect(headers).to be(:trailer?) + expect(headers.header.to_a).to be == [["etag", "abcd"]] + expect(headers.trailer.to_a).to be == [["foo", "bar"]] + end end end From db4c64ecda3c71230d75ffbc8f993796e7f62fa1 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:10:26 +1300 Subject: [PATCH 2/4] Only mark the trailer section when a trailer field is actually received. --- lib/protocol/http1/body/chunked.rb | 6 +++--- releases.md | 2 +- test/protocol/http1/body/chunked.rb | 30 +++++++++++++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/protocol/http1/body/chunked.rb b/lib/protocol/http1/body/chunked.rb index 0ad044d..975d6c9 100644 --- a/lib/protocol/http1/body/chunked.rb +++ b/lib/protocol/http1/body/chunked.rb @@ -149,15 +149,15 @@ def as_json(...) # Read the trailer from the connection, and add any headers to the trailer. # - # The trailer section is marked before any fields are added, so that `headers.header` and `headers.trailer` remain distinct. A recipient must not merge trailer fields into the header section (RFC 9110, Section 6.5.2). If the trailer section was already marked, this is a no-op. + # The trailer section is marked before the first field is added, so that `headers.header` and `headers.trailer` remain distinct. A recipient must not merge trailer fields into the header section (RFC 9110, Section 6.5.2). If no trailer fields are received, which is the common case, the headers are left untouched, so an empty trailer section does not change `headers.trailer?` and does not fail on frozen headers. If the trailer section was already marked, marking it again is a no-op. def read_trailer - @headers.trailer! - while line = @connection.read_line # Empty line indicates end of trailer: break if line.empty? if match = line.match(HEADER) + # Mark the start of the trailer section (no-op if already marked): + @headers.trailer! @headers.add(match[1], match[2], trailer: true) else raise BadHeader, "Could not parse header: #{line.inspect}" diff --git a/releases.md b/releases.md index 24cf828..5f97747 100644 --- a/releases.md +++ b/releases.md @@ -2,7 +2,7 @@ ## Unreleased - - Keep received chunked trailers in the trailer section of the headers, rather than merging them into the header section. + - Keep received chunked trailers in the trailer section of the headers, rather than merging them into the header section. The trailer section is only marked when at least one trailer field is received, so messages with an empty trailer section leave the headers unchanged. ## v0.41.0 diff --git a/test/protocol/http1/body/chunked.rb b/test/protocol/http1/body/chunked.rb index 0914f9e..4db573b 100644 --- a/test/protocol/http1/body/chunked.rb +++ b/test/protocol/http1/body/chunked.rb @@ -112,6 +112,36 @@ expect{body.read}.to raise_exception(Protocol::HTTP1::BadRequest) end + with "empty trailer section" do + it "does not mark the trailer section" do + headers.add("etag", "abcd") + + body.join + + expect(headers).not.to be(:trailer?) + expect(headers.header.to_a).to be == [["etag", "abcd"]] + expect(headers.trailer.to_a).to be == [] + end + + it "does not modify frozen headers" do + headers.add("etag", "abcd") + headers.freeze + + expect(body.join).to be == "Hello World" + + expect(headers).not.to be(:trailer?) + expect(connection).to be(:half_closed_remote?) + end + + it "allows headers to be added after the body is read" do + body.join + + headers.add("content-type", "text/plain") + + expect(headers.header.to_a).to be == [["content-type", "text/plain"]] + end + end + with "trailer" do let(:postfix) {"ETag: abcd\r\n"} From 0d63905eaa3631d8c652b5af7b29cf34d1cb8ee0 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:10:26 +1300 Subject: [PATCH 3/4] Add a trailer field before reading the body so the already-marked test can distinguish re-marking from preserving the trailer section. --- test/protocol/http1/body/chunked.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/protocol/http1/body/chunked.rb b/test/protocol/http1/body/chunked.rb index 4db573b..98baa31 100644 --- a/test/protocol/http1/body/chunked.rb +++ b/test/protocol/http1/body/chunked.rb @@ -172,11 +172,13 @@ it "preserves the trailer section if it was already marked" do headers.add("trailer", "etag") headers.trailer! + headers.add("x-sent", "1", trailer: true) body.join + # The existing trailer field is retained, so the trailer section was not re-marked: expect(headers.header.to_a).to be == [["trailer", "etag"]] - expect(headers.trailer.to_a).to be == [["ETag", "abcd"]] + expect(headers.trailer.to_a).to be == [["x-sent", "1"], ["ETag", "abcd"]] end end From 2781094b040018476b8006a16f32d69f01515899 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 4 Nov 2026 13:10:57 +1300 Subject: [PATCH 4/4] Add request-side tests for received chunked trailers using `Connection#read_request`. --- test/protocol/http1/trailer.rb | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/protocol/http1/trailer.rb b/test/protocol/http1/trailer.rb index c080646..ec45732 100644 --- a/test/protocol/http1/trailer.rb +++ b/test/protocol/http1/trailer.rb @@ -71,4 +71,41 @@ expect(headers.trailer.to_a).to be == [["foo", "bar"]] end end + + with "request trailers" do + it "keeps received request trailers separate from the header section" do + expect(body).to receive(:length).and_return(nil) + trailer["x-forwarded-host"] = "evil" + + client.write_request("localhost", "POST", "/", "HTTP/1.1", {"content-type" => "text/plain"}) + client.write_body("HTTP/1.1", body, false, trailer) + + authority, method, target, version, headers, body = server.read_request + + expect(method).to be == "POST" + expect(headers).not.to be(:trailer?) + expect(headers.header.to_a).to be == [["content-type", "text/plain"]] + + expect(body.join).to be == "HelloWorld" + + # Request trailers are controlled by the client, so they must not appear in the header section: + expect(headers).to be(:trailer?) + expect(headers.header.to_a).to be == [["content-type", "text/plain"]] + expect(headers.trailer.to_a).to be == [["x-forwarded-host", "evil"]] + end + + it "does not mark the trailer section when no request trailers are received" do + expect(body).to receive(:length).and_return(nil) + + client.write_request("localhost", "POST", "/", "HTTP/1.1", {"content-type" => "text/plain"}) + client.write_body("HTTP/1.1", body) + + authority, method, target, version, headers, body = server.read_request + + expect(body.join).to be == "HelloWorld" + + expect(headers).not.to be(:trailer?) + expect(headers.header.to_a).to be == [["content-type", "text/plain"]] + end + end end