Keep received chunked trailers in the trailer section rather than merging them into the header section.

lib/protocol/http1/body/chunked.rb

@@ -148,12 +148,16 @@ def as_json(...)
 				private
 
 				# Read the trailer from the connection, and add any headers to the trailer.
+				#
+				# 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
 					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}"

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. 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
 
   - Map remote socket failures to `Protocol::HTTP::RemoteError` and prevent connection reuse.

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"}
 
@@ -126,6 +156,43 @@
 
 				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!
+				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 == [["x-sent", "1"], ["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

test/protocol/http1/trailer.rb

@@ -51,5 +51,61 @@
 			# 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
+
+	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