From e60f6a9d62d3c408bcf42aa2885ccb7224595780 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Tue, 10 Mar 2026 19:50:04 +1300 Subject: [PATCH] Better support for trailers. --- lib/protocol/http1/connection.rb | 6 +-- lib/protocol/http1/version.rb | 2 +- releases.md | 4 ++ test/protocol/http1/trailer.rb | 63 +++++++++++++++++++++++++++++++- 4 files changed, 69 insertions(+), 6 deletions(-) diff --git a/lib/protocol/http1/connection.rb b/lib/protocol/http1/connection.rb index 845c2b4..c73ab8b 100644 --- a/lib/protocol/http1/connection.rb +++ b/lib/protocol/http1/connection.rb @@ -755,15 +755,15 @@ def write_body(version, body, head = false, trailer = nil) # While writing the body, we don't know if trailers will be added. We must choose a different body format depending on whether there is the chance of trailers, even if trailer.any? is currently false. # - # Below you notice `and trailer.nil?`. I tried this but content-length is more important than trailers. + # When trailers are present, we must use chunked encoding (RFC 7230) since Content-Length cannot coexist with trailers. The trailer.nil? checks ensure we only use fixed-length or empty body when no trailers will be sent. if body.nil? write_connection_header(version) write_empty_body(body) - elsif length = body.length # and trailer.nil? + elsif length = body.length and trailer.nil? write_connection_header(version) write_fixed_length_body(body, length, head) - elsif body.empty? + elsif body.empty? and trailer.nil? # Even thought this code is the same as the first clause `body.nil?`, HEAD responses have an empty body but still carry a content length. `write_fixed_length_body` takes care of this appropriately. write_connection_header(version) write_empty_body(body) diff --git a/lib/protocol/http1/version.rb b/lib/protocol/http1/version.rb index 68d05dd..5906e5f 100644 --- a/lib/protocol/http1/version.rb +++ b/lib/protocol/http1/version.rb @@ -5,6 +5,6 @@ module Protocol module HTTP1 - VERSION = "0.37.0" + VERSION = "0.38.0" end end diff --git a/releases.md b/releases.md index 7755873..ddf15ac 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - Use chunked encoding when trailers are present, even when body has known length or is empty. Previously, `write_body` would use `write_fixed_length_body` or `write_empty_body` when the body had a length, silently dropping trailers. Per RFC 7230, trailers require chunked transfer encoding since `content-length` cannot coexist with trailers. + ## v0.37.0 - `Protocol::HTTP1::BadRequest` now includes `Protocol::HTTP::BadRequest` for better interoperability and handling of bad request errors across different HTTP protocol implementations. diff --git a/test/protocol/http1/trailer.rb b/test/protocol/http1/trailer.rb index ad08c4d..9f2345f 100644 --- a/test/protocol/http1/trailer.rb +++ b/test/protocol/http1/trailer.rb @@ -27,11 +27,16 @@ server.write_body("HTTP/1.0", body, false, trailer) end - it "ignores trailers with content length" do - expect(server).to receive(:write_fixed_length_body) + it "uses chunked encoding when trailers are present even with content length" do + expect(server).to receive(:write_chunked_body).with(body, false, trailer) server.write_body("HTTP/1.1", body, false, trailer) end + it "uses fixed length when no trailers" do + expect(server).to receive(:write_fixed_length_body) + server.write_body("HTTP/1.1", body, false, nil) + end + it "uses chunked encoding when given trailers without content length" do expect(body).to receive(:length).and_return(nil) trailer["foo"] = "bar" @@ -51,5 +56,59 @@ # Headers are updated: expect(headers).to be == {"foo" => ["bar"]} end + + it "uses chunked encoding when given trailers with empty body" do + empty_body = Protocol::HTTP::Body::Buffered.new + trailer["grpc-status"] = "2" + + expect(server).to receive(:write_chunked_body).with(empty_body, false, trailer) + server.write_body("HTTP/1.1", empty_body, false, trailer) + end + + it "sends trailers with empty body (round-trip)" do + empty_body = Protocol::HTTP::Body::Buffered.new + trailer["grpc-status"] = "2" + + server.write_response("HTTP/1.1", 200, {}) + server.write_body("HTTP/1.1", empty_body, false, trailer) + + version, status, reason, headers, body = client.read_response("GET") + + expect(version).to be == "HTTP/1.1" + expect(status).to be == 200 + expect(headers).to be == {} + + # Read all of the response body, including trailers: + body.join + + # Headers are updated: + expect(headers).to be == {"grpc-status" => ["2"]} + end + + it "uses chunked encoding when given trailers with known body length" do + trailer["grpc-status"] = "0" + + expect(server).to receive(:write_chunked_body).with(body, false, trailer) + server.write_body("HTTP/1.1", body, false, trailer) + end + + it "sends trailers with known body length (round-trip)" do + trailer["grpc-status"] = "0" + + server.write_response("HTTP/1.1", 200, {}) + server.write_body("HTTP/1.1", body, false, trailer) + + version, status, reason, headers, body = client.read_response("GET") + + expect(version).to be == "HTTP/1.1" + expect(status).to be == 200 + expect(headers).to be == {} + + # Read all of the response body, including trailers: + body.join + + # Headers are updated: + expect(headers).to be == {"grpc-status" => ["0"]} + end end end