Skip to content

zlib: validate flush kind for brotli streams#63746

Open
Ic3b3rg wants to merge 2 commits into
nodejs:mainfrom
Ic3b3rg:fix/zlib-brotli-flush-invalid-kind
Open

zlib: validate flush kind for brotli streams#63746
Ic3b3rg wants to merge 2 commits into
nodejs:mainfrom
Ic3b3rg:fix/zlib-brotli-flush-invalid-kind

Conversation

@Ic3b3rg

@Ic3b3rg Ic3b3rg commented Jun 4, 2026

Copy link
Copy Markdown

Summary

BrotliCompress.flush(kind) and BrotliDecompress.flush(kind) accepted numeric kind values that are not valid Brotli operations and forwarded them to the native Brotli encoder/decoder.

Passing zlib flush constants such as Z_FINISH (4) or Z_BLOCK (5) could make the process spin at 100% CPU indefinitely, because Brotli only supports operation values in the range 0..3.

This PR validates kind for Brotli streams before queuing the fake flush chunk. Numeric values outside the Brotli operation range now throw ERR_OUT_OF_RANGE, matching the existing options.flush and options.finishFlush validation path.

Scope

The validation is intentionally limited to Brotli streams. zlib and zstd stream flush() behavior is unchanged.

Valid Brotli operations continue to work:

  • BROTLI_OPERATION_PROCESS (0)
  • BROTLI_OPERATION_FLUSH (1)
  • BROTLI_OPERATION_FINISH (2)
  • BROTLI_OPERATION_EMIT_METADATA (3)

Observable behavior changes

Call Before After
flush(4) / Z_FINISH Hangs / spins at 100% CPU Throws ERR_OUT_OF_RANGE
flush(5) / Z_BLOCK Hangs / spins at 100% CPU Throws ERR_OUT_OF_RANGE
flush(6) Throws incidentally from the fake-chunk path Throws ERR_OUT_OF_RANGE during validation
flush(0..3) Works Works
flush() / flush(cb) Works, uses default Brotli flush operation Works, uses default Brotli flush operation

Error type

ERR_OUT_OF_RANGE is intentional for invalid numeric values: kind has the correct JavaScript type, but the value is outside the valid Brotli operation range. Non-number values should continue to use the existing type-validation behavior.

Cross-runtime note

Bun is fixing the same bug on their side (oven-sh/bun#31505) but plans to throw ERR_INVALID_ARG_TYPE to match the current incidental behaviour of flush(6) on Node. This PR chooses ERR_OUT_OF_RANGE for internal consistency with the checkRangesOrGetDefault validation already used elsewhere in lib/zlib.js.

Fixes: #63701

BrotliCompress/Decompress.flush(kind) forwarded any value to the native
layer, causing a 100% CPU hang for kinds outside the brotli operation
range (e.g. Z_FINISH). Validate against [0, 3] and throw
ERR_OUT_OF_RANGE on invalid input.

Fixes: nodejs#63701
Signed-off-by: Ic3b3rg <s.ceccarini94@gmail.com>
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem. labels Jun 4, 2026
Comment thread lib/zlib.js Outdated
Signed-off-by: Ic3b3rg <s.ceccarini94@gmail.com>
@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 5, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 5, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@addaleax addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 5, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 5, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@alii

alii commented Jun 5, 2026

Copy link
Copy Markdown

WTBU that ERR_INVALID_ARG_TYPE is currently thrown for flush(6) but this PR uses ERR_OUT_OF_RANGE. Is that the desired behaviour? oven-sh/bun#31505 currently implemented ERR_INVALID_ARG_TYPE, but of course happy to switch before either of these merge.

@addaleax

Copy link
Copy Markdown
Member

@Ic3b3rg Any thoughts on the most recent comment here?

@Ic3b3rg

Ic3b3rg commented Jun 16, 2026

Copy link
Copy Markdown
Author

@alii The previous ERR_INVALID_ARG_TYPE for flush(6) was incidental: 6 fell through to the fake flush chunk path, where kFlushBuffers[6] was undefined, and that later became write(undefined). With explicit validation, kind is a number but outside the valid operation range, so ERR_OUT_OF_RANGE matches the existing checkRangesOrGetDefault() behavior used for options.flush and options.finishFlush.

Also, after @addaleax’s earlier suggestion, this PR now validates flush(kind) for all zlib-style streams, not only Brotli. I’ll update the PR description to reflect that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zlib: BrotliCompress.flush() with Z_FINISH or Z_BLOCK hangs at 100% CPU

4 participants