Better support for trailers.

Author: samuel-williams-shopify

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)

lib/protocol/http1/version.rb

@@ -5,6 +5,6 @@
 
 module Protocol
 	module HTTP1
-		VERSION = "0.37.0"
+		VERSION = "0.38.0"
 	end
 end

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.

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