Repository navigation
Conversation
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>
|
Heads-up: #2357, opened before this PR, which I missed, rewrites the same If you'd like #2357 first, I'd rebuild the demand bounding on top of its inflate loop instead of Netty's pull |
|
Related to "Why HTTP/2 is unchanged" above: netty/netty#17774 makes |
Summary
ResponseBodyControl, stop decompressing its body. Decompressed body parts are at most 64 KiB, so a handler that suspends after each part receives one part perresume().Http1ContentDecompressorbecomes aChannelDuplexHandleron Netty's pull-basedDecompressor, instead of extending Netty'sHttpContentDecompressor. Codings, multi-member gzip, header rewrites and themaxDecompressedResponseSizelimit are kept.Problem
Suspending a response stops further socket reads, but Netty's
HttpContentDecompressorinflates 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:
maindelivered 1,955,380 bytes in 30 more parts past the suspending part;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
Http1ContentDecompressoruses Netty's pull-basedDecompressor(Netty 4.2.17+, AHC pins 4.2.18):100 Continuepass-through, and themaxDecompressedResponseSizeaccounting and message. The limit counts every gzip member.io.netty.compression.defaultMaxForwardBytesis 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.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.channelInactive, even to a suspended response, because Netty tears the pipeline down with the channel.ABORT, timeout), held input is dropped instead of decompressed.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 is unchanged
After
END_STREAM, Netty's HTTP/2 stream channel hands every queued frame over on the next read, regardless ofautoRead, 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 afterEND_STREAMand close only when drained) or AHC delivering the body independently of the stream channel.Behavior changes
-Dio.netty.noJdkZlibDecoderis no longer honored on HTTP/1.1; the JDK's zlib is always used.mainon the same inputs:main, 120 ms here).Compatibility
Http1ContentDecompressoris 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 (ChannelManagerinstantiates 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 inpom.xml, so any other change to the class is still flagged.DecompressorAPI as unstable (@UnstableApi) while it migrates its codecs to it (netty/netty#16743). A future Netty release may require changes to this handler.AI disclosure
Claude Code on behalf of Matthias Kurz. The commit includes
Co-Authored-ByperAGENTS.md.Test plan
New tests:
Http1DecompressionSuspensionTest(7 tests, live server):ABORT, andHEADand empty gzip responses.On
main, its two bound tests fail deterministically.Http1ContentDecompressorTest(19 tests,EmbeddedChannel):100 Continue;Http1ContentDecompressorLargeOutputTest(1 test):-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;Broken variants, each run against these 27 tests:
channelInactiveordering test.Runs:
Http1DecompressionLimitTest,AutomaticDecompressionTest,Http2ContentDecompressorTest,ResponseBodyControlTest,Expect100ContinueTestandLargeResponseTest) on JDK 11: 60 tests passed../mvnw -B -ntp -Dgpg.skip=true clean verifyon JDK 11: 1,818 tests, 0 failures, 0 errors, 22 skipped; Revapi passed. No test-skipping flags were used.-Djvm): 0 failures on each. The fresh-JVM test's child runs on the same JDK.HttpContentDecompressor(whatmainuses) 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 onmain, 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.Cookieheaders 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 installon JDK 11 passes with 1,853 tests, 0 failures, 0 errors, 22 skipped; Revapi passed.