fix(netty): stop inflating a suspended HTTP/1.1 response by mkurz · Pull Request #2362 · AsyncHttpClient/async-http-client

mkurz · 2026-10-07T08:15:17Z

Summary

While an HTTP/1.1 response is suspended through ResponseBodyControl, stop decompressing its body, for the content codings AHC decompresses: gzip, deflate, br, zstd and snappy. Body parts are at most 64 KiB, so a handler that suspends after each part receives one part per resume().
Rebuilt on the inflate loop of Inflate HTTP/1.1 gzip and deflate without an EmbeddedChannel #2357. Http1ContentDecompressor keeps its superclass and constructor, and also implements ChannelOutboundHandler to see read requests. Revapi needs no entries.
br, zstd and snappy are decompressed on demand too, with Netty's pull-based Decompressor, in a class of their own. Only these codings use that API, which Netty added in 4.2.17; without it they go to Netty's own decoders, as before.
A body that is being drained is no longer decompressed, and a failed decoder result on the last chunk is kept, for these content codings.
HTTP/2 decompression is unchanged; see below. The replay fix that used to be part of this pull request is now fix(netty): detach a replayed request's old channel before completing its body control #2367.

Problem

Suspending a response stops further socket reads, but Netty's HttpContentDecompressor, and the inflate loop that replaces it for gzip and deflate since #2357, decompress every chunk completely as soon as it is read. So one socket read of a highly compressible body still became many parts that nobody had asked for.

For example, one 64 KiB read of gzipped zeros (about 1032:1) becomes some 67 MB of body parts for a handler that has suspended the response. Only maxDecompressedResponseSize (256 MiB by default) bounds it.

With 64 MiB of gzipped zeros and a handler that suspends inside each body callback, main before #2357 delivered 1,955,380 bytes in 30 more parts past the suspending part, and then up to 33,713,376 bytes per resume. This branch without its suspension check delivers the same 1,955,380 bytes in 30 parts.

AHC 2 behaved the same way, so this is hardening, not a regression fix. It matters to consumers that bound their buffering through ResponseBodyControl, such as Play WS's streaming API.

Change

gzip and deflate, on #2357's loop:

Before each part, the handler checks whether the response is suspended. Parts are cut where JdkZlibDecoder cuts them, but each inflate() call is capped at what is left of a 64 KiB part, so no part is larger.
While suspended, the input the inflater has not used yet stays in the handler, together with every message that arrives after it. The chunk is retained, not copied, since the inflater may still point into it; a direct buffer stays allocated until the inflater has used it up. When the output buffer filled up, the inflater may also hold output without any input left; that is remembered, and handed on after the resume.
On resume, the handler intercepts the read request that resume() issues. It hands on what was held back first, and lets the read reach the socket only if the response is not suspended again. This works the same with auto-read on and off. While paused, channelReadComplete does not request a read.
Without suspension, each part goes straight through, as before.

br, zstd and snappy: PullDecompression decompresses them one part at a time with Netty's Decompressor, under the same checks. Small outputs, such as snappy's frames, are merged into a part sized for them. Outputs larger than 64 KiB, such as zstd's when io.netty.compression.defaultMaxForwardBytes is raised, are split into retained slices, so the bound applies to delivered parts, not to allocations. Only this class refers to Netty's Decompressor, and it is loaded only once one of these codings shows up. Without the API, which Netty added in 4.2.17, these codings go to HttpContentDecompressor's own decoders, as before. So does zstd when io.netty.compression.defaultMaxForwardBytes is 0 or less, where Netty's pull decompressor hands out no output.

For all five content codings:

End of the body: only parts with content wait for a resume. Once the body has been decompressed in full, its end, with any trailers, is handed on even to a suspended response, as for an unencoded body. A consumer whose demand ended with the last part still learns that the body is complete.
Connection close: if the connection closes while input is held back, the rest is decompressed and delivered before channelInactive, even to a suspended response, because Netty tears the pipeline down with the channel. Suspension no longer bounds that output; only maxDecompressedResponseSize does.
Bodies nobody reads: once the exchange has ended (cancel, ABORT, timeout), or while the channel is drained (after a redirect, an authentication challenge or a replay), the rest of the body is dropped instead of decompressed. Its end still reaches a drain callback.
The last chunk's decoder result: HttpObjectDecoder reports a chunk it cannot parse as a LastHttpContent with a failed result. The end of the body now keeps that result, so the request fails instead of completing with a truncated body. A transfer coding such as Transfer-Encoding: gzip still goes through Netty's decoder, which drops it, as on main.

A suspension made from another thread takes effect when its queued task runs. Until then, the event loop can keep reading and decompressing, as before.

Why HTTP/2 decompression is unchanged

After END_STREAM, Netty's HTTP/2 stream channel hands every queued frame over on the next read, regardless of autoRead, and then closes. So a handler in the stream pipeline cannot hold compressed input back, and would have to inflate up to a whole stream window at once (16 MiB by default). That is exactly the decompression-bomb case, so a partial fix would be misleading. It needs either a Netty change (honor demand after END_STREAM and close only when drained), proposed in netty/netty#17774, or AHC delivering the body independently of the stream channel.

Behavior changes

Suspended HTTP/1.1 responses with one of these content codings receive no further part with content until they resume. A handler that suspends after each part receives one part of at most 64 KiB per resume. The empty last part, trailers and completion still arrive while suspended once the body has been decompressed in full.
Connection close while suspended: what the decompressor still holds is delivered at once, bounded only by maxDecompressedResponseSize, not by demand. This applies to any close that finds input held back, not only to bodies delimited by the close.
Drains: a body with one of these content codings is no longer decompressed while it is drained. Before, a body beyond maxDecompressedResponseSize failed the drain, closed its connection and could fail the request that followed on the same future, for example when a ResponseFilter replays it. Redirects and 401 or 407 challenges with a compressed body usually close the connection instead of draining it, because the decompressed response looks chunked to them, as on main.
Parts of these codings are at most 64 KiB. With AHC's default chunk size of 8 KiB, gzip and deflate parts were 64 KiB already; a larger chunk could inflate into a larger part before. For br, zstd and snappy, small outputs are now merged and large ones split, so their part counts differ from Netty's decoders.
Framing errors on the final chunk of a body in one of these content codings, such as a bad chunk size, now fail the request. Before, it completed with a truncated body. A transfer coding is unchanged.
br, zstd and snappy wait for demand with Netty 4.2.17 or later. With an older Netty, they are decompressed by Netty's own decoders as soon as they are read, as before. gzip and deflate do not use the API.
Unchanged: a transfer coding (Transfer-Encoding: gzip) is still decoded by Netty's HttpContentDecompressor, as on main: as soon as it is read, also while suspended or drained, and in Netty's parts. ResponseBodyControl.suspend() says so. After a failure, such as the size limit, what is left of the body is dropped, its end included, as on main.

Compatibility

API: Http1ContentDecompressor keeps its superclass, HttpContentDecompressor, and its constructor. It now also implements ChannelOutboundHandler, and gains the public methods of that interface, which pass every outbound event through and only look at read requests. Revapi reports no problem.
Netty: AHC uses Netty 4.2.19. Only br, zstd and snappy use Netty's Decompressor API, added in 4.2.17, and fall back to Netty's own decoders without it. Netty still marks it as unstable (@UnstableApi) while it migrates its codecs to it (netty/netty#16743), so a future Netty release may require changes to PullDecompression. The README says what the version buys.
Performance: not measured again for this revision. Without suspension, gzip and deflate run Inflate HTTP/1.1 gzip and deflate without an EmbeddedChannel #2357's loop plus the checks before each part; br, zstd and snappy use Netty's pull decompressors instead of its stream decoders.

Follow-ups

At the end of the body, check that the compressed stream is complete, so a body cut off in the middle of a gzip member, or missing its checksum, fails instead of completing. For gzip and deflate that means the inflater's state and the gzip trailer, for br, zstd and snappy the pull decoder's endOfInput(). That changes behavior main has, so it gets its own pull request.
Output that is still held when the connection closes, and its HTTP/2 counterpart: the output of the final DATA frame once the stream channel closes (HTTP/2: Respect read limits for frames queued at end of stream netty/netty#17774 is a prerequisite for HTTP/2). They are related, but may need different fixes.

Review fixes

Four commits on top, one per review finding, each with a test that fails without it: inflate buffers are capped at the part size, so a large codec chunk no longer gives every 64 KiB part a buffer twice the chunk's size; br, zstd and snappy fall back to Netty's own decoders when the Decompressor API is missing instead of failing; zstd is left to Netty at a forwarding threshold of 0, where its pull decompressor hands out nothing and the loop never ended; and the README and Javadoc say when these codings wait for demand.

AI disclosure

Claude Code on behalf of Matthias Kurz. The commits include Co-Authored-By per AGENTS.md.

Test plan

New and changed tests:

Http1DecompressionSuspensionTest (10 tests, live server): the bound with auto-read on and off, connection reuse, a chunked body with trailers, a body ended by connection close, cancel, ABORT, HEAD and empty gzip responses, a replay while input is held back with auto-read on and off, and a ResponseFilter replay of a body that inflates far beyond the limit, whose drained connection must go back to the pool.
Http1ContentDecompressorSuspensionTest (18 tests, EmbeddedChannel):
- one part per read, suspension before the body, and the end of a fully inflated body without another resume, also for a body without bytes;
- close while input is held, removal releasing it, and the size limit on the resume path;
- suspension across a gzip member boundary, and every coding with fragmented input and a resume per part, with auto-read on and off;
- a direct and a pooled direct chunk paused mid-way: held while paused (refCnt 1 after the decoder has returned), body correct after resuming, released at the end;
- a held direct chunk released on removal, on close and on failure;
- output the inflater still holds without input: raw deflate bodies of 64 KiB plus a few bytes, of which 13 sizes pause in exactly that state, checked through package-private test accessors;
- a drained or discarded channel: the body is not inflated, its end is handed on.
Http1ContentDecompressorTest (Inflate HTTP/1.1 gzip and deflate without an EmbeddedChannel #2357's, 34 tests): its comparisons with Netty now expect more parts where Netty hands on a part larger than 64 KiB, which happens in 3 of its scenarios; bodies, errors, the end of the response and releases are compared as before, and every part must be at most 64 KiB. New: a failed decoder result on the last chunk; br, zstd and snappy decoded like Netty's decoders, with every split of Inflate HTTP/1.1 gzip and deflate without an EmbeddedChannel #2357's helper, from heap and direct buffers; the size limit for those codings; merged parts sized for their content; read requests with auto-read off compared with Netty's for those codings too; and a class loader that hides Netty's Decompressor API, under which gzip and deflate decode and a zstd response fails with a LinkageError. The test that mixes this handler's messages with Netty's in one read now uses a transfer coding, since snappy is no longer left to Netty.
Http1ContentDecompressorForwardThresholdTest (1 test): in a fresh JVM with -Dio.netty.compression.defaultMaxForwardBytes=0, which hands on the output of every inflate step, a 200,000-byte gzip body is decoded at once and with a suspension after each part. The parent waits at most 60 seconds and kills the child.
Http1ContentDecompressorLargeOutputTest (1 test): in a fresh JVM with -Dio.netty.compression.defaultMaxForwardBytes=262144, checks that zstd output really exceeds 64 KiB, that parts stay within 64 KiB, one per resume, and that a tracking allocator sees every buffer released, also when the handler is removed while it holds the rest of a split output.

Broken variants, each run against these test classes:

Without the suspension check: 16 failures and 2 errors, including both bound tests, which report the 1,955,380 bytes in 30 parts from above.
Read requests passed straight through: 15 failures and 2 errors, including the chunked-body test and both replay tests.
Held input not handed on when the connection closes: 2 failures, the close-delimited body test and the channelInactive ordering test.
Parts not capped at 64 KiB: 3 failures, the comparisons with Netty where a large chunk inflates at once.
Each inflate call capped by the forwarding threshold instead of the part size: the zero-threshold test hits its 60-second limit, as the inflater is asked for 0 bytes forever.
Held input kept as a slice without retaining it: 18 failures and 3 errors, including both direct-buffer tests.
Output still held by the inflater not restored on resume: 1 failure, the held-output test, which loses the last byte of a 65,537-byte body.
A drained body inflated anyway: 3 failures, the ResponseFilter replay test and both unit variants.
A failed decoder result dropped: 1 failure, the last-chunk test.

Runs:

Each commit compiles with its tests.
./mvnw -B -ntp -Dgpg.skip=true clean verify on JDK 11: 1,856 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used.
The test classes above, ResponseBodyControlTest and Http2ResponseBodyControlTest, compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's -Djvm): 83 tests, 0 failures on each. The fresh-JVM test's child runs on the same JDK.
All open AHC changes merged together (this branch, ResponseBodyControl.execute, raw Cookie headers on retries, suspend/resume order): they merge without conflicts, in every pair, and ./mvnw -B -ntp -Dgpg.skip=true clean install on JDK 11 passes with 1,903 tests, 0 failures, 0 errors, 11 skipped; Revapi passed.
Play WS against that merged build, Scala 2.13 and 3.3, JDK 17: all tests pass. Its gzip streaming test demands one part early from another thread and finds 0 bytes delivered beyond demand, against 1,875,916 bytes with AHC 3.0.14. The installed AHC jar is byte-identical to the one built, and Play WS's shaded jar contains PullDecompression.

mkurz · 2026-10-07T09:39:04Z

Heads-up: #2357, opened before this PR, which I missed, rewrites the same Http1ContentDecompressor for a different goal: no per-response EmbeddedChannel and Inflater, and no copy of direct input. The two conflict, but the goals are complementary. #2357 still inflates a suspended response in full, and this PR doesn't pool inflaters or avoid the copy.

If you'd like #2357 first, I'd rebuild the demand bounding on top of its inflate loop instead of Netty's pull Decompressor. That drops this PR's dependency on Netty's @UnstableApi for gzip and deflate, and keeps the pull decompressors only for br, zstd and snappy. Happy to coordinate with @pavel-ptashyts either way.

mkurz · 2026-10-07T09:58:40Z

Related to "Why HTTP/2 is unchanged" above: netty/netty#17774 makes Http2StreamChannel respect auto-read and the read limits for frames queued at END_STREAM. Once that is in a Netty release AHC uses, an HTTP/2 counterpart of this change becomes possible; output from the final DATA frame would still need handling in AHC.

mkurz · 2026-10-22T10:41:52Z

Thanks for the review. I've pushed four commits on top, one per fix, and updated the description: the Compatibility section now states that Netty 4.2.17 is required, and Behavior changes covers the replay fix, which also changes HTTP/2. The endOfInput() check, and output left over when the connection closes, are follow-ups. Replies are inline.

mkurz · 2026-10-23T16:52:27Z

With #2357 now approved, would you prefer to land it first? As @pavel-ptashyts suggested there, I'm happy to rebuild the demand bounding here on top of its inflate loop, keeping its pooling and direct-input optimizations and the fixes from this review. I can prepare that locally in the meantime.

hyperxpro · 2026-11-03T17:46:00Z

@mkurz

Thanks, the four fixes look right. I re-ran my replay probe against b5db977 and it passes with auto-read on and off.

#2357 is merged now, so let's go with your plan and rebuild on its inflate loop. Two asks.

Split 5604fea with the HTTP/2 test into its own PR first. It fixes main and does not depend on either decompressor, I'll merge it right away.
Then force-push the rebuild here and carry over the drain discard, the part sizing, the docs and the failed DecoderResult on the last chunk, which the new loop drops like Netty does. Watch the point @pavel-ptashyts raised, a pause mid chunk has to retain the input the inflater still points at, a direct buffer test for that would be good. If br, zstd and snappy keep the pull decompressors, put them in their own class so the 4.2.17 floor only applies when one of those codings shows up.

Your other three PRs probably need a rebase after #2357 too, worth a look while you are at it.

… its body control (#2367) ## Summary - When a request is replayed, for example by an `IOExceptionFilter`, detach the old channel from the future before completing the old response's body control. - Fixes two bugs on `main`: on HTTP/2, the handler that has just been told to retry receives frames of the abandoned response; on HTTP/1.1 with auto-read off, the old connection is never drained or closed. - Split out of #2362 at the maintainer's request, since it does not depend on either decompressor. ## Problem `NettyRequestSender.replayRequest` completed the old response's `ResponseBodyControl` first, and only then detached the old channel from the future. - **HTTP/2:** completing the control resumes a suspended stream channel, and the stream channel delivers the DATA frames queued in it on the spot. The channel still named the future, so the handler that had just received `onRetry` got parts of the abandoned response before the replayed response's status. - **HTTP/1.1 with auto-read off:** `replayRequest` installs a drain callback that waits for the end of the old response, but after the replay nothing read the old channel. The drain never ran, and the connection was never drained or closed. ## Change - **HTTP/2:** mark the stream channel as discarded before completing the control, then close it as before. - **HTTP/1.1:** install the drain callback before completing the control, and when auto-read is off, issue one read so the drain starts. With auto-read on, HTTP/1.1 on `main` delivers nothing of the old response after `onRetry`, because NIO reads are asynchronous; the new order also holds when the old response holds back input that is delivered on the spot, which #2362 introduces. ## AI disclosure Claude Code on behalf of Matthias Kurz. The commit includes `Co-Authored-By` per `AGENTS.md`. ## Test plan - [x] `Http2ResponseBodyControlTest.replayDoesNotDeliverFramesQueuedForTheReplacedStream`: queues DATA frames in a suspended stream channel, injects an I/O error, and checks that after `onRetry` the handler receives the replayed body, not the queued frames. Fails without the change. - [x] `ResponseBodyControlTest.ioExceptionReplayDrainsTheOldConnectionWithAutoReadDisabled`: with auto-read off, an I/O error replays a suspended response; the old connection must be drained and closed once its last chunk arrives. Fails without the change. Over HTTPS the bug does not show, since `SslHandler` issues reads of its own, so the test uses plain HTTP. - [x] Broken variants: completing the control before marking the HTTP/2 stream discarded fails only the HTTP/2 test; dropping the read fails only the HTTP/1.1 test. - [x] Both test classes, compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's `-Djvm`): 20 tests each, 0 failures. - [x] `./mvnw -B -ntp -Dgpg.skip=true clean verify` on JDK 11: 1,821 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Netty's HttpContentDecompressor, and the inflate loop that replaces it for gzip and deflate since AsyncHttpClient#2357, inflate each chunk completely as soon as it is read. Suspending a response through ResponseBodyControl stops further socket reads but not that, so one read of a highly compressible body still became tens of megabytes of body parts nobody had asked for. Check before each part whether the response is suspended. If it is, keep the input the inflater has not used, and whether it still holds output, together with every message that arrives after it, and inflate it on the next read request, such as the one resume() issues, before the read is passed on. To see read requests, the handler also implements ChannelOutboundHandler and passes every other outbound event through; its superclass and constructor stay as they are. A held chunk is retained, not copied, since the inflater may still point into it. Parts are cut where JdkZlibDecoder cuts them, but never past 64 KiB, so that a suspension takes effect within one part even when a large chunk inflates at once: each inflate() call is capped at what is left of a part, which also keeps a forwarding threshold of 0 making progress. The end of a fully inflated body still reaches a suspended response, as for an unencoded body, and a channel that closes while input is held hands the rest on at once. The differential tests against Netty now expect more parts where Netty hands on parts larger than 64 KiB. New unit tests cover suspension, removal, closure and the limit, a direct or pooled direct chunk paused mid-way, and output held without input; a fresh JVM checks a forwarding threshold of 0; the integration tests are those from before the rebuild. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

While a channel is drained after a redirect, an authentication challenge or a replay, AsyncHttpClientHandler discards every body part until the last one. The decompressor still inflated a compressed body in full and copied each part before it was dropped. A body beyond maxDecompressedResponseSize failed the drain, which closed the connection and could fail the request that had moved on with the same future. main behaves the same way. Before the next part, check whether the channel is being drained, is discarded or belongs to an exchange that is over. If so, drop the rest of the body's input instead of inflating it. Its end is still handed on, so a drain callback still runs. When a response filter replays the request on a compressed response that inflates far beyond the limit, the replayed request succeeds and the drained connection returns to the pool. A unit test covers a draining and a discarded channel. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

HttpObjectDecoder reports a chunk it cannot parse as a LastHttpContent with a failed decoder result, and HttpHandler fails the response on it. The inflate loop handed on a new LastHttpContent that carried the trailers only, as Netty's HttpContentDecoder does, so a compressed body cut off by such a chunk completed normally. Keep the result on the end of the body, also when it is handed on after a suspension. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

br, zstd and snappy still went through Netty's HttpContentDecompressor, which decompresses each chunk as soon as it is read, so a suspended response could still receive parts nobody had asked for. Decompress them in a new PullDecompression class, one part at a time with Netty's pull-based Decompressor, under the same checks before each part as gzip and deflate. Small outputs, such as snappy's frames, are merged into a part sized for them; outputs larger than 64 KiB, such as zstd's when io.netty.compression.defaultMaxForwardBytes is raised, are split. Netty added the Decompressor API in 4.2.17 and still marks it unstable. Only PullDecompression refers to it, and it is loaded only once one of these codings shows up, so gzip and deflate keep working with older Netty 4.2 releases. A test hides the API from a class loader to check both sides. The test of reads requested with auto-read off now covers these codings too. The test that mixes this handler's messages with Netty's in one read uses a transfer coding instead of snappy, since that is still left to Netty. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ResponseBodyControl.suspend() did not say that a suspension also stops decompressing an HTTP/1.1 body, nor what it still does not hold back. Document that this holds for the content codings AHC decompresses, not for a transfer coding such as Transfer-Encoding: gzip, and that only parts with content wait for a resume: the end of a fully decompressed body, with its trailers and completion, arrives even while suspended, as for an unencoded body. If the connection closes while input is held back, the rest is decompressed and delivered at once, bounded only by maxDecompressedResponseSize. The README notes that br, zstd and snappy need Netty 4.2.17, and the config and ChannelManager comments mention the suspension. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

mkurz · 2026-11-04T19:58:25Z

Thanks. Done as you suggested:

The replay fix was split out as fix(netty): detach a replayed request's old channel before completing its body control #2367, with the HTTP/2 test. It also has an HTTP/1.1 test: with auto-read off, the same lines fix a second bug on main, where a replay after an I/O error left the old connection undrained and open.
Thanks for merging fix(netty): fail a capped inflate buffer only once it is full, as Netty 4.2.19 does #2366, fix(netty): detach a replayed request's old channel before completing its body control #2367 and test: let the auto-read-off replay test accept a chunk delivered before the replay #2368. fix(netty): fail a capped inflate buffer only once it is full, as Netty 4.2.19 does #2366 fixed main's CI after the Netty 4.2.19 bump, and test: let the auto-read-off replay test accept a chunk delivered before the replay #2368 fixed fix(netty): detach a replayed request's old channel before completing its body control #2367's new HTTP/1.1 test, which was flaky (the macOS failure you re-ran).
I've force-pushed the rebuild on Inflate HTTP/1.1 gzip and deflate without an EmbeddedChannel #2357's inflate loop here. Http1ContentDecompressor keeps its superclass and constructor, and also implements ChannelOutboundHandler to see read requests, so Revapi needs no entries.
- A pause mid-chunk keeps a retained slice of the chunk the inflater still points into, and remembers whether the inflater still holds output without input left. Tests with direct and pooled direct buffers check that the chunk is held while paused, and released after the resume, on removal, on close and on failure.
- Carried over: the drain discard, the part sizing (now for br, zstd and snappy, whose small outputs are merged), the docs, and the failed DecoderResult on the last chunk.
- br, zstd and snappy are in a package-private PullDecompression, which only these codings load. A test hides Netty's Decompressor API from a class loader and checks that gzip and deflate still work, and that a zstd response fails instead of being decoded another way.
- gzip and deflate parts are capped at 64 KiB, so Inflate HTTP/1.1 gzip and deflate without an EmbeddedChannel #2357's comparisons with Netty expect more parts where Netty hands on a larger one.
The description lists the rest, including what differs from the previous revision.
feat: add ResponseBodyControl.execute to decide in sequence with body delivery #2359, fix(netty): keep caller Cookie headers as written on redirects and auth retries #2360 and fix(netty): never let a queued suspend override a later resume #2361 merge cleanly into main and pass the full build there together with this branch, so I'd leave them as they are unless you'd prefer a rebase.

mkurz · 2026-11-04T20:26:11Z

@hyperxpro looking forward to your feedback, thanks! 😉

mkurz · 2026-11-04T21:18:42Z

👍 LGTM Thanks!

zstd is left to Netty's push decoder when io.netty.compression.defaultMaxForwardBytes is zero or negative, so a suspension does not hold it back then. Say so in ResponseBodyControl.suspend(), the Http1ContentDecompressor class Javadoc and the README, and rewrap the suspend() paragraph, which had a short line. Claude Code on behalf of Matthias Kurz Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

mkurz · 2026-11-04T21:33:44Z

Added one small docs commit (8838a42): since zstd is left to Netty's push decoder when io.netty.compression.defaultMaxForwardBytes is zero or negative, a suspension doesn't hold it back then. suspend()'s Javadoc, the class Javadoc and the README now say so, and I rewrapped the short line in the suspend() paragraph. No code changes.

mkurz · 2026-11-04T21:47:22Z

@hyperxpro Now I am done 😉

hyperxpro · 2026-11-05T17:28:25Z

Thank you very much!

## Summary - A `ResponseBodyControl.suspend()` queued from another thread no longer takes effect after a `resume()` that the event loop made in the meantime, which could leave a response suspended with nothing left to resume it. - A `resume()` is still always applied, so it is never lost to a `suspend()` that a callback makes after it was queued. - Document when control calls take effect. No API change. ## Problem `NettyResponseBodyControl` applies a call made on the event loop before it returns, and queues a call made on any other thread to the event loop. Queued calls take effect in the order they were queued, without regard to calls the event loop made inline in the meantime. The stall: 1. A consumer thread calls `suspend()`, which is queued. 2. Before it runs, a body callback on the event loop calls `resume()`, which takes effect immediately. 3. The queued `suspend()` then runs and suspends reads again. Nothing calls `resume()` after that, so the response stalls until the request timeout. The callback's `resume()` was the latest decision, but the older `suspend()` overrode it. This pattern occurs in reactive consumers that suspend from whichever thread finds the buffer full. Play WS hit a variant of it, and avoids it today by suspending only on the event loop. ## Change - `suspend()` records a suspend request, and `resume()` clears it. On the event loop, `suspend()` suspends at once. From another thread, it queues the suspension. - A queued suspension is skipped if, when it would take effect, the most recent call to `suspend()` or `resume()` was a resume. - `resume()` is always applied, and calls made on the event loop take effect before they return, as before. This is not "the latest call wins". Only suspensions can be skipped. Skipping a resume instead would reintroduce a lost-wakeup stall: a consumer takes the last buffered part and calls `resume()`, a callback on an older view calls `suspend()` before that resume is applied, and the resume must still win. A test pins this case. Calls made concurrently on different threads are not ordered beyond these rules. For example, with suspend, resume and suspend queued from other threads, the first suspension still takes effect briefly, because the most recent call is a suspend when it runs; the queued resume and the second suspension then follow. The `ResponseBodyControl` Javadoc now states when calls take effect: - a call on the event loop takes effect before it returns; - calls from other threads take effect later on the loop, possibly after further callbacks; - a `resume()` always takes effect; - a `suspend()` from another thread is skipped if the most recent call was a resume; - concurrent calls on different threads are not ordered otherwise. ## Compatibility No public API change. Behavior changes only for a `suspend()` from another thread whose queued suspension is overtaken by a `resume()`: it no longer takes effect. All other call orders behave as before. ## Related The separate pull request [#2359](#2359), "feat: add ResponseBodyControl.execute to decide in sequence with body delivery", lets a consumer make its suspend/resume decision on the event loop in the first place. The two changes are independent and merge cleanly with each other. ## AI disclosure Claude Code on behalf of Matthias Kurz. The commits include `Co-Authored-By` per `AGENTS.md`. ## Test plan New tests, for HTTP/1.1, for HTTP/2, and as unit tests in `NettyResponseBodyControlTest` (with Netty auto-read on and off): - an earlier `suspend()` from another thread does not undo a later `resume()`; - a `resume()` from another thread is not lost to a later `suspend()`. `NettyResponseBodyControlTest` also covers: - sequences of calls queued from another thread: suspend, resume, suspend ends suspended, and resume, suspend, resume ends resumed, with the exact number of suspensions started and ended; - a `suspend()` after a skipped one still takes effect; - a `suspend()` on the event loop takes effect even if a `resume()` from another thread clears its request between recording and acting on it. The channel's event loop is a proxy whose `inEventLoop()` runs that resume at exactly that moment. The unit tests also check that the suspension start/end hooks stay balanced and that exactly one read is requested per resume. Calls from another thread go through a single-thread executor owned by each test, not the common pool. Broken variants, each run against the 31 control tests: - [x] A queued suspension always applied, as on `main`: 7 failures and 1 error (HTTP/2, by timeout). These are the tests that an earlier `suspend()` must not undo a later `resume()`, the sequence test, and the test of a suspension after a skipped one. - [x] The latest call always wins, so a queued `resume()` is dropped for a later `suspend()`: 5 failures and 1 error (HTTP/2, by timeout). These are the tests that a `resume()` must not be lost, and the sequence test. - [x] A skipped suspension that still runs the suspension start hook: 4 failures, all in `NettyResponseBodyControlTest`, through the counts of started suspensions. The integration tests don't catch this variant. - [x] A `suspend()` on the event loop that still goes through the queued-suspend check, as in the first revision: 1 failure, the race test. Runs: - [x] `ResponseBodyControlTest`, `Http2ResponseBodyControlTest` and `NettyResponseBodyControlTest` on JDK 11: 31 tests passed. - [x] `./mvnw -B -ntp -Dgpg.skip=true clean verify` on JDK 11: 1,804 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used. - [x] The same 31 tests, compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's `-Djvm`): 0 failures on each. - [x] All four open AHC changes merged together (raw `Cookie` headers on retries, `ResponseBodyControl.execute`, suspend/resume order, demand-bounded decompression): they merge without conflicts, together and in every pair, and `./mvnw -B -ntp -Dgpg.skip=true clean install` on JDK 11 passes with 1,861 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. Rerun with the review fixes of this pull request and #2362. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

## Summary - Upgrades Netty from 4.2.19.Final to 4.2.20.Final. Its [security fixes](https://github.com/netty/netty/releases/tag/netty-4.2.20.Final) include several in modules AHC uses: three out-of-bounds reads in `netty-buffer`, unbounded memory usage in `netty-codec-http2`, insufficient control character neutralization in `netty-codec-http`, insufficient entropy in `netty-resolver-dns`, and an mTLS bypass via session resumption in `netty-handler`. - On 4.2.20, two of AHC's tests fail, both because of Netty changes. This PR adapts AHC to them. ## Change - **Upgrade Netty to 4.2.20.Final, and check the second gzip magic byte:** `JdkZlibDecoder` now rejects a gzip header with a wrong second magic byte (netty/netty#17972). The gzip inflate loop from #2357 follows `JdkZlibDecoder`, and `Http1ContentDecompressorTest` compares the two, so it now checks that byte too. Both bytes are read unsigned, since `0x8b` is negative as a signed byte. - **Keep zstd working at a non-positive threshold:** since netty/netty#17896, the `ZstdDecoder` that `HttpContentDecompressor` creates sizes its output buffers by `io.netty.compression.defaultMaxForwardBytes`. At 0 it hands on an empty body without an error; below 0 it fails with `IllegalArgumentException: initialCapacity : -1`. AHC leaves zstd to that decoder while the property is not positive (#2362), so it now replaces it with a `ZstdDecoder` whose buffers hold `MAX_PART_SIZE`. The size limit still applies, and the body is still decompressed eagerly there, as before. At the default value of the property nothing changes. netty/netty#18289 fixes the decoder in Netty; this workaround is harmless with that fix and can go once AHC requires a Netty version that has it. ## Notes - Netty 4.2.0 to 4.2.4 have no `ZstdDecoder(int)` constructor. Overriding AHC's Netty with one of those and also setting the property to 0 or below would fail zstd responses with a `NoSuchMethodError`. ## AI disclosure Claude Code on behalf of Matthias Kurz. Both commits include `Co-Authored-By` per `AGENTS.md`. ## Test plan - [x] `main` at `5542c9fc4` with `-Dnetty.version=4.2.20.Final`: 1,857 tests, 2 failures: - `Http1ContentDecompressorTest.decodesLikeNettyForEveryBodyAndSplit`, case "gzip second magic byte ignored": Netty now fails that body, AHC did not. - `Http1ContentDecompressorForwardThresholdTest`: "the zstd body differs". - [x] `Http1ContentDecompressorForwardThresholdTest` now runs at thresholds 0 and -1, each in a fresh JVM, and also checks the size limit for zstd and that every exchange releases its input and buffers. On 4.2.20 without the zstd commit it fails at both ("the zstd body differs", "the zstd body did not end"). With it, it passes on 4.2.19 and 4.2.20. - [x] `./mvnw -B -ntp -Dgpg.skip=true clean verify` on JDK 11, on `main` `3e95ec8c2`: 1,905 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used. - [x] `Http1ContentDecompressorForwardThresholdTest` and `Http1ContentDecompressorTest` compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's `-Djvm`): 36 tests each, 0 failures. - [x] This branch merged with #2369: `clean verify` on JDK 11, 1,914 tests, 0 failures, 0 errors, 11 skipped; Revapi passed (run before #2359, #2360 and #2361 were merged, with those three merged in as well). --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

This is the first of the two follow-ups promised on #2362 ([discussion](#2362 (comment))). A compressed HTTP/1.1 response body that ends in the middle of its compressed stream now fails the request, instead of completing as if it were whole. Netty's `HttpContentDecompressor` behaves like `main` here, so this is a behavior change and gets its own PR. ## Problem A body can end early, for example when the connection closes in the middle of a close-delimited body. On `main` such a body completed normally: - gzip: a member cut off in its header, data or trailer completed, so its CRC32 and length were never checked. - deflate: a stream that never finished completed. - br, zstd and snappy: Netty's `Decompressor` was never told that the input had ended, so its own truncation check never ran. ## Change Once a body has been decompressed in full, `Http1ContentDecompressor` checks that it ends where a gzip member or deflate stream ends, and `PullDecompression` calls `Decompressor.endOfInput()`, which throws for a truncated br, zstd or snappy stream. Output the decompressor still produces after that waits for a resume like any other part. The check only runs once held input and output have been handed on, also after a suspension and when the connection closes while input is held back. A truncated body fails the request with a `DecompressionException`. For zstd, Netty wraps zstd-jni's `ZstdIOException`, and since AHC fails a request with the innermost cause, that looked like a failed connection and went to the `IOExceptionFilter`s, where a replaying filter could leave the first connection open. `PullDecompression` now throws a plain `DecompressionException("Truncated zstd stream")` instead and keeps zstd-jni's exception as a suppressed one. Still accepted: - a body without any bytes, for every coding (`endOfInput()` is only called once input was added, and only in `NEED_INPUT`) - concatenated gzip members - a few bytes after a complete gzip member, fewer than a gzip header, which `JdkZlibDecoder` leaves unread Not checked: - a body nobody wants, such as the rest of a response drained for a replay, or one whose exchange is over. That includes a body too short to have produced any output, so the end of the body checks again whether it is wanted. - a last chunk that failed to parse, so that its own failure surfaces. A premature close of a `Content-Length` or chunked body produces no `LastHttpContent` at all. - bodies left to Netty's `HttpContentDecompressor`: br, zstd and snappy with a Netty older than 4.2.17, zstd at a non-positive `io.netty.compression.defaultMaxForwardBytes`, and transfer codings such as `Transfer-Encoding: gzip` - HTTP/2 response bodies Some cuts also go unnoticed. snappy has no end marker, so a body cut between two of its chunks looks complete. A zstd body of several frames completes when it is cut inside a frame that started in the same chunk as the end of the previous one, because zstd-jni only notices the new frame on a fresh read. The class Javadoc and the README describe all of this. The second follow-up, output left over when the connection closes, comes separately. ## Tests Fail on `main`, pass here: - `Http1DecompressionTruncationTest.aBodyThatTheConnectionEndsInTheMiddleOfItsStreamFails`: through the client, a close-delimited body still decodes in full for all five codings, and the same body cut short by 3 bytes fails with a `DecompressionException`. - `Http1DecompressionTruncationTest.aCutOffBodyIsNotPassedToTheIOExceptionFilters`: a cut-off `Content-Length` body with a replaying `IOExceptionFilter` fails with a `DecompressionException`, the filter is not called, no second request is sent and the client closes the connection. - `Http1ContentDecompressorTest.decodesLikeNettyForEveryBodyAndSplit`: the truncated gzip and deflate bodies (header, header name, data, missing trailer, partial trailer, second member, zlib, raw, missing Adler-32, one byte) now fail with `Truncated gzip stream` or `Truncated deflate stream` across all splits and buffer kinds, with no `IOException` in the chain. Every other body still decodes like Netty. - `Http1ContentDecompressorTest.truncatedBrZstdAndSnappyBodiesFailAndBodiesWithoutBytesEnd`: br, zstd and snappy bodies cut in half or by one byte fail, and bodies without bytes still end. - `Http1ContentDecompressorSuspensionTest.aTruncatedBodyFailsAtItsEndAfterTheResumes` (gzip and zstd) and `aTruncatedBodyHeldWhenTheConnectionClosesFails`: the parts before the cut are still handed on, then the body fails. - `reusesTheInflaterAcrossResponsesOnOneConnection` and `aBufferThatCannotGrowFailsLikeNetty` sent cut-off bodies on purpose and now expect the failure. Pass on `main` too, and fail if the end of the body stops checking whether it is wanted: - `Http1DecompressionTruncationTest.aReplayIsNotFailedByTheCutOffBodyItDrains`: a `ResponseFilter` replays a 503 whose gzip body is only 6 bytes, and the replay still gets its 200. - `Http1ContentDecompressorSuspensionTest.aShortBodyThatIsNotWantedEndsEvenIfItIsCutOff`: the first 1 to 12 bytes of a body that is discarded, drained or whose exchange is over end without a failure, for every coding. Each of these fails without the code it covers: - `aFailedLastChunkStaysFailed`, for the check of the last chunk's decoder result - `outputThatOnlyFollowsTheEndOfInputIsHandedOn` and `outputThatOnlyFollowsTheEndOfInputWaitsForResumes`, for output that only comes after `endOfInput()` - `reusesTheInflaterAcrossResponsesOnOneConnection`, for the reset of the gzip member state between responses `./mvnw -B -ntp -Dgpg.skip=true clean verify` on JDK 11, merged with `main` at `f9eafb982` (so with #2375 and #2380): 1,960 tests, 0 failures, 0 errors, 11 skipped, Revapi passed. That run predates a test-only fix in the new client test. The `Http1ContentDecompressor*Test`, `Http2ContentDecompressorTest` and `Http1Decompression*Test` classes also pass on JDK 17, 21 and 25, 101 tests each. Claude Code on behalf of Matthias Kurz. The commits carry `Co-Authored-By` as `AGENTS.md` asks. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

This was referenced Oct 7, 2026

Inflate HTTP/1.1 gzip and deflate without an EmbeddedChannel #2357

Merged

HTTP/2: Respect read limits for frames queued at end of stream netty/netty#17774

Open

mkurz mentioned this pull request Oct 7, 2026

Upgrade to AHC 3.0.15 and decide streamed-body resumes through ResponseBodyControl.execute playframework/play-ws#1487

Draft

18 of 22 tasks

hyperxpro requested changes Oct 21, 2026

View reviewed changes

mkurz mentioned this pull request Oct 22, 2026

fix(netty): never let a queued suspend override a later resume #2361

Merged

8 tasks done

mkurz requested a review from hyperxpro October 22, 2026 10:48

This was referenced Nov 3, 2026

fix(netty): fail a capped inflate buffer only once it is full, as Netty 4.2.19 does #2366

Merged

fix(netty): detach a replayed request's old channel before completing its body control #2367

Merged

mkurz and others added 5 commits November 4, 2026 20:54

mkurz force-pushed the fix/demand-bounded-decompression branch from b5db977 to 497aad1 Compare November 4, 2026 19:57

hyperxpro added 4 commits November 4, 2026 21:00

Cap inflate buffers at the part size

b8b8111

Fall back to Netty's decoders without the pull API

3af3a7f

Leave zstd to Netty at a zero forwarding threshold

7ae4ec4

Say when br, zstd and snappy wait for demand

d4e74a7

hyperxpro approved these changes Nov 4, 2026

View reviewed changes

hyperxpro merged commit 5542c9f into AsyncHttpClient:main Nov 5, 2026
13 checks passed

mkurz deleted the fix/demand-bounded-decompression branch November 5, 2026 18:22

This was referenced Nov 5, 2026

Upgrade Netty to 4.2.20.Final #2375

Merged

Fail a compressed body that is cut off #2376

Merged

mkurz mentioned this pull request Nov 7, 2026

Drop held input of a body cut off by the close #2386

Open

5 tasks done

Conversation

mkurz commented Oct 7, 2026 • edited by hyperxpro Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Change

Why HTTP/2 decompression is unchanged

Behavior changes

Compatibility

Follow-ups

Review fixes

AI disclosure

Test plan

Uh oh!

mkurz commented Oct 7, 2026

Uh oh!

mkurz commented Oct 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkurz commented Oct 22, 2026

Uh oh!

mkurz commented Oct 23, 2026

Uh oh!

hyperxpro commented Nov 3, 2026

Uh oh!

mkurz commented Nov 4, 2026

Uh oh!

mkurz commented Nov 4, 2026

Uh oh!

mkurz commented Nov 4, 2026

Uh oh!

mkurz commented Nov 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkurz commented Nov 4, 2026

Uh oh!

Uh oh!

hyperxpro commented Nov 5, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mkurz commented Oct 7, 2026 •

edited by hyperxpro

Loading

mkurz commented Nov 4, 2026 •

edited

Loading