Skip to content

Fix HTTP/1 pipelining and streaming request state - #511

Open
ericmj wants to merge 8 commits into
mainfrom
http1-pipelining
Open

ericmj wants to merge 8 commits into
mainfrom
http1-pipelining

Conversation

@ericmj

@ericmj ericmj commented Sep 24, 2026

Copy link
Copy Markdown
Member

Fixes to how Mint.HTTP1 matches pipelined responses to requests and tracks request state. Each commit is one fix with its own tests, so this is meant to be rebase-merged rather than squashed.

  • Parse pipelined responses after bodiless responses. A response with no body (HEAD, 204, 304, 2xx to CONNECT) left the rest of the buffer untouched, so a response that followed it in the same message wasn't delivered until more data arrived.
  • Treat data after the last in-flight response as an error. Leftover bytes stayed in the buffer and were delivered as the response to the next request. They now return {:unexpected_data, data} and close the connection, the same as when they arrive in a separate message. Bytes after a 2xx response to CONNECT still stay buffered for the tunnel.
  • Parse responses that arrive while a request body is streaming. A :stream request was only queued on :eof, so a 100 Continue or an early 413 closed the connection with {:unexpected_data, _}. The request is now queued when its headers are sent.
  • Fail requests pipelined behind a response that closes the connection. They stayed queued with no response. Behind a Connection: close response they now get {:error, ref, %HTTPError{reason: :unprocessed}}, like HTTP/2 streams the server didn't process before a GOAWAY, since the server must not process requests after it (RFC 9112 §9.6). Behind an HTTP/1.0 response without keep-alive, or a close-delimited response once the server closes the connection, they get {:error, ref, %TransportError{reason: :closed}}, because the server might have processed them and a non-idempotent request mustn't be retried automatically (RFC 9112 §9.3.1).
  • Stop parsing after a response that closes the connection. Bytes after it in the same message were decoded as the next queued request's response, so the request got a response the server sent after closing, or a status followed by an error.
  • Return errors from stream_request_body/3 for requests that aren't streaming. A ref that isn't streaming, has already sent :eof, or is unknown raised FunctionClauseError. It now returns :request_is_not_streaming or :unknown_request_to_stream, and :closed as an HTTPError on a closed connection (previously a TransportError from the socket write), matching HTTP/2.

Every commit's tests were run without its fix applied and fail there.

Responses without a body (HEAD, 204, 304, and 2xx to CONNECT) left the
rest of the buffer untouched, so a pipelined response arriving in the same
message was not delivered until further data arrived on the socket.
Bytes left over once the response queue was empty were kept in the
connection buffer and delivered as the response to whichever request was
issued next, while the real response to that request was buffered in
turn. Return {:unexpected_data, data} and close the connection instead,
the same as when such bytes arrive in a separate message. Bytes after a
2xx response to CONNECT still stay buffered, since they belong to the
tunnel.
A request with a :stream body was only added to the response queue when
the caller sent :eof, so a response the server sent before that, such as
100 Continue or an early 413, hit handle_data/2 with no request in flight
and closed the connection with {:unexpected_data, _}. Enqueue the request
when its headers are sent and track only the ref and body encoding in
streaming_request.
…ection

A response with "Connection: close", or an HTTP/1.0 response without
"keep-alive", closes the connection once it completes. Requests already
pipelined behind it stayed queued and never got a response or an error,
so a caller waiting on them only found out by checking open?/1. They
now fail with an :unprocessed error, matching what HTTP/2 reports for
streams the server didn't process before a GOAWAY.
When a response has neither Content-Length nor Transfer-Encoding its body
ends when the server closes the connection. Requests that were pipelined
behind such a response got no response at all: the close produced only
the {:done, ref} for the in-flight request and the queued requests stayed
in the connection with open_request_count above zero. They now get the
same {:error, ref, :unprocessed} that queued requests behind a
"Connection: close" response get.
… not streaming

stream_request_body/3 only had clauses for the request currently
streaming its body, so calling it with any other request ref, with a
ref whose body already ended with :eof, or with an unknown ref raised
FunctionClauseError instead of returning an error tuple. It now returns
:closed on a closed connection, :request_is_not_streaming for a request
of this connection that is not streaming, and :unknown_request_to_stream
for a ref that doesn't belong to the connection, matching HTTP/2.
@coveralls

coveralls commented Sep 24, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 1

Coverage increased (+0.04%) to 88.771%

Details

  • Coverage increased (+0.04%) from the base build.
  • Patch coverage: 3 uncovered changes across 1 file (48 of 51 lines covered, 94.12%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
lib/mint/http1.ex 51 48 94.12%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
lib/mint/http1.ex 1 86.44%

Coverage Stats

Coverage Status
Relevant Lines: 1790
Covered Lines: 1589
Line Coverage: 88.77%
Coverage Strength: 556.71 hits per line

💛 - Coveralls

After a response with "Connection: close" (or an HTTP/1.0 response
without "keep-alive") the connection was closed, but bytes that followed
it in the same message were still decoded as the response to the next
queued request. A complete one was delivered as that request's response,
and a partial one produced a status for the request followed by an
:unprocessed error. The server sends nothing after such a response
(RFC 9112 section 9.6), so the remaining bytes are now dropped and the
queued requests fail.
… :closed

Requests pipelined behind a response that closes the connection all got
an :unprocessed error, which says they're safe to retry. That only holds
for a "Connection: close" response, since the server must not process
any request after it (RFC 9112 section 9.6). A server that answers with
HTTP/1.0 without "keep-alive", or delimits the body by closing the
connection, might have processed the requests behind it already, and a
non-idempotent one mustn't be retried automatically (RFC 9112 section
9.3.1). Those requests now get a TransportError with reason :closed.
@ericmj
ericmj marked this pull request as ready for review September 24, 2026 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants