Skip to content

Close instead of draining when a plug never honors Expect: 100-continue - #695

Open
tomciopp wants to merge 1 commit into
mtrudel:mainfrom
tomciopp:fix/expect-continue-close-not-drain
Open

Close instead of draining when a plug never honors Expect: 100-continue#695
tomciopp wants to merge 1 commit into
mtrudel:mainfrom
tomciopp:fix/expect-continue-close-not-drain

Conversation

@tomciopp

Copy link
Copy Markdown
Contributor

What happens today

A client sends Expect: 100-continue and waits for the 100 before sending the
body. If the plug responds without ever reading the body (a 401, say), Bandit
never sends the 100, but after the response it still tries to drain the
request body to keep the connection alive. So the client is waiting on a 100
that will never come, and Bandit is waiting on a body that may never arrive.
The connection hangs until read_timeout, and a request whose response was
already delivered ends in a read-timeout error and an error log.

RFC9110§10.1.1 addresses exactly this case:

A server that responds with a final status code before reading the entire
request content SHOULD indicate whether it intends to close the connection
(e.g., see Section 9.6 of [HTTP/1.1]) or continue reading the request
content.

The change

When a final response is sent on HTTP/1.1 with the 100-continue expectation
still pending (and the plug didn't set its own connection header), add
connection: close. The existing keepalive handling does the rest: it signals
the close on the wire and skips the body drain.

An explicit inform(conn, 100, ...) now also clears the pending expectation,
since it unblocks the client the same way the implicit 100 does.

HTTP/2 needs no change: its ensure_completed already handles an unread body
without draining (RST_STREAM with NO_ERROR).

Test

POST with expect: 100-continue and a content-length but no body; the plug
401s without reading. Asserts the response carries connection: close, the
connection closes, and the request's telemetry stop event has no error.
Before the fix:

Assertion with == failed
code:  assert Keyword.get_values(headers, :connection) == ["close"]
left:  []
right: ["close"]

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.

1 participant