Skip to content

fix(netty): stop inflating a suspended HTTP/1.1 response - #2362

Open
mkurz wants to merge 5 commits into
AsyncHttpClient:mainfrom
mkurz:fix/demand-bounded-decompression
Open

mkurz wants to merge 5 commits into
AsyncHttpClient:mainfrom
mkurz:fix/demand-bounded-decompression

Conversation

@mkurz

@mkurz mkurz commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • While an HTTP/1.1 response is suspended through ResponseBodyControl, stop decompressing its body. Decompressed body parts are at most 64 KiB, so a handler that suspends after each part receives one part per resume().
  • Http1ContentDecompressor becomes a ChannelDuplexHandler on Netty's pull-based Decompressor, instead of extending Netty's HttpContentDecompressor. Codings, multi-member gzip, header rewrites and the maxDecompressedResponseSize limit are kept.
  • HTTP/2 decompression is unchanged; see below.
  • A replay (from an IOExceptionFilter or a ResponseFilter) now detaches the old channel before it completes the old response's body control, so the retried handler gets nothing of the old response. This also fixes HTTP/2 on main.

Problem

Suspending a response stops further socket reads, but Netty's HttpContentDecompressor inflates 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) became some 67 MB of body parts for a handler that had suspended the response. Only maxDecompressedResponseSize (256 MiB by default) bounded it.

With 64 MiB of gzipped zeros and a handler that suspends inside each body callback:

  • main delivered 1,955,380 bytes in 30 more parts past the suspending part;
  • after that, it delivered up to 33,713,376 bytes per resume, so the whole body arrived in 2–3 resumes.

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

Http1ContentDecompressor uses Netty's pull-based Decompressor (Netty 4.2.17+, AHC pins 4.2.18):

  • Codings: gzip and x-gzip, including bodies made of several gzip members (RFC 1952, section 2.2) as before; deflate and x-deflate; br and zstd when available; and snappy.
  • Kept behavior: the header rewrites, the 100 Continue pass-through, and the maxDecompressedResponseSize accounting and message. The limit counts every gzip member.
  • Output in bounded parts: it hands the body on in parts of at most 64 KiB. Decompressor output larger than that, such as zstd's when Netty's io.netty.compression.defaultMaxForwardBytes is raised, is split into retained slices. A slice shares the larger buffer until the slice and the rest have both been released, so the 64 KiB bound applies to delivered parts, not to allocations. Before each part, the handler checks whether the response is suspended.
  • While suspended: the compressed input stays in the handler, together with every message that arrived after it. To fill a part, the handler may take one more decompressor output buffer than it hands on, so decompression can run ahead of delivery by one such buffer (at most 64 KiB with Netty's defaults).
  • On resume: the handler intercepts the read request that resume() issues. It continues decompression first, and only then lets the read reach the socket. This works the same with auto-read on and off.
  • Without suspension: each part goes straight through, as before, and a single resume can deliver many parts.
  • End of the body: only parts with content wait for a resume. Once the body has been decompressed in full, its terminating chunk, 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. The terminating chunk still reaches a drain callback.
  • Small outputs: they are copied together into parts of up to 64 KiB, so part counts match main. A merged part is allocated for what it holds and grows as needed.

Replays

replayRequest completed the old response's body control before it detached the old channel from the future. Completing the control turns auto-read back on, and the read that this issues delivers whatever the old response still holds back right away:

  • on HTTP/1.1, input held by a suspended decompressor (new with this change);
  • on HTTP/2, DATA frames queued in a suspended stream channel (also on main).

The channel still named the future, so the handler that had just been told to retry received parts of the abandoned response before the retry's status. With auto-read off, nothing read the old HTTP/1.1 channel, so it was never drained or closed. The old channel is now detached first: through the drain callback on HTTP/1.1, through the discard marker on HTTP/2. With auto-read off, one read starts the drain.

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), now proposed in netty/netty#17774, or AHC delivering the body independently of the stream channel.

Behavior changes

  • Suspended HTTP/1.1 responses 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.
  • Replays: after onRetry(), the handler no longer receives parts of the old response, on HTTP/1.1 and HTTP/2.
  • Drains: a compressed body that is drained is no longer decompressed. 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 are now at most 64 KiB; before, they were usually just over 64 KiB.
  • Framing errors on the final chunk, such as a bad chunk size, now fail the request. Netty's decoder used to turn such a failure into a successful, truncated response.
  • The gzip header check is stricter: the pull decoder checks both gzip magic bytes.
  • -Dio.netty.noJdkZlibDecoder is no longer honored on HTTP/1.1; the JDK's zlib is always used.
  • A corrupt gzip body may deliver fewer bytes before it fails. It fails in the same cases as before, but output decompressed in the same step that detects the damage (for example a checksum mismatch, or trailing data that is not another member) is dropped instead of handed on first.
  • Unchanged, compared with main on the same inputs:
    • several gzip members are decompressed in full, and a corrupt checksum or length in a later member fails the request;
    • bytes after a gzip member must form another member, so other trailing data fails the request, unless it is shorter than a gzip header;
    • bytes after the end of a deflate stream are dropped;
    • truncated or empty compressed bodies are accepted. That includes corrupt compressed data that makes the stream look truncated.
  • Performance without suspension: median elapsed time of 10 × 64 MiB over loopback on JDK 11, from two alternating runs. Measured on the first revision of this change; the later multi-member and part-splitting fixes do not touch the inflate path.
    • Gzipped zeros: unchanged (117–120 ms on main, 120 ms here).
    • Gzipped text: about 3–7% slower (135–143 ms against 145–147 ms), which comes from Netty's pull decompressor making more, smaller inflate calls.
    • Plain bodies: unchanged.

Compatibility

  • API: Http1ContentDecompressor is a public class, so this is a binary-incompatible change to it: Revapi reports its changed superclass, plus the members inherited from Netty's decoder and its hooks. AHC only uses it internally (ChannelManager instantiates it, and its public constructor is unchanged), so nothing changes for code that uses AHC's client API. Each difference is listed as an exact Revapi entry with a justification in pom.xml, so any other change to the class is still flagged.
  • Netty 4.2.17 or later is required. Netty added the pull-based Decompressor API in 4.2.17. With automatic decompression on, the default, AHC creates this handler for every HTTP/1.1 connection, so an application that pins netty.version below 4.2.17 gets a NoClassDefFoundError when an HTTP/1.1 connection is set up, also for responses that are not compressed. Please mention this in the release notes.
  • Netty API stability: Netty still marks its Decompressor API as unstable (@UnstableApi) while it migrates its codecs to it (netty/netty#16743). A future Netty release may require changes to this handler.

Follow-ups

  • Call the decompressor's endOfInput() at the end of the body, so a body cut off in the middle of a gzip member, or missing its checksum, fails instead of completing. 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.

AI disclosure

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

Test plan

New tests:

  • Http1DecompressionSuspensionTest (10 tests, live server):

    • the bound with auto-read on and off: full length, all-zeros content;
    • connection reuse, a chunked body with trailers, and a body ended by connection close;
    • cancel, ABORT, and HEAD and empty gzip responses;
    • a replay while input is held back, with auto-read on and off. The IOException is injected after the body callback and the decompressor's pass have returned. The test checks that the retry's status follows onRetry() directly and that the old connection is drained and closed;
    • a ResponseFilter replay on a compressed response that inflates far beyond the limit: the replayed request succeeds, and the test waits until the pool has accepted the drained connection.

    On main, its two bound tests fail deterministically.

  • Http1ContentDecompressorTest (22 tests, EmbeddedChannel):

    • every encoding, the header rewrites, and unsupported encodings passed through;
    • bodiless responses, trailers and 100 Continue;
    • one part per read, and suspension before the body;
    • close while input is held, removal releasing held input, and the size limit on the resume path;
    • gzip bodies of two members, in one buffer, in 1-byte fragments and split at the member boundary;
    • suspension across a member boundary, a corrupt checksum or length in the second member, trailing data after gzip and deflate streams, and the size limit across members;
    • every coding with fragmented input and a resume per part, with auto-read on and off;
    • the end of a fully inflated body, with its trailers, without another resume, also for a body without bytes;
    • merged parts allocated for what they hold, and growing up to 64 KiB.
  • Http2ResponseBodyControlTest (1 new test): a replay while DATA frames wait in the suspended stream channel. Before the failure is injected, the client acknowledges a PING that the server sent after the DATA frames, so they are known to have arrived. The retried handler must get the replayed response, not the old stream's frames. It fails on main.

  • Http1ContentDecompressorLargeOutputTest (1 test):

    • starts a fresh JVM with -Dio.netty.compression.defaultMaxForwardBytes=262144, because Netty reads that property once per JVM. The child writes to a file, the test fails if the child has not finished after 60 seconds, and the child is killed in any case. With a deliberately hanging child, the test failed after 64 seconds and left no process behind;
    • first checks that zstd output in that JVM really exceeds 64 KiB, then that parts stay within 64 KiB, one per resume;
    • checks 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 of the first revision, each run against its 27 tests:

  • Without the suspension check before each part: 11 failures and 1 error, including both bound tests.
  • Read requests passed straight through, without continuing decompression first: 9 failures and 2 errors, including both bound tests and the chunked-body test.
  • Held input not decompressed when the connection closes: 2 failures, the close-delimited body test and the channelInactive ordering test.
  • Without decoding concatenated gzip members: 6 failures, all in the new gzip member tests.
  • Without splitting output larger than a part: the fresh-JVM test fails.

Broken variants of the review fixes, each run against Http1DecompressionSuspensionTest and Http1ContentDecompressorTest (32 tests):

  • The old body control completed before the drain callback is installed: the replay test with auto-read on fails, because the retried handler gets parts of the old response after onRetry().
  • No read when auto-read is off: the replay test with auto-read off fails, because the old connection is never drained or closed.
  • A drain callback not treated as discarded: the ResponseFilter replay test fails, because the failed drain closes the connection instead of returning it to the pool. Whether the replayed request also fails depends on timing.
  • Merged parts allocated at 64 KiB: the small merged-part test fails (a capacity of 65,536 for 1,000 bytes).
  • Merged parts that cannot grow: the growth test fails, as do most tests that merge outputs.
  • The end of the body held back while suspended: the end-of-body test fails.
  • HTTP/2, on unmodified main: the new Http2ResponseBodyControlTest test fails, because the retried handler first gets the old stream's frame a.

Runs:

  • ./mvnw -B -ntp -Dgpg.skip=true clean verify on JDK 11: 1,825 tests, 0 failures, 0 errors, 11 skipped; Revapi passed. No test-skipping flags were used.
  • Targeted tests (the new tests above, plus Http1DecompressionLimitTest, AutomaticDecompressionTest, Http2ContentDecompressorTest, ResponseBodyControlTest, the rest of Http2ResponseBodyControlTest, Expect100ContinueTest and LargeResponseTest), compiled on JDK 11 and run on JDK 17, 21 and 25 (Maven Surefire's -Djvm): 73 tests, 0 failures on each. The fresh-JVM test's child runs on the same JDK.
  • Compared with Netty's HttpContentDecompressor (what main uses) on 15 kinds of input, each in one buffer and in 1-byte fragments (concatenated, truncated and corrupt gzip members, trailing data after gzip, zlib and raw deflate streams): every input succeeds or fails as on main, with the same body when it succeeds. In 5 of the 30 runs both fail, and this branch hands on fewer bytes before the failure. Run on the first revision; the review fixes don't change how a body is decoded.
  • 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 fix(netty): never let a queued suspend override a later resume #2361 and this pull request.
  • 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. It finds 0 bytes delivered beyond demand, against 1,875,916 bytes with AHC 3.0.14. It measures delivered parts, not memory inside the decompressor.
    • Play WS's shaded jar includes Netty's pull decompressors.

Motivation:
HTTP/1.1 automatic decompression ran Netty's HttpContentDecompressor,
which inflates every chunk completely as soon as it is read. Suspending
a response through ResponseBodyControl stops further socket reads, but
not that. One 64 KiB read of a highly compressible gzip body (about
1032:1) still became some 67 MB of body parts for a handler that had
asked for none; only maxDecompressedResponseSize (256 MiB) bounded it.
AHC 2 behaved the same way, so this is hardening, not a regression fix.

Modification:
Http1ContentDecompressor is now a ChannelDuplexHandler built on
Netty's pull-based Decompressor (Netty 4.2.17+; Netty still marks this
API unstable). It keeps the codings (gzip, including bodies of several
gzip members, deflate, br, snappy, zstd), the header rewrites and the
maxDecompressedResponseSize accounting. It hands the body on in parts
of at most 64 KiB, splitting larger decompressor output, and checks
before each part whether the response is suspended. If it is, the
compressed input stays in the handler, and so does every message
behind it. The read request that resume() issues continues
decompression before it reaches the socket. Without suspension, each
part goes straight through as before.

The pipeline is torn down with its channel, so if the connection
closes while input is held back, the rest is inflated and delivered
before channelInactive. Once the exchange has ended (cancel, ABORT,
timeout), held input is dropped instead of inflated. A framing failure
on the final chunk now reaches the handler; Netty's decoder used to
replace it with success.

Revapi reports the changed superclass of this public handler class and
the members inherited from Netty's decoder; they are justified in the
pom.

HTTP/2 is unchanged. After END_STREAM, Netty's stream channel hands
every queued frame over on the next read and then closes. A handler in
the stream pipeline therefore cannot hold compressed input back.

Result:
A response whose handler suspends it after each body part receives one
part of at most 64 KiB per resume, and decompression runs ahead of the
delivered parts by at most one decompressor output buffer. For 64 MiB
of gzipped zeros, main delivered 1.96 MB in 30 more parts past the
suspending part, then up to 33.7 MB per resume; now nothing past it,
and 65,536 bytes per resume.

Http1DecompressionSuspensionTest covers auto-read on and off, chunked
bodies with trailers, close-delimited bodies, cancel, ABORT, bodiless
responses and connection reuse; its two bound tests fail on main.
Http1ContentDecompressorTest covers the handler on its own, including
gzip bodies of several members, corrupt and trailing data, and every
coding across repeated suspension.
Http1ContentDecompressorLargeOutputTest covers decompressor output
larger than a part, in a JVM with Netty's
io.netty.compression.defaultMaxForwardBytes raised. Loopback throughput
without suspension: unchanged for gzipped zeros, about 5% more elapsed
time for gzipped text.

Claude Code on behalf of Matthias Kurz

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mkurz

mkurz commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

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

mkurz commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

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.

}

@Override
public void read(ChannelHandlerContext ctx) {

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.

This drain also runs from replayRequest. It calls NettyResponseBodyControl.complete(future) before drainChannelAndOffer swaps the attribute, and setAutoRead(true) issues a channel.read() right there. So whatever is held back gets inflated under the old future and the handler that just got onRetry() receives the old body. I reproduced it with an injected IOException and a replaying IOExceptionFilter, 31 parts of the first response arrived before the retried status. With AUTO_READ off nothing issues that read at all, so the old channel is never drained and never closed. We should install the drain callback before complete(future) and read once when autoRead is off. Not reachable on main since nothing was held back there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed and fixed in 5604fea. replayRequest now installs the drain callback (HTTP/1.1) or the discard marker (HTTP/2) before complete(future). With auto-read off, it issues one read so the drain starts.

The new tests inject the IOException the way you did, after the body callback and the decompressor's pass have returned. They run with auto-read on and off, check that the retry's onStatusReceived comes right after onRetry, and check that the old connection is drained and closed.

The same ordering also affected HTTP/2, on main too. Completing the control turns auto-read back on, and the stream channel then hands its queued DATA frames to the retried handler right away. Http2ResponseBodyControlTest.replayDoesNotDeliverFramesQueuedForTheReplacedStream fails on main: the retried handler first gets the old stream's first frame. Before it injects the failure, the test waits until the client has acknowledged a PING that the server sent after the DATA frames, so the frames are known to wait in the stream channel.

closeDecompressor();
continue;
}
if (!inputClosed && isSuspended(channel)) {

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.

This is the bomb case the PR is about, just with a close after it. A suspended handler gets the whole remainder in one go with no resume, bounded only by maxDecompressedResponseSize. I get that the pipeline is gone right after, so no code fix here, but the javadoc on suspend() and the behavior changes section should say that close delimited bodies are not bounded. Worth a follow-up next to the HTTP/2 one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, no code change. Documented in b5db977, in suspend() and in the class Javadoc, and added to the description. If the connection closes while input is held back, the rest is inflated and delivered at once, even while suspended, and only maxDecompressedResponseSize bounds it. That applies to any close that finds input held back, not only to close-delimited bodies. I'll take it up as a follow-up alongside the HTTP/2 one, although the two may need different fixes.

if (part != null) {
ctx.fireChannelRead(new DefaultHttpContent(part));
}
} else if (lastContent != null) {

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.

Same for the last content, it goes through while suspended, so a handler that suspends on the last data part still sees the trailers and onCompleted(). Fine if intended, but then say so next to the no further part until resume statement, or hold it until the next read.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Intended, and now documented next to that statement (b5db977). Only parts with content wait for a resume. Once the body has been inflated in full, the empty last part, trailers and onCompleted() follow even while suspended, as they do for an unencoded body.

I chose this over holding the end back because a Reactive Streams consumer, such as Play WS's publisher, resumes only while it has demand. It still has to signal completion to a subscriber that has received exactly what it requested. If the end waited for another resume, such a stream would never complete. A test pins this behavior, also for a compressed response without body bytes.

} else {
if (!merged) {
ByteBuf first = part;
part = alloc.heapBuffer(MAX_PART_SIZE);

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.

This always allocates MAX_PART_SIZE even when the merged part ends up a few hundred bytes, and a consumer that keeps parts around holds 64 KiB for each of them. Size it to what is actually being merged, the buffer grows if it has to.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 9e825e6. The merged part is allocated for its first two buffers and grows as needed, up to 64 KiB. Ten 100-byte gzip members now give one part of 1,000 bytes with a capacity of 1,024 instead of 65,536.

import io.netty.handler.codec.compression.BrotliDecompressor;
import io.netty.handler.codec.compression.DecompressionException;
import io.netty.handler.codec.http.HttpContentDecompressor;
import io.netty.handler.codec.compression.Decompressor;

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.

This class only exists since 4.2.17, so anyone managing netty.version below that gets a NoClassDefFoundError on every HTTP/1.1 channel init, not just on compressed responses. Needs at least a release note.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. The Compatibility section of the description now has it as a release note. AHC now needs Netty 4.2.17 or later. With an older netty.version, the NoClassDefFoundError comes when an HTTP/1.1 pipeline is set up with automatic decompression on, the default, so it also hits requests whose responses are not compressed. The class Javadoc says so too (b5db977).

&& decompressor.status() == Decompressor.Status.NEED_INPUT) {
decompressor.addInput(input.retain());
}
if (content instanceof LastHttpContent) {

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.

We never call endOfInput(), so a body cut off mid member still completes as a success and a missing CRC32 is never checked. Same as main, but with this API it is a two liner now, call it when any body byte was fed and keep empty bodies as they are. Follow-up is fine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I'll send it as a follow-up. Failing a body that ends in the middle of a member changes behavior that main has, so it seems worth its own PR. It also has to avoid calling endOfInput() while held input or output is still pending, and keep empty bodies and concatenated members working.

&& NettyResponseBodyControl.isSuspended((NettyResponseFuture<?>) attribute, channel);
}

private static boolean isAbandoned(Channel channel) {

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.

A drain callback is neither suspended nor abandoned, so a redirect or 401 with a compressed body still inflates all of it up to the limit, and every part is copied and released right away. Same as main, but treating OnLastHttpContentCallback like DISCARD here would make the drain almost free.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 7a00b8a. A drain callback is now treated like DISCARD, so the rest of the body is dropped instead of inflated, and the last chunk still reaches the callback.

One detail: redirects and 401/407 challenges with a compressed body rarely drain here. The decompressor rewrites Content-Length to chunked, as Netty's does, and those interceptors close the connection for a chunked response instead of draining it. The drain that did inflate is the one after a replay from a ResponseFilter. The new test replays on a compressed response far beyond the limit, and waits until the pool has accepted the drained connection. Before the fix, the failed drain closed the connection and could fail the replayed request, which shares the future.

* bounds how far the body may inflate in total.
* <p>
* Netty's {@code HttpContentDecompressor} inflates each compressed chunk completely as soon as it is read.
* Suspending a response through {@link org.asynchttpclient.ResponseBodyControl} stops further socket reads but

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.

Import ResponseBodyControl instead, AGENTS.md asks for no fully qualified names.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b5db977.

mkurz and others added 4 commits October 22, 2026 01:28
Motivation:
replayRequest completed the old response's body control before it
detached the old channel from the future. Completing the control
restores auto-read, and the read that issues delivers what the old
response still holds back on the spot: input held by a suspended
HTTP/1.1 decompressor, or DATA frames queued in a suspended HTTP/2
stream channel. The channel still named the future, so the handler
that had just been told to retry received parts of the abandoned
response before the replayed response's status. With auto-read off,
nothing read the old HTTP/1.1 channel at all, so it was never drained
and never closed.

The HTTP/1.1 case comes with the demand-bounded decompressor; the
HTTP/2 case also happens on main.

Modification:
Install the HTTP/1.1 drain callback, or mark the HTTP/2 stream channel
as discarded, before completing the control. When auto-read is off,
issue one read so that the drain starts.

Result:
After onRetry, the handler receives the replayed response, starting
with its status. The HTTP/1.1 tests, with auto-read on and off, inject
the failure after the body callback and the decompressor have
returned. The HTTP/2 test queues DATA frames in the suspended stream
channel; it fails on main.

Claude Code on behalf of Matthias Kurz

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Motivation:
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.

Modification:
Treat a channel whose attribute is an OnLastHttpContentCallback like a
discarded one: drop the remaining input instead of inflating it. The
terminating chunk is still handed on, so the callback still runs.

Result:
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. Redirects and 401 or 407
challenges with a compressed body rarely get here: the decompressor
rewrites Content-Length to chunked, as Netty's does, so they close the
connection instead of draining it.

Claude Code on behalf of Matthias Kurz

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Motivation:
A part merged from several small decompressor buffers was always
allocated with a capacity of 64 KiB, even when it held a few hundred
bytes, and a consumer that keeps parts holds that capacity for each.

Modification:
Allocate the merged part for the first two buffers and let it grow,
up to 64 KiB, as more are copied in.

Result:
Ten gzip members of 100 bytes become one part of 1,000 bytes with a
capacity of 1,024 bytes instead of 65,536.

Claude Code on behalf of Matthias Kurz

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Motivation:
Neither the decompressor nor ResponseBodyControl.suspend() said which
parts of a compressed HTTP/1.1 body a suspension does not hold back,
or which Netty version the decompressor needs.

Modification:
Document in suspend() and in the decompressor that:
- only parts with content wait for a resume. Once the body has been
  inflated in full, its last part, trailers and completion arrive even
  while suspended, as for an unencoded body;
- if the connection closes while input is held back, the rest is
  inflated and delivered at once, bounded only by
  maxDecompressedResponseSize.
Note that the decompressor needs Netty 4.2.17 or later, and import
ResponseBodyControl instead of naming it in full.

Result:
A test pins that the end of a fully inflated body, and of a body
without bytes, does not wait for a resume. Holding it back would leave
a consumer whose demand ended with the last part without completion.

Claude Code on behalf of Matthias Kurz

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mkurz

mkurz commented Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

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
mkurz requested a review from hyperxpro October 22, 2026 10:48
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