Skip to content

HTTPCLIENT-2433: Prevent concurrent HTTP/2 request body replay - #884

Merged
ok2c merged 1 commit into
apache:masterfrom
mkurz:HTTPCLIENT-2433
Sep 18, 2026
Merged

ok2c merged 1 commit into
apache:masterfrom
mkurz:HTTPCLIENT-2433

Conversation

@mkurz

@mkurz mkurz commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

This fixes HTTPCLIENT-2433.

When an HTTP/2 authentication challenge completed before the request body had finished, H2AsyncMainClientExec previously notified the execution chain that the exchange was complete immediately. The authentication handler could then release and reuse the same repeatable entity producer while the original HTTP/2 stream was still consuming it, corrupting the replayed request body.

The HTTP/2 execution handler now completes the exchange only after both request output and response input have terminated. It also handles a graceful RST_STREAM(NO_ERROR) after a complete response as request-side termination, allowing authentication replay to continue without hanging.

Regression coverage includes:

  • Uploading a deterministic 2 MB file through challenge-based Basic authentication and verifying that the replay contains the exact original bytes.
  • A body-less 401 followed by RST_STREAM(NO_ERROR), verifying that the authenticated retry completes with the exact 2 MB request body.

Reproducer:
https://github.com/mkurz/apache-httpclient5-h2-auth-replay-reproducer

Tests:

  • HTTP/2 authentication suites with and without TLS: 28 tests, 0 failures
  • Graceful-reset regression test repeatedly verified with and without TLS

@ok2c

ok2c commented Sep 17, 2026

Copy link
Copy Markdown
Member

@mkurz The test coverage looks awesome! However I need a little while to digest the proposed fix. Please bear with me.

@ok2c ok2c left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mkurz Your suggested fix is perfectly fine.

For the sake of code consistency could we use the same approach as used in HttpAsyncMainClientExec using AtomicInteger instead of two AtomicBoolean?

Wait for both sides of an HTTP/2 exchange to terminate before completing it, including graceful request termination after RST_STREAM(NO_ERROR).

Cover authentication replay with a large file and with a body-less 401 followed by a graceful stream reset.
@mkurz

mkurz commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@ok2c done!

@mkurz

mkurz commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@mkurz
mkurz requested a review from ok2c September 18, 2026 12:41
@ok2c
ok2c merged commit 3849cf9 into apache:master Sep 18, 2026
10 checks passed
@mkurz
mkurz deleted the HTTPCLIENT-2433 branch September 18, 2026 14:51
@ok2c

ok2c commented Sep 18, 2026

Copy link
Copy Markdown
Member

Cherry-picked to 5.6.x

@ok2c

ok2c commented Sep 18, 2026

Copy link
Copy Markdown
Member

@mkurz Many thanks for reporting the problem and contributing the fix

@mkurz

mkurz commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@ok2c thanks for merging them ;)

@mkurz

mkurz commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

@ok2c do you have an eta when next core and client patch release will be cut?

@ok2c

ok2c commented Sep 18, 2026

Copy link
Copy Markdown
Member

@mkurz Soon. I will try to cut the core releases this weekend. The client bug I see as less severe as it affects H2 only client, so it may take a bit longer.

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