Prevent connection reuse when writing a chunked or fixed length body fails.

Author: samuel-williams-shopify

lib/protocol/http1/connection.rb

@@ -710,6 +710,9 @@ def write_fixed_length_body(body, length, head)
 					end
 				end
 			rescue => error
+				# The body may have been partially written, so the connection can't be reused, even if the caller swallows the error:
+				@persistent = false
+				
 				raise
 			ensure
 				# Close the body after the response is fully flushed, so that completion callbacks run after the client has received the response:
@@ -760,6 +763,9 @@ def write_chunked_body(body, head, trailer = nil)
 					flush_stream
 				end
 			rescue => error
+				# The body may have been partially written, so the connection can't be reused, even if the caller swallows the error:
+				@persistent = false
+				
 				raise
 			ensure
 				# Close the body after the complete chunked response (including terminal chunk) is flushed, so that completion callbacks don't block the client from seeing the response as complete:

releases.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
   - Validate trailer fields in `write_chunked_body` against the header policy, so framing fields such as `content-length` and `transfer-encoding` are rejected with `Protocol::HTTP::InvalidTrailerError` rather than written to the wire, regardless of the trailer collection type.
+  - Mark the connection as non-persistent when `write_chunked_body` or `write_fixed_length_body` fails, so a connection with a partially written body is never reused, even if the caller swallows the error.
 
 ## v0.41.0
 

test/protocol/http1/connection.rb

@@ -523,6 +523,20 @@
 				# The valid field preceding the invalid one must not have been written either:
 				expect(written).to be == "transfer-encoding: chunked\r\n\r\n5\r\nHello\r\n5\r\nWorld\r\n"
 			end
+			
+			it "prevents the connection from being reused" do
+				# As a server which has finished reading the request:
+				server.receive_end_stream!
+				expect(server).to be(:half_closed_remote?)
+				
+				expect do
+					server.write_chunked_body(body, false, {"content-length" => "5"})
+				end.to raise_exception(Protocol::HTTP::InvalidTrailerError)
+				
+				# The body was partially written, so even if the error is swallowed, the connection must not return to the idle state:
+				expect(server.persistent).to be == false
+				expect(server).to be(:closed?)
+			end
 		end
 
 		with "HEAD request" do
@@ -564,6 +578,9 @@
 				expect do
 					server.write_fixed_length_body(body, 100, false)
 				end.to raise_exception(Protocol::HTTP1::ContentLengthError)
+				
+				# The body was partially written, so the connection can't be reused:
+				expect(server.persistent).to be == false
 			end
 		end
 
@@ -572,6 +589,9 @@
 				expect do
 					server.write_fixed_length_body(body, 1, false)
 				end.to raise_exception(Protocol::HTTP1::ContentLengthError)
+				
+				# The body was partially written, so the connection can't be reused:
+				expect(server.persistent).to be == false
 			end
 		end