Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/protocol/http1/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,9 @@ def close!(error = nil)
# @parameter body [Object] the body to write.
# @parameter head [Boolean] whether the request was a `HEAD` request.
# @parameter trailer [Hash | Nil] the trailers to write.
def write_body(version, body, head = false, trailer = nil)
# @parameter request [Boolean] whether the body belongs to a request, which cannot be delimited by closing the connection.
# @raises [ProtocolError] if a request body has unknown length and the version does not support chunked encoding.
def write_body(version, body, head = false, trailer = nil, request = false)
# HTTP/1.0 cannot in any case handle trailers.
if version == HTTP10 # or te: trailers was not present (strictly speaking not required.)
trailer = nil
Expand All @@ -815,7 +817,13 @@ def write_body(version, body, head = false, trailer = nil)
write_connection_header(version)
# We specifically ensure that non-persistent connections do not use chunked response, so that hijacking works as expected.
write_chunked_body(body, head, trailer)
elsif request
# A request body cannot be delimited by closing the connection, as the server would read it as the next request:
error = ProtocolError.new("Cannot write #{version} request body of unknown length!")
body.close(error)
raise error
else
# Only a response body may be delimited by closing the connection:
@persistent = false
write_connection_header(version)
write_body_and_close(body, head)
Expand Down
3 changes: 2 additions & 1 deletion lib/traces/provider/protocol/http1/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ def write_interim_response(version, status, headers, reason = nil)
end
end

def write_body(version, body, head = false, trailer = nil)
def write_body(version, body, head = false, trailer = nil, request = false)
attributes = {
version: version,
head: head,
trailer: trailer,
request: request,
body: body&.as_json,
}

Expand Down
4 changes: 4 additions & 0 deletions releases.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Releases

## Unreleased

- `Protocol::HTTP1::Connection#write_body` accepts a trailing `request` argument. When true, an HTTP/1.0 body of unknown length raises `ProtocolError` instead of being written close-delimited, which the server would read as the next request. Clients should pass it when writing request bodies.

## v0.41.0

- Map remote socket failures to `Protocol::HTTP::RemoteError` and prevent connection reuse.
Expand Down
76 changes: 76 additions & 0 deletions test/protocol/http1/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -632,9 +632,33 @@
expect(body).to receive(:empty?).and_return(false)
expect(body).to receive(:length).and_return(nil)

# Only a response body may be delimited by closing the connection:
expect(server).to receive(:write_body_and_close)
server.write_body("HTTP/1.0", body)
end

it "refuses to write closed body for a request" do
expect(body).to receive(:empty?).and_return(false)
expect(body).to receive(:length).and_return(nil)
expect(body).to receive(:close)

# A request body cannot be delimited by closing the connection, so nothing should be written:
expect do
server.write_body("HTTP/1.0", body, false, nil, true)
end.to raise_exception(Protocol::HTTP1::ProtocolError, message: be =~ /unknown length/)

expect(server).to be(:open?)
server.close
expect(client.stream.read).to be == ""
end

it "can write chunked body for a request" do
expect(body).to receive(:empty?).and_return(false)
expect(body).to receive(:length).and_return(nil)

expect(server).to receive(:write_chunked_body)
server.write_body("HTTP/1.1", body, false, nil, true)
end
end

with "bad requests" do
Expand Down Expand Up @@ -739,6 +763,58 @@
expect(client).to be(:closed?)
end

with "HTTP/1.0 request body" do
it "can write a request body of known length" do
client.write_request("localhost", "POST", "/", "HTTP/1.0", {"connection" => "keep-alive"})

body = Protocol::HTTP::Body::Buffered.new(["Hello World"])
client.write_body("HTTP/1.0", body, false, nil, true)
expect(client).to be(:half_closed_local?)

authority, method, path, version, headers, body = server.read_request
expect(method).to be == "POST"
expect(body).to be_a(Protocol::HTTP1::Body::Fixed)
expect(body.join).to be == "Hello World"
expect(server).to be(:persistent)
end

it "can write an empty request body of unknown length" do
client.write_request("localhost", "POST", "/", "HTTP/1.0", {})

body = Protocol::HTTP::Body::Writable.new
body.close_write

client.write_body("HTTP/1.0", body, false, nil, true)
expect(client).to be(:half_closed_local?)

authority, method, path, version, headers, body = server.read_request
expect(body).to be_nil
expect(server).to be(:persistent)
end

it "refuses to write a request body of unknown length" do
client.write_request("localhost", "POST", "/", "HTTP/1.0", {"connection" => "keep-alive"})

# Without a content length, the server would treat these bytes as the start of the next request:
body = Protocol::HTTP::Body::Writable.new
body.write "GET /other HTTP/1.1\r\nhost: localhost\r\n\r\n"
body.close_write

expect do
client.write_body("HTTP/1.0", body, false, nil, true)
end.to raise_exception(Protocol::HTTP1::ProtocolError, message: be =~ /unknown length/)

expect(body).to be(:closed?)

# The connection is still open, so the caller is responsible for closing it:
expect(client).to be(:open?)
client.close

# Only the request head was written; none of the body bytes reached the server:
expect(server.stream.read).to be == "POST / HTTP/1.0\r\nhost: localhost\r\nconnection: keep-alive\r\n"
end
end

it "raises RefusedError when the stream is broken" do
client.stream.close

Expand Down
Loading