FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Auto-port 5.0: Fix JdkZlibDecompressor losing the tail of highly compressible streams by netty-project-bot · Pull Request #17196 · netty/netty · GitHub

Repository navigation

Auto-port 5.0: Fix JdkZlibDecompressor losing the tail of highly compressible streams - #17196

Merged
normanmaurer merged 1 commit into
5.0from
auto-port-pr-17191-to-5.0
Aug 3, 2026
Merged

normanmaurer merged 1 commit into
5.0from
auto-port-pr-17191-to-5.0

Conversation

Copy link
Copy Markdown
Contributor

Auto-port of #17191 to 5.0
Cherry-picked commit: 7681aff


Motivation

JdkZlibDecompressor#processOutput sizes every output buffer as
inflater.getRemaining() << 1. The number of remaining input bytes is only a hint: the inflater
may still hold decoded data that did not fit into the previous output buffer, and by then it may
have pulled the last input bytes into its internal state, so getRemaining() is 0 and the
proposed buffer has zero capacity. inflate(...) cannot make progress with it, and because
status() maps inflater.needsInput() straight to NEED_INPUT, the caller is asked for more
input instead of draining the pending output. At the end of the stream endOfInput() then fails
with Compressed stream ended before the end-of-stream marker, even though the stream was
complete and valid.

Measured with 100000 bytes of 'a' deflated raw (115 bytes, ratio ~870:1), driving the
decompressor exactly as its own tests do:

new JdkZlibDecompressor (ZlibWrapper.NONE): 99848 of 100000 bytes, then
    DecompressionException: Compressed stream ended before the end-of-stream marker
legacy JdkZlibDecoder  (ZlibWrapper.NONE): 100000 bytes
java.util.zip.Inflater                   : 100000 bytes

Replaying the same buffer sizing against a bare Inflater shows the state that is mis-read:

iteration 2148: inflate returned 0 cap=0 getRemaining=0 needsInput=true finished=false producedSoFar=99848
drained with a 64K buffer afterwards: 152B, finished=true

So the tail is inside the Inflater the whole time and only the buffer sizing keeps it there.
The legacy JdkZlibDecoder is not affected because it inflates into a single buffer that it keeps
expanding, so inflate(...) always has room. ZlibWrapper.ZLIB and ZlibWrapper.GZIP hide the
problem as well, because their trailer keeps getRemaining() above zero until the inflater is
finished; raw deflate has no trailer, which is what permessage-deflate and
Content-Encoding: deflate payloads look like.

Modification

  • Never propose a zero-sized output buffer: processOutput floors the proposed capacity at
    MIN_OUTPUT_BUFFER_SIZE (512 bytes). maxAllocation still caps it exactly as before, so a
    configured limit keeps its meaning.
  • Remember whether the last inflate(...) filled the output buffer completely and, if it did,
    report NEED_OUTPUT from status() so the pending output is drained before more input is
    requested. needsInput() only says that all input bytes were consumed, not that all output was
    produced. The flag is cleared once a call does not fill the buffer or the inflater is finished,
    so this cannot loop.

Result

A valid deflate stream is fully decompressed regardless of its compression ratio.

Verification done: added JdkZlibDecompressorTest#testHighlyCompressibleStreamIsFullyDecompressed.
It runs for all three wrappers, cross-checks the compressed input with the JDK inflater first so a
failure cannot be blamed on the fixture, and then asserts the decompressor returns the same bytes.
It fails on unpatched 4.2 for ZlibWrapper.NONE with the exception above and passes with this
change. ./mvnw -pl codec-compression clean install is green on JDK 21 (499 tests, checkstyle,
forbidden-apis and revapi included).

I also ran the other Decompressor implementations through the same battery (round trip at chunk
sizes 1..64K, addInput ownership on malformed input, close() idempotency, concatenated frames,
and hand-crafted gzip headers covering every FLG combination against GZIPInputStream) and found
no other divergence, so this change is limited to the zlib one.

#17191)

### Motivation

`JdkZlibDecompressor#processOutput` sizes every output buffer as
`inflater.getRemaining() << 1`. The number of remaining input bytes is
only a hint: the inflater
may still hold decoded data that did not fit into the previous output
buffer, and by then it may
have pulled the last input bytes into its internal state, so
`getRemaining()` is `0` and the
proposed buffer has zero capacity. `inflate(...)` cannot make progress
with it, and because
`status()` maps `inflater.needsInput()` straight to `NEED_INPUT`, the
caller is asked for more
input instead of draining the pending output. At the end of the stream
`endOfInput()` then fails
with `Compressed stream ended before the end-of-stream marker`, even
though the stream was
complete and valid.

Measured with 100000 bytes of `'a'` deflated raw (115 bytes, ratio
~870:1), driving the
decompressor exactly as its own tests do:

```
new JdkZlibDecompressor (ZlibWrapper.NONE): 99848 of 100000 bytes, then
    DecompressionException: Compressed stream ended before the end-of-stream marker
legacy JdkZlibDecoder  (ZlibWrapper.NONE): 100000 bytes
java.util.zip.Inflater                   : 100000 bytes
```

Replaying the same buffer sizing against a bare `Inflater` shows the
state that is mis-read:

```
iteration 2148: inflate returned 0 cap=0 getRemaining=0 needsInput=true finished=false producedSoFar=99848
drained with a 64K buffer afterwards: 152B, finished=true
```

So the tail is inside the `Inflater` the whole time and only the buffer
sizing keeps it there.
The legacy `JdkZlibDecoder` is not affected because it inflates into a
single buffer that it keeps
expanding, so `inflate(...)` always has room. `ZlibWrapper.ZLIB` and
`ZlibWrapper.GZIP` hide the
problem as well, because their trailer keeps `getRemaining()` above zero
until the inflater is
finished; raw deflate has no trailer, which is what `permessage-deflate`
and
`Content-Encoding: deflate` payloads look like.

### Modification

- Never propose a zero-sized output buffer: `processOutput` floors the
proposed capacity at
`MIN_OUTPUT_BUFFER_SIZE` (512 bytes). `maxAllocation` still caps it
exactly as before, so a
  configured limit keeps its meaning.
- Remember whether the last `inflate(...)` filled the output buffer
completely and, if it did,
report `NEED_OUTPUT` from `status()` so the pending output is drained
before more input is
requested. `needsInput()` only says that all input bytes were consumed,
not that all output was
produced. The flag is cleared once a call does not fill the buffer or
the inflater is finished,
  so this cannot loop.

### Result

A valid deflate stream is fully decompressed regardless of its
compression ratio.

**Verification done:** added
`JdkZlibDecompressorTest#testHighlyCompressibleStreamIsFullyDecompressed`.
It runs for all three wrappers, cross-checks the compressed input with
the JDK inflater first so a
failure cannot be blamed on the fixture, and then asserts the
decompressor returns the same bytes.
It fails on unpatched `4.2` for `ZlibWrapper.NONE` with the exception
above and passes with this
change. `./mvnw -pl codec-compression clean install` is green on JDK 21
(499 tests, checkstyle,
forbidden-apis and revapi included).

I also ran the other `Decompressor` implementations through the same
battery (round trip at chunk
sizes 1..64K, `addInput` ownership on malformed input, `close()`
idempotency, concatenated frames,
and hand-crafted gzip headers covering every `FLG` combination against
`GZIPInputStream`) and found
no other divergence, so this change is limited to the zlib one.

(cherry picked from commit 7681aff)
normanmaurer merged commit 2b38263 into 5.0 Aug 3, 2026
13 checks passed
normanmaurer deleted the auto-port-pr-17191-to-5.0 branch August 3, 2026 13:48
chrisvest added this to the 5.0.0.Final milestone Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL