diff --git a/async-http.gemspec b/async-http.gemspec index 62ea379b..861da016 100644 --- a/async-http.gemspec +++ b/async-http.gemspec @@ -31,7 +31,7 @@ Gem::Specification.new do |spec| spec.add_dependency "io-endpoint", "~> 0.18" spec.add_dependency "io-stream", "~> 0.14" spec.add_dependency "protocol-http", "~> 0.72" - spec.add_dependency "protocol-http1", "~> 0.41" + spec.add_dependency "protocol-http1", "~> 0.42" spec.add_dependency "protocol-http2", "~> 0.28" spec.add_dependency "protocol-url", "~> 0.2" end diff --git a/lib/async/http/protocol/http1/client.rb b/lib/async/http/protocol/http1/client.rb index 105a2ca8..50743ace 100644 --- a/lib/async/http/protocol/http1/client.rb +++ b/lib/async/http/protocol/http1/client.rb @@ -69,7 +69,7 @@ def call(request, task: Task.current) else task.async(annotation: "Streaming request...") do # Once we start writing the body, we can't recover if the request fails. That's because the body might be generated dynamically, streaming, etc. - write_body(@version, body, false, trailer) + write_body(@version, body, false, trailer, true) rescue => error self.close(error) end @@ -77,7 +77,7 @@ def call(request, task: Task.current) elsif protocol = request.protocol write_upgrade_body(protocol) else - write_body(@version, request.body, false, trailer) + write_body(@version, request.body, false, trailer, true) end return Response.read(self, request) diff --git a/releases.md b/releases.md index f56fb22f..41f7f95a 100644 --- a/releases.md +++ b/releases.md @@ -1,5 +1,9 @@ # Releases +## Unreleased + + - `Async::HTTP::Protocol::HTTP1::Client` passes `request = true` to `Protocol::HTTP1::Connection#write_body` so an HTTP/1.0 request body of unknown length raises instead of being written close-delimited, where the server would parse it as the next request (requires `protocol-http1` >= 0.42). + ## v0.105.0 - Respond with `400 Bad Request` for any `Protocol::HTTP::BadRequest` raised while parsing HTTP/1 or HTTP/2 requests, include the exception class name without reflecting request data, and avoid reporting them as unhandled server errors. diff --git a/test/async/http/protocol/http10.rb b/test/async/http/protocol/http10.rb index 6770e699..2a58a897 100644 --- a/test/async/http/protocol/http10.rb +++ b/test/async/http/protocol/http10.rb @@ -6,6 +6,45 @@ require "async/http/protocol/http10" require "async/http/a_protocol" +require "protocol/http/body/writable" + describe Async::HTTP::Protocol::HTTP10 do it_behaves_like Async::HTTP::AProtocol + + with "request body of unknown length" do + include Sus::Fixtures::Async::HTTP::ServerContext + + let(:protocol) {subject} + + let(:paths) {[]} + + let(:app) do + ::Protocol::HTTP::Middleware.for do |request| + paths << request.path + + ::Protocol::HTTP::Response[200, {}, [request.path]] + end + end + + it "fails rather than writing the body where the server expects the next request" do + # HTTP/1.0 has no chunked encoding, and a request body cannot be delimited by closing the connection, so without a content length the server would parse these bytes as a second request: + body = ::Protocol::HTTP::Body::Writable.new + body.write "GET /smuggled HTTP/1.1\r\nhost: localhost\r\n\r\n" + body.close_write + + response = nil + + # The body is written in a child task which closes the connection on failure, so the error is observed when reading the response: + expect do + response = client.post("/", {"connection" => "keep-alive"}, body) + end.to raise_exception(::Protocol::HTTP1::ProtocolError, message: be =~ /closed/) + + expect(body).to be(:closed?) + + # The server never saw a complete request, let alone a second one: + expect(paths).to be(:empty?) + ensure + response&.close + end + end end