Skip to content

Reset HTTP/2 stream if an error response ends before the request body - #943

Open
beshkenadze wants to merge 1 commit into
swift-server:mainfrom
beshkenadze:fix/h2-reset-stream-after-early-response
Open

Reset HTTP/2 stream if an error response ends before the request body#943
beshkenadze wants to merge 1 commit into
swift-server:mainfrom
beshkenadze:fix/h2-reset-stream-after-early-response

Conversation

@beshkenadze

@beshkenadze beshkenadze commented Oct 22, 2026

Copy link
Copy Markdown

Closes #940

Motivation

After an early HTTP/2 response with status >= 300, HTTPRequestStateMachine returns .forwardResponseEnd(.close, ...). Since #657, HTTP2ClientRequestHandler ignores .close. The stream stays half-closed (remote), and HTTPClient.shutdown() waits until the connection closes.

Modifications

HTTP2ClientRequestHandler sets requestEndSent when it writes the request END_STREAM.

requestEndSent Handler action on .close
false Closes the stream channel. NIOHTTP2 then sends RST_STREAM(CANCEL). END_STREAM is not correct, because the request body is not complete.
true Ignores .close, as before.

The handler schedules the close with eventLoop.assumeIsolated().execute, so the close runs after the current read. It runs only if the handler is still in the pipeline and the stream channel is active.

Reason: a server can send RST_STREAM(NO_ERROR) after a complete response (RFC 9113, section 8.1). If both come in one read:

  1. NIOHTTP2 processes the RST_STREAM and closes the stream.
  2. The stream channel gives the buffered response end to the handler and closes.
  3. An immediate close then answers RST_STREAM with RST_STREAM, which RFC 9113, section 5.4.2 forbids.

4 new tests in HTTP2ClientRequestHandlerTests each fail without their part of the change:

Test Checks Fails on This branch
testStreamIsClosedIfErrorResponseEndsBeforeRequestBodyIsSent Early 401: the stream channel closes. main Passes
testStreamIsResetIfErrorResponseEndsBeforeRequestBodyIsSent Early 401, client and server NIOHTTP2Handler: the client sends 1 RST_STREAM(CANCEL). main Passes
testStreamIsNotClosedIfResponseEndsAfterRequestEnd 401 after the request end: the handler does not close the stream channel. close without the requestEndSent check Passes
testStreamIsNotResetIfServerResetsStreamInSameReadAsErrorResponseEnd 401 and RST_STREAM(NO_ERROR) in one read: the client sends no RST_STREAM. immediate close Passes

Result

Case Result
Early response with status >= 300 The connection releases the stream, and shutdown() can complete.
Response after the request end, early response with status < 300, failure, cancellation No change.
Repro output, macOS 27.0.1 (arm64), Swift 6.4 main and 1.36.2, 4 runs each (2 eager, 2 lazy body) This branch, 4 runs (2 eager, 2 lazy body)
openH2Streams in the afterWait line, 30 s after the response 1, in 8 of 8 runs 0, in 4 of 4 runs
outcome in the shutdown line shutdown() does not complete in 10 s, in 8 of 8 runs. shutdown() completes in 0.001 s or less, in 4 of 4 runs (results/h2-reset-stream-fix.jsonl).

To repeat in the repro directory:

AHC_PATH=/absolute/path/to/this/branch ./run.sh

The README has the lazy body variant.

This PR does not depend on other PRs. #942 changes other parts of the same 2 files. Git merges the 2 branches in either order. With both, the 109 swift test --filter HTTP2 tests pass.

@beshkenadze
Reset HTTP/2 stream if an error response ends before the request body
2d9497f 
Motivation:

A server can send a complete response before the client sends the
whole request body. An example is a 401 response to an upload without
credentials. RFC 9113 section 8.1 permits this.

If the response status is >= 300, `HTTPRequestStateMachine` finishes
the request and returns `.forwardResponseEnd(.close, ...)`.
`HTTP2ClientRequestHandler` ignores `.close`, because it expects that
every h2 stream closes after the request. This is not true here. The
client stops the request body, but it never sends END_STREAM or
RST_STREAM. The stream stays half-closed (remote).

`HTTP2IdleHandler` gets no `StreamClosedEvent` for this stream. The
connection counts the stream as open, and `HTTPClient.shutdown()` does
not complete until the server closes the connection.

Modifications:

- `HTTP2ClientRequestHandler` sets the new `requestEndSent` flag when
  it writes the request END_STREAM. The flag tells the handler if the
  client side of the stream is still open.
- On `.close`, if `requestEndSent` is false, the handler closes the
  stream channel. NIOHTTP2 then sends RST_STREAM(CANCEL). The request
  body is not complete, so END_STREAM is not correct here.
- If `requestEndSent` is true, the handler ignores `.close` as before.
  The stream closes when both END_STREAM flags pass. A close at this
  point makes NIOHTTP2 send RST_STREAM on a closed stream.
- The handler closes the stream one event loop tick later. It closes
  the stream only if the handler is still in the pipeline and the
  stream channel is still active. A server can send the complete
  response and RST_STREAM(NO_ERROR) in the same read, as RFC 9113
  section 8.1 suggests. NIOHTTP2 gives the response end to the handler
  before it marks the stream as closed. An immediate close then sends
  RST_STREAM(CANCEL) as a reply to RST_STREAM. RFC 9113 section 5.4.2
  does not permit this. One tick later, NIOHTTP2 has closed the
  stream, and the handler does nothing.
- Add two `EmbeddedChannel` tests. The first test makes sure that the
  handler closes the stream after an early 401 response. The second
  test makes sure that a response after the request end does not
  close the stream. Both tests run the event loop before they examine
  the channel.
- Add two tests that connect a client and a server `NIOHTTP2Handler`
  through `EmbeddedChannel`s and record the RST_STREAM frames of the
  client. After an early 401 response, the client writes
  RST_STREAM(CANCEL). If the server sends RST_STREAM(NO_ERROR) in the
  same read as the 401 response, the client writes no RST_STREAM.

Result:

After an early response with status >= 300, the client resets the
stream. The connection releases the stream, and `HTTPClient.shutdown()`
can complete. If the server resets the stream in the same read, the
client does not reply with RST_STREAM. Responses after the request
end, early 2xx responses, failures and cancellation do not change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

HTTP/2 stream stays half-closed after an early response with status >= 300

1 participant

@beshkenadze