Conversation
* Migrate tests to JUnit 5 * Use Amazon Corretto JDK 11
* Add MacOS Workflow and IO_URING Transport
* Automatically set the number of IoThreads * Retry tests if failed
This was referenced Aug 25, 2026
hyperxpro
added a commit
that referenced
this pull request
Aug 31, 2026
## Summary - Add a thread-safe `ResponseBodyControl` callback after final response headers. - Support suspending, resuming, and cancelling HTTP/1.1 and HTTP/2 response bodies without coupling AHC to a streaming API. - Pause the network read timeout while reads are intentionally suspended while leaving the request timeout active. - Keep suspended HTTP/2 streams independent and cover transport backpressure, cancellation, timeouts, and connection reuse. ## Motivation AHC 3 removed `StreamedAsyncHandler` and its Reactive Streams integration in pull request [#1843](#1843). That removal avoids coupling AHC to a particular streaming library, but `AsyncHandler` by itself has no way to stop transport reads while a downstream consumer has no demand. [Play WS](https://github.com/playframework/play-ws) is the driving consumer for this change. Play WS needs transport backpressure to preserve its existing Pekko Streams and Reactive Streams response APIs while upgrading to AHC 3. A working adapter on a currently local Play WS development branch turns this control into a single-subscriber Reactive Streams publisher and has been tested against this AHC branch. This pull request adds only the transport primitive. Streaming-library policy and dependencies remain in Play WS, so AHC does not regain a dependency on Reactive Streams or JDK Flow. ## Semantics - `AsyncHandler.onResponseBodyStart` runs after final headers and before body parts, including for a response with no body. - Calls to the supplied control are thread-safe, idempotent, and ignored after response completion. - `suspend()` stops requesting new transport data, although body parts already read may still be delivered. - `resume()` permits transport reads again. - Returning `State.ABORT` is the synchronous callback-time way to stop processing. A handler can retain the control and call `cancel()` when an asynchronous decision is made after the callback returns. - Cancelling an HTTP/1.1 body closes its connection when bytes may remain unread; cancellation after terminal content has been received can reuse a keep-alive connection. Cancelling an HTTP/2 body closes only its stream. - Fully consumed responses retain the existing HTTP/1.1 pooling and HTTP/2 parent-connection reuse behavior. - Suspension pauses only the network read timeout. The request timeout remains active; if the request timeout is disabled, an application that never resumes or cancels can retain the exchange and its transport resources indefinitely. ## HTTP/2 flow control Connections with no actively suspended response retain Netty's normal connection-level receive-window accounting. The default 65,535-byte connection window therefore continues to cap unconsumed flow-controlled DATA across all streams for users that never call `suspend()`. When the first response on a connection is suspended, AHC returns connection-level credit that has already accumulated and continues returning that shared credit as DATA arrives. This prevents the suspended stream from exhausting the connection window and starving sibling streams. Per-stream credit remains consumption-driven at all times, so each suspended stream is still bounded by its own receive window. Multiple simultaneous suspensions are counted, and normal connection accounting resumes after the last one ends. Credit already returned and data already queued cannot be revoked, and the controller tracks credit returned early so later application consumption does not return it twice. During an active suspension, aggregate buffering can still scale with the number of concurrent streams; the relevant controls are `http2InitialWindowSize`, `http2MaxConcurrentStreams`, and the connection limits. The defaults do not impose a hard client-side aggregate bound during that interval: the initial per-stream window is 16 MiB and `http2MaxConcurrentStreams = -1` leaves concurrency server-controlled. A rough upper-bound estimate is connection count times effective concurrent streams times the initial window, excluding network and decoder overhead. Applications requiring a finite policy must configure these values together. A hard aggregate byte budget is a separate design and is outside this pull request. Netty's connection auto-refill state is private and fixed when `DefaultHttp2LocalFlowController` is constructed, so it cannot be enabled only for the lifetime of a suspension through composition or subclassing. The package-private `SuspensionAwareHttp2LocalFlowController` therefore adapts `DefaultHttp2LocalFlowController` from Netty 4.2.17.Final, preserving its normal behavior while making connection refill suspension-scoped. This adaptation has a maintenance cost: AHC owns the copied flow-control logic and every Netty upgrade must compare it with the corresponding upstream implementation for correctness and security fixes. The exact source version and that obligation are recorded in the class Javadoc. An upstream Netty API that permits connection auto-refill to be changed at runtime would provide the exit path and allow AHC to remove the adaptation; no such API exists in Netty 4.2.17.Final. The auto-refill mode requires a custom `Http2Connection`. Netty's builder treats `server()` and `connection()` as mutually exclusive, so `ClientHttp2FrameCodecBuilder` supplies the connection through the protected builder API and overrides `isServer()` to retain client mode. The explicit `gracefulShutdownTimeoutMillis(0)` is not a new shutdown policy. `Http2FrameCodecBuilder.forClient()` selects zero through its package-private client constructor; the subclass must use the protected no-argument constructor, so it sets zero explicitly to preserve the existing client-factory behavior. ## Scope and commit structure The API/lifecycle work and the HTTP/2 independence work are kept as separate logical commits, but they belong in one pull request. Without the HTTP/2 work, a suspended response can consume the shared connection window and block unrelated sibling streams, so the public control would not have correct multiplexed behavior. Follow-up review fixes are also split into focused commits covering exchange ownership, terminal cleanup, terminal HTTP/1.1 cancellation, bodyless responses, interim responses, indefinite-suspension diagnostics, and suspension-scoped HTTP/2 refill. ## History checked - Issue [#544](#544) originally identified the lack of `AsyncHandler` backpressure, and pull request [#963](#963) addressed it by adding Reactive Streams support. - Issues [#1233](#1233) and [#1721](#1721) document the interaction between downstream demand and read timeouts in the former streamed handler. - Pull request [#1843](#1843) removed `StreamedAsyncHandler` for AHC 3. - Discussion [#1925](#1925) asks how to migrate streamed consumers to AHC 3; the maintainer response declines restoring Reactive Streams because other libraries provide that policy and maintaining it adds overhead. - Focused GitHub issue, pull-request, discussion, and local history searches found no existing AHC 3 proposal for a streaming-library-neutral suspend/resume/cancel response-body control. ## Compatibility - `AsyncHandler.onResponseBodyStart` is a new Java `default` method, so existing handler implementations remain source- and binary-compatible and retain their previous behavior unless they override it. - `ResponseBodyControl` is a new public interface. - `NettyResponseBodyControl` is public only to support AHC's cross-package transport integration and is marked `@ApiStatus.Internal`; consumers should depend on `ResponseBodyControl` instead. - HTTP/1.1 informational responses from 102 through 199 are now treated as interim and no longer reach `onStatusReceived` or `onHeadersReceived`; this matches the existing HTTP/2 behavior and prevents a 103 Early Hints response from completing the exchange before the final response. Existing 100 Continue handling and 101 protocol switching are preserved. - Cancelling HTTP/1.1 processing from a terminal trailer or body callback now reuses a keep-alive connection because `LastHttpContent` has already been received; it previously closed that fully read connection. - The full JDK 11 verification, including Revapi, passes. ## AI disclosure OpenAI Codex on behalf of Matthias Kurz. The commits include `Co-Authored-By: OpenAI Codex <codex@openai.com>` per `AGENTS.md`. ## Test plan - [x] On the first commit alone, the new HTTP/2 sibling-stream test reproduced shared connection-window starvation: a sibling response did not progress while the first response remained suspended. - [x] The focused `ResponseBodyControlTest`, `Http2ResponseBodyControlTest`, `NettyResponseFutureTest`, `TimeoutTimerTaskTest`, `SuspensionAwareHttp2LocalFlowControllerTest`, and `Continue100InterceptorTest` suites pass 38 tests: 12 HTTP/1.1 control tests, 6 HTTP/2 integration tests, 9 future-lifecycle tests, 4 timeout-task tests, 5 flow-controller accounting tests, and 2 HTTP/1.1 Continue tests. - [x] The flow-controller tests cover unchanged connection accounting without suspension, connection-only refill during suspension, DATA received before the suspension callback, overlapping suspensions, and avoiding double credit after normal accounting resumes. - [x] `JAVA_HOME=<jdk-11> ./mvnw clean verify`: BUILD SUCCESS for the full reactor, including tests, Javadocs, coverage, and Revapi. - [x] The locally published AHC snapshot passes the full Play WS Scala 2.13 and Scala 3.3.8 test matrices on Java 17 and Java 21: 183 integration tests and 76 unit tests pass in each combination, with 2 expected pending tests. - [x] Play WS code validation, documentation, Scala 2 and Scala 3 MiMa checks, and dependency-tree verification pass against the locally published AHC branch. --------- Co-authored-by: OpenAI Codex <codex@openai.com> Co-authored-by: Aayush Atharva <24762260+hyperxpro@users.noreply.github.com>
hyperxpro
added a commit
that referenced
this pull request
Aug 31, 2026
## Summary - Build keep-body redirects from the original request, preserving every supported body representation and per-request setting. - Clear target-specific routing and credential state when a redirect crosses an origin, including separately stored Cookie objects. - Preserve an explicit `Content-Length` when replaying a raw `InputStream` or an unknown-length `InputStreamBodyGenerator`. - Replay resettable streams and fail promptly for consumed raw streams, streamed multipart parts, and vanished files that cannot be replayed safely. - Pin body bytes, headers, body-selection precedence, caller-owned `ByteBuf` references, and the new failure modes with focused tests. ## Problem `Redirect30xInterceptor` rebuilds a request when it follows a strict 302, 307, or 308 redirect. Its keep-body copy chain handled form parameters, strings, byte arrays, `ByteBuffer`, body generators, and multipart bodies, but omitted four real request send paths: - `List<byte[]>` / composite byte arrays - Netty `ByteBuf` - `InputStream` - `File` The redirected request therefore kept its method and Content-Type but sent zero bytes. The `File` case is especially risky for uploads because the target can accept an apparently valid empty PUT or POST. Reconstructing the request field by field also omitted unrelated per-request state such as the read timeout and range offset, and maintaining a second body-selection chain alongside `NettyRequestFactory` made future drift likely. This is a pre-existing omission. AHC issue [#1643](#1643) previously fixed the same class of bug for multipart bodies. The copy chain was carried through pull request [#1843](#1843) without a policy discussion. Focused searches found no existing issue or pull request covering these four representations. ## Change Build a keep-body redirect with `request.toBuilder()` and then replace only redirect-specific state. This preserves all current and future body representations and per-request options without duplicating `NettyRequestFactory.body`. Headers are copied before redirect-only values are removed, so the original request is not mutated. On a cross-origin redirect, the copied request drops the previous resolved address, virtual host, realm, authorization headers, and Cookie objects before the cookie store adds cookies that legitimately match the new URI. The body is not covered by that boundary. It follows the existing keep-body policy, which means a `File` or `InputStream` body that a cross-origin redirect leg previously received as empty is now sent in full, and a target that keeps redirecting can receive it once per hop up to `maxRedirects`. That is the same exposure byte arrays, strings, form parameters, and multipart bodies already have today. Composite byte arrays, caller-owned `ByteBuf`s, and files are repeatable. A resettable `InputStream`, such as `ByteArrayInputStream`, also replays. A caller-supplied `Content-Length` is retained for a raw `InputStream` or an `InputStreamBodyGenerator` without a declared length, because neither has an intrinsic size from which to recompute it. A consumed stream that cannot be reset reaches the existing fail-fast guard added in #2312 and completes the future with `IOException`; that is preferable to silently succeeding with an empty body. An `InputStreamPart` is closed by the first multipart send and has no equivalent replay guard, so a keep-body redirect now fails promptly instead of risking a hang or incomplete multipart request. A selected `File` or `FileBodyGenerator` is also checked before dispatching the redirect; if it disappeared after the first send, the future fails with `IOException` before a target pooled channel can be removed and an unchecked constructor exception can escape. The validation follows `NettyRequestFactory` precedence so a sticky `File` field is ignored when a higher-priority body representation was actually sent. The change does not alter which methods or status codes keep a body, nor does it introduce a new cross-origin policy. It makes the existing strict-302, 307, and 308 behavior complete for every supported request-body representation. ## Compatibility There is no public API change. Requests that previously sent an empty body on a keep-body redirect now resend their configured body. **Behavior changes:** - A non-resettable `InputStream` on a keep-body redirect previously completed successfully after sending an empty redirected request. It now completes the request future exceptionally with `IOException`. This includes `FileInputStream`, which is closed after the first send and cannot be reset for replay. - A multipart `InputStreamPart` now fails promptly with `IOException` when a keep-body redirect requires replay. Reusing its already-consumed and closed stream could previously hang or send incomplete multipart content. - A selected file that disappears between the first request and redirect now fails with `IOException` before redirect dispatch rather than allowing an unchecked `IllegalArgumentException` to escape while constructing the next request. - A `File` or `InputStream` body is now sent on a keep-body redirect to a different origin, where the redirected request previously carried no body. Credentials are still stripped at that boundary, but the payload is not. Callers that accidentally relied on an empty or incomplete redirected request will observe an exception, but the failure is explicit instead of silently losing configured content. There is no public API change. ## AI disclosure OpenAI Codex on behalf of Matthias Kurz. The commit includes `Co-Authored-By: OpenAI Codex <codex@openai.com>` per `AGENTS.md`. ## Test plan - [x] On untouched `upstream/main`, the focused suite reproduced five failures: four body types arrived as zero bytes and a non-resettable stream incorrectly completed successfully. - [x] Before the generator fix, a one-argument `InputStreamBodyGenerator` sent `Content-Length: 13` on the first leg and no `Content-Length` on the redirected leg. - [x] `./mvnw -pl client -Dtest=RedirectBodyTest,RedirectCredentialSecurityTest test` on JDK 11: 40 tests passed, including Netty leak detection. - [x] `./mvnw clean verify` on JDK 11: 1,496 tests passed and Revapi completed without failures (`BUILD SUCCESS`). Generated with OpenAI Codex. --------- Co-authored-by: OpenAI Codex <codex@openai.com> Co-authored-by: Aayush Atharva <24762260+hyperxpro@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.