node:zlib: don't abort when the native handle is driven outside the zlib.ts lifecycle

[robobun]

What

The zlib.ts wrapper drives the NativeZlib / NativeBrotli / NativeZstd handles as constructor -> init() -> write*() -> close(), caches onerror and the write callback in init(), and nulls _handle on close. The native binding assumed that protocol. Driving a handle outside it, which only takes reaching _handle (and its constructor), aborts the whole process. Eleven distinct ways, all reproduced on the release binary; two are null-pointer segfaults and one needs nothing but new:

const zlib = require("node:zlib");

// 1) 2^32-byte dictionary via the PUBLIC API (no _handle needed):
zlib.deflateSync(Buffer.from("hi"), { dictionary: new Uint8Array(2 ** 32) });
// panic: int cast: TryFromIntError(PosOverflow)

// 2) constructing a handle and never initializing it; the GC finalizer
//    frees/resets native state that was never created:
new (zlib.createBrotliCompress()._handle.constructor)(8); Bun.gc(true);
// panic: called `Option::unwrap()` on a `None` value
new (zlib.createZstdCompress()._handle.constructor)(10); Bun.gc(true);
// panic(main thread): Segmentation fault at address 0xE38

// 3) writeSync() before init() (brotli/zstd): NULL state pointer into the C library
new (zlib.createZstdCompress()._handle.constructor)(10).writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64);
// panic(main thread): Segmentation fault at address 0xE38

// 4) writeSync() after close() (brotli/zstd):
const h = zlib.createBrotliCompress()._handle;
h.close(); h.writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64);
// panic: internal error: entered unreachable code

// 5) init() after close() (all three):
const h2 = zlib.createDeflate()._handle;
h2.close(); h2.init(15, 6, 8, 0, new Uint32Array(2), () => {}, undefined);
// panic: internal error: entered unreachable code

// 6) init() whose deflateInit2_ rejects the arguments, then init() again:
const h3 = new (zlib.createDeflate()._handle.constructor)(1);
h3.init(100, 6, 8, 0, new Uint32Array(2), () => {}, undefined);
h3.init(15, 6, 8, 0, new Uint32Array(2), () => {}, undefined);
// panic: internal error: entered unreachable code

// 7) any stream error with no cached onerror:
new (zlib.createDeflate()._handle.constructor)(1).reset();
// panic: Assertion failure: cachedErrorCallback is null in node:zlib binding

// 8) an async write completing with no cached writeCallback:
const h4 = new (zlib.createBrotliDecompress()._handle.constructor)(9);
h4.reset(); h4.write(0, null, 0, 0, new Uint8Array(64), 0, 64);
// panic: called `Option::unwrap()` on a `None` value

Plus two that are not aborts but are worse: init() or params() called while an async write() is still running on the thread pool mutates the same native z_stream / encoder from two threads (the worker holds &mut Context inside deflate() while the JS thread runs deflateInit2_ on it), and a brotli/zstd init() whose parameter setup fails tears the Context down without marking the handle closed, so the next call aborts anyway.

Why

• After close(), or after a failed init(), the Context is in NodeMode::NONE with its native state freed; Context::{do_work,init} and brotli's get_error_info have no arm for NONE and hit unreachable!(). reset() already guarded on closed; write, writeSync and init did not, and nothing guarded against an in-flight write.
• init() can fail three different ways without anything recording it: zlib's deflateInit2_/inflateInit2_ rejecting the arguments, and brotli/zstd's per-parameter setup failing after the state was created. All three leave { closed: false, mode: NONE }.
• Before init(), brotli/zstd have no encoder/decoder state at all. do_work() passes the null pointer into BrotliEncoderCompressStream / ZSTD_compressStream2, and the GC finalizer does the same through Context::close() (deinit_state unwraps the None; zstd hands the null CCtx to ZSTD_CCtx_reset). So a handle that is merely constructed and collected already aborts.
• NativeZlib::Context::close() asserted that deflateEnd/inflateEnd returned Z_OK/Z_DATA_ERROR; on a z_stream whose init never ran they return Z_STREAM_ERROR (with nothing to free), which fired the assertion in debug/ASAN builds.
• emit_error() and the async write-completion path unwrap() JS callbacks that only init() caches.
• NativeZlib::init() copied the dictionary into a Vec and only then did u32::try_from(dict.len()).expect("int cast"); JSC allows a 2^32-byte Uint8Array and deflateSetDictionary takes a uInt length.

Fix

• throw_if_closed is generalized into CompressionStream::throw_unless_idle (write_in_progress / pending_close / closed, in the shared mixin so all three stream types at once). write(), writeSync(), each class's init(), and NativeZlib::params() call it; write()/writeSync() previously carried their own copies of the first two checks.
• Every init() failure path now marks the handle closed: NativeZlib::init() checks whether Context::init ended in NodeMode::NONE, and the brotli/zstd parameter-failure branches set it after tearing the Context down.
• Brotli and zstd Context::close() early-return when the native state was never created. This also makes close() idempotent, which overlaps the user-called double-close() case that zlib: make brotli/zstd native handle close() idempotent #32382 fixes in the same function; it became required here because the GC finalizer goes through the same close().
• NativeZlib::Context::close()'s debug assertion also accepts Z_STREAM_ERROR.
• Brotli/Zstd Context::do_work() return early when the state was never created. NativeZlib already tolerates this (zlib's own deflateStateCheck null-checks the stream).
• emit_error() and the write-completion path skip the JS callback when none is cached instead of panicking; the pending reset/close handling still runs. Node behaves the same way.
• NativeZlib::init() rejects a dictionary longer than u32::MAX before copying it: RangeError [ERR_OUT_OF_RANGE]: The value of "dictionary.byteLength" is out of range. It must be <= 4294967295. Received 4294967296. With the check before the copy, a 2^32-byte Uint8Array argument is never read, so the test stays cheap.

Intentionally not touched here:

• brotli set_flush receiving zlib flush constants 4..=6 is zlib: accept zlib flush constants in brotli set_flush instead of panicking #32358 (whose fix, mapping them like Node does, is the right one).
• a second init() on an already-initialized, idle handle leaks the first encoder/internal_state. That is a bounded leak rather than an abort, Node has the same gap, and it needs its own leak-regression test; left as a follow-up.
• NativeZlib::params() after close() is already safe (Context::set_params has a _ => {} arm).

Test

test/js/node/zlib/zlib-handle-bounds-check.test.ts gains one subprocess test per case (21 new). The children run with BUN_DESTRUCT_VM_ON_EXIT=1 so the GC finalizers run deterministically on every lane, not only the ASAN lanes that destroy the VM at exit (which is how CI caught the finalizer class in the first place).

bun bd test test/js/node/zlib/zlib-handle-bounds-check.test.ts   # 30 pass
bun bd test test/js/node/zlib/zlib.test.js                       # 376 pass (2 pre-existing debug-only timeouts)
USE_SYSTEM_BUN=1 bun test <same file>                            # the new cases fail (aborts / wrong result)

The 14 test-zlib-{write-after-close,close-after-*,flush*,brotli*,params,dictionary*,reset-before-write,...} node-upstream tests still pass. The two streaming encode doesn't wait for entire input failures in zlib.test.js are pre-existing on debug+ASAN runners: the test's own byte-fill setup loop takes 42s against a 15s limit before any compression happens.

[robobun]

^{Updated 9:52 AM PT - Jun 26th, 2026}

❌ @robobun, your commit fa7abd9 has 3 failures in Build #65036 (All Failures):

• test/js/bun/terminal/terminal.test.ts - code 1 on 🍎 14 x64
• test/regression/issue/26030.test.ts - code 1 on 🐧 3.23 aarch64
• test/cli/install/bun-install.test.ts - code 1 on 🪟 11 aarch64
• test/cli/install/bun-install.test.ts - code 1 on 🪟 2019 x64-baseline
• test/cli/install/bun-install.test.ts - code 1 on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 32762

That installs a local version of the PR into your bun-32762 executable, so you can run:

bun-32762 --bun

[coderabbitai]

Walkthrough

The PR adds closed-state checks to zlib, brotli, and zstd init paths, guards Brotli and zstd work loops against missing state, makes zlib callback dispatch optional when callbacks are absent, validates zlib dictionary length, and adds subprocess-based tests.

Changes

Zlib lifecycle and bounds checks

Layer / File(s)	Summary
Closed init guard src/runtime/node/node_zlib_binding.rs, src/runtime/node/zlib/NativeZlib.rs, src/runtime/node/zlib/NativeBrotli.rs, src/runtime/node/zlib/NativeZstd.rs	CompressionStream<T> adds throw_if_closed, and the zlib, brotli, and zstd init() paths call it before continuing.
Write callbacks and error dispatch src/runtime/node/node_zlib_binding.rs	write and write_sync reject closed handles, run_from_js_thread skips missing write callbacks, and emit_error skips missing error callbacks.
Work state checks src/runtime/node/zlib/NativeBrotli.rs, src/runtime/node/zlib/NativeZstd.rs	Context::do_work returns early when the native Brotli or zstd state is None.
Dictionary bounds src/runtime/node/zlib/NativeZlib.rs	NativeZlib::init rejects dictionary buffers whose byteLength exceeds u32::MAX with a RangeError.
Lifecycle tests test/js/node/zlib/zlib-handle-bounds-check.test.ts	A concurrent subprocess suite covers closed-handle misuse across zlib, brotli, and zstd, and checks the oversized dictionary error case.

Suggested reviewers

• Jarred-Sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preventing native zlib handles from aborting outside the wrapper lifecycle.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description clearly explains the change and includes verification details, satisfying the repository template.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/runtime/node/node_zlib_binding.rs`:
- Around line 282-296: The guard logic in throw_if_closed is fine, but the doc
comment above it contains a broken trailing clause that does not read correctly.
Clean up the documentation text in that comment by removing or rephrasing the
malformed “which none of its `mode` matches handle” fragment so the explanation
of closed Context state is grammatically correct and concise.

In `@test/js/node/zlib/zlib-handle-bounds-check.test.ts`:
- Around line 160-168: The run helper in zlib-handle-bounds-check.test.ts is
leaving Bun.spawn’s stderr pipe unread, which can deadlock when the child emits
large diagnostics. Update the run function to drain stderr concurrently with
stdout and exited, using the existing Bun.spawn proc object in run so all pipes
are consumed before returning. Keep the current stdout/exitCode result shape and
the existing assertions unchanged, but make sure stderr is actively read rather
than ignored.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b08cfbc9-4c5c-4577-9ab2-7de6a8f8284a

📥 Commits

Reviewing files that changed from the base of the PR and between 0589548 and 08f0dbd.

📒 Files selected for processing (5)

• src/runtime/node/node_zlib_binding.rs
• src/runtime/node/zlib/NativeBrotli.rs
• src/runtime/node/zlib/NativeZlib.rs
• src/runtime/node/zlib/NativeZstd.rs
• test/js/node/zlib/zlib-handle-bounds-check.test.ts

[robobun]

CI status note for reviewers: the real failure CI ever found on this PR was its own doing, and it is fixed. The x64-asan lane destroys the VM at exit, which ran the GC finalizer for the never-initialized handles the new tests create and exposed the finalizer aborts described above; those are fixed and the test children now set BUN_DESTRUCT_VM_ON_EXIT=1 so every lane exercises them.

Since that fix, test/js/node/zlib/zlib-handle-bounds-check.test.ts has not appeared in any build's failures. The remaining red across builds 65005 and 65036 is confined to lanes and files this diff does not touch (:darwin: 26 aarch64 with serve-body-leak / node-http-uaf timeouts, bun-install.test.ts, update_interactive_install.test.ts, sql-mysql.transactions container startup), and main's latest build is green on the same lanes. The diff is ready for review.