From 08f0dbd1df76b0e9112729e37a8ae7b0b62b16ed Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:16:33 +0000 Subject: [PATCH 1/4] node:zlib: don't abort when the native handle is driven outside the zlib.ts lifecycle The zlib.ts wrapper drives NativeZlib/NativeBrotli/NativeZstd as constructor -> init() -> write*() -> close(), caches the onerror and writeCallback JS functions in init(), and nulls `_handle` on close. The native binding assumed that protocol; a handle driven outside it (reachable from JS through `_handle` and `_handle.constructor`) aborted the whole process with a Rust panic, or handed a null state pointer straight into brotli/zstd. - CompressionStream::throw_if_closed rejects write()/writeSync()/init() on a closed handle (the Context is in NodeMode::NONE, which the mode matches in do_work()/Context::init() treat as unreachable). reset() already had this check. - Brotli/Zstd Context::do_work() returns early when the native state was never created: BrotliEncoderCompressStream / ZSTD_compressStream2 dereference the state pointer unconditionally. - emit_error() and the async write-completion path skip the JS callback when none is cached instead of panicking. The pending reset/close handling still runs. - NativeZlib::init() rejects a dictionary longer than u32::MAX before copying it; deflateSetDictionary takes a uInt length. Brotli/Zstd Context::close() idempotency is #32382 and brotli set_flush with zlib flush constants is #32358; both are intentionally not touched here. --- src/runtime/node/node_zlib_binding.rs | 54 +++++--- src/runtime/node/zlib/NativeBrotli.rs | 8 ++ src/runtime/node/zlib/NativeZlib.rs | 15 ++ src/runtime/node/zlib/NativeZstd.rs | 8 ++ .../zlib/zlib-handle-bounds-check.test.ts | 131 ++++++++++++++++++ 5 files changed, 198 insertions(+), 18 deletions(-) diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index 7e0491912b00..a7f46ade01c5 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -279,6 +279,21 @@ pub(crate) trait CompressionStreamImpl: Sized + Taskable + 'static { } impl CompressionStream { + /// Rejects a call on a handle whose `Context` was already torn down by + /// `close()`. A closed `Context` is in `NodeMode::NONE` with its native + /// state freed, which none of its `mode` matches handle. + pub(crate) fn throw_if_closed(this: &T, global_this: &JSGlobalObject) -> JsResult<()> { + if this.closed().get() { + return Err(global_this + .err( + ErrorCode::INVALID_STATE, + format_args!("zlib binding closed"), + ) + .throw()); + } + Ok(()) + } + pub(crate) fn write( this: &T, global_this: &JSGlobalObject, @@ -388,6 +403,7 @@ impl CompressionStream { .err(ErrorCode::INVALID_STATE, format_args!("Pending close")) .throw()); } + Self::throw_if_closed(this, global_this)?; // Pin both buffers before mutating any state: materializing a // FastTypedArray's backing store can fail on OOM, and failing here // leaves nothing to unwind. @@ -540,10 +556,12 @@ impl CompressionStream { this.flush_write_result(global, this_value); this_value.ensure_still_alive(); - let write_callback: JSValue = T::write_callback_get_cached(this_value).unwrap(); - - vm.event_loop_ref() - .run_callback(write_callback, global, this_value, &[]); + // `init()` caches the JS write callback; a handle whose `init()` was + // never called has none, so there is nothing to notify. + if let Some(write_callback) = T::write_callback_get_cached(this_value) { + vm.event_loop_ref() + .run_callback(write_callback, global, this_value, &[]); + } if this.pending_reset().get() { Self::reset_internal(&this, global, this_value); @@ -677,6 +695,7 @@ impl CompressionStream { .err(ErrorCode::INVALID_STATE, format_args!("Pending close")) .throw()); } + Self::throw_if_closed(this, global_this)?; this.write_in_progress().set(true); this.ref_(); @@ -827,20 +846,19 @@ impl CompressionStream { Err(_) => return, }; - let callback: JSValue = T::error_callback_get_cached(this_value).unwrap_or_else(|| { - bun_core::Output::panic(format_args!( - "Assertion failure: cachedErrorCallback is null in node:zlib binding", - )) - }); - - // SAFETY: `bun_vm()` and `event_loop()` are non-null for a Bun-owned global. - let vm = global_this.bun_vm(); - vm.event_loop_ref().run_callback( - callback, - global_this, - this_value, - &[msg_value, err_value, code_value], - ); + // The `zlib.ts` wrapper installs `onerror` right after construction, + // but a handle driven directly has none; there is nobody to notify. + // The pending reset/close handling below still runs either way. + if let Some(callback) = T::error_callback_get_cached(this_value) { + // SAFETY: `bun_vm()` and `event_loop()` are non-null for a Bun-owned global. + let vm = global_this.bun_vm(); + vm.event_loop_ref().run_callback( + callback, + global_this, + this_value, + &[msg_value, err_value, code_value], + ); + } if this.pending_reset().get() { Self::reset_internal(this, global_this, this_value); diff --git a/src/runtime/node/zlib/NativeBrotli.rs b/src/runtime/node/zlib/NativeBrotli.rs index 95a1b87e4554..2520b5a1946b 100644 --- a/src/runtime/node/zlib/NativeBrotli.rs +++ b/src/runtime/node/zlib/NativeBrotli.rs @@ -189,6 +189,9 @@ mod _impl { ) .throw()); } + // A closed `Context` is in `NodeMode::NONE`, which `Context::init` + // cannot re-initialize. + CompressionStream::::throw_if_closed(self, global_this)?; // `flush_write_result` writes two u32s into this array, so the // caller-supplied array must hold at least 2 elements. @@ -429,6 +432,11 @@ mod _impl { } pub fn do_work(&mut self) { + // A handle driven before `init()` has no encoder/decoder state; + // brotli dereferences the state pointer unconditionally. + if self.state.is_none() { + return; + } match self.mode { bun_zlib::NodeMode::BROTLI_ENCODE => { let mut next_in = self.next_in; diff --git a/src/runtime/node/zlib/NativeZlib.rs b/src/runtime/node/zlib/NativeZlib.rs index f452a45204ed..dbe661a0907c 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -130,6 +130,9 @@ mod _impl { ) .throw()); } + // A closed `Context` is in `NodeMode::NONE`, which `Context::init` + // cannot re-initialize. + CompressionStream::::throw_if_closed(self, global)?; let window_bits = validators::validate_int32(global, arguments.ptr[0], "windowBits", None, None)?; @@ -183,6 +186,18 @@ mod _impl { )); } }; + // `deflateSetDictionary`/`inflateSetDictionary` take a `uInt` + // length; reject anything larger before `Context::init` copies it. + if dictionary_buf.byte_len > u32::MAX as usize { + return Err(global.throw_range_error( + dictionary_buf.byte_len as i64, + bun_jsc::RangeErrorOptions { + field_name: b"dictionary.byteLength", + max: i64::from(u32::MAX), + ..Default::default() + }, + )); + } Some(dictionary_buf.byte_slice()) }; diff --git a/src/runtime/node/zlib/NativeZstd.rs b/src/runtime/node/zlib/NativeZstd.rs index 2e6227071738..5df21f63ede3 100644 --- a/src/runtime/node/zlib/NativeZstd.rs +++ b/src/runtime/node/zlib/NativeZstd.rs @@ -152,6 +152,9 @@ mod _impl { ) .throw()); } + // A closed `Context` is in `NodeMode::NONE`, which `Context::init` + // cannot re-initialize. + CompressionStream::::throw_if_closed(self, global)?; let init_params_array_value = arguments[0]; let pledged_src_size_value = arguments[1]; @@ -420,6 +423,11 @@ mod _impl { } pub fn do_work(&mut self) { + // A handle driven before `init()` has no CCtx/DCtx; zstd + // dereferences the context pointer unconditionally. + if self.state.is_none() { + return; + } self.remaining = match self.mode { // SAFETY: state is a valid CCtx; input/output point to caller-kept-alive buffers (set_buffers). NodeMode::ZSTD_COMPRESS => unsafe { diff --git a/test/js/node/zlib/zlib-handle-bounds-check.test.ts b/test/js/node/zlib/zlib-handle-bounds-check.test.ts index ffac59ab607c..fe12bb48f991 100644 --- a/test/js/node/zlib/zlib-handle-bounds-check.test.ts +++ b/test/js/node/zlib/zlib-handle-bounds-check.test.ts @@ -146,3 +146,134 @@ describe("zlib native handle writeState", () => { expect(exitCode).toBe(0); }); }); + +// The zlib.ts wrapper always drives the native handle as +// constructor -> init() -> write*() -> close(), caches onerror/writeCallback +// in init(), and nulls `_handle` on close. The native binding assumed that +// protocol: driving a handle outside it (reachable through `_handle` and +// `_handle.constructor`) used to abort the whole process with a Rust +// `unreachable!()` / `unwrap()` on `None`, or hand a null state pointer +// straight into brotli/zstd. Each case runs in a subprocess so a regression +// fails one test instead of taking down the runner. `handled` means the child +// reached the statement after the call; `threw ...` echoes the JS error. +describe.concurrent("zlib native handle driven outside the zlib.ts lifecycle", () => { + async function run(body: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `const zlib = require("node:zlib");\n${body}`], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + return { stdout: stdout.trim(), exitCode }; + } + + const CLOSED = "threw ERR_INVALID_STATE: zlib binding closed"; + const cases: [name: string, body: string, expected: string][] = [ + // init() after close(): the Context is in NodeMode::NONE, which + // Context::init treats as unreachable. + [ + "zlib: init() after close() throws", + `const h = zlib.createDeflate()._handle; + h.close(); + try { h.init(15, 6, 8, 0, new Uint32Array(2), () => {}, undefined); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + CLOSED, + ], + [ + "brotli: init() after close() throws", + `const h = zlib.createBrotliCompress()._handle; + h.close(); + try { h.init(new Uint32Array(0), new Uint32Array(2), () => {}); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + CLOSED, + ], + [ + "zstd: init() after close() throws", + `const h = zlib.createZstdCompress()._handle; + h.close(); + try { h.init(new Uint32Array(0), undefined, new Uint32Array(2), () => {}); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + CLOSED, + ], + // write/writeSync after close(): brotli/zstd do_work() treated + // NodeMode::NONE as unreachable; zlib silently no-op'd on an ended stream. + [ + "zlib: writeSync() after close() throws", + `const h = zlib.createDeflate()._handle; + h.close(); + try { h.writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + CLOSED, + ], + [ + "brotli: writeSync() after close() throws", + `const h = zlib.createBrotliCompress()._handle; + h.close(); + try { h.writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + CLOSED, + ], + [ + "zstd: writeSync() after close() throws", + `const h = zlib.createZstdCompress()._handle; + h.close(); + try { h.writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + CLOSED, + ], + // writeSync() before init(): brotli/zstd were handed a null state + // pointer, which their C APIs dereference unconditionally. + [ + "brotli: writeSync() before init() does not dereference a null encoder", + `const C = zlib.createBrotliCompress()._handle.constructor; + new C(8).writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64); console.log("handled");`, + "handled", + ], + [ + "zstd: writeSync() before init() does not dereference a null CCtx", + `const C = zlib.createZstdCompress()._handle.constructor; + new C(10).writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64); console.log("handled");`, + "handled", + ], + // With no onerror / writeCallback cached (init() never ran), an error or + // an async write completion had no callback to unwrap. + [ + "zlib: an error with no onerror cached is dropped, not fatal", + `const C = zlib.createDeflate()._handle.constructor; + new C(1).reset(); console.log("handled");`, + "handled", + ], + [ + "brotli: async write() with no writeCallback cached completes", + `const C = zlib.createBrotliDecompress()._handle.constructor; + const h = new C(9); + h.reset(); + h.write(0, null, 0, 0, new Uint8Array(64), 0, 64); + console.log("handled");`, + "handled", + ], + ]; + + for (const [name, body, expected] of cases) { + test.concurrent(name, async () => { + expect(await run(body)).toEqual({ stdout: expected, exitCode: 0 }); + }); + } + + // deflateSetDictionary / inflateSetDictionary take a `uInt` length; a 2**32 + // byte dictionary overflowed the cast after the native handle had already + // copied it. The length is now rejected before the copy, so the Uint8Array + // below is never read and stays virtual (cheap). + test.concurrent("zlib: a 2**32-byte dictionary throws instead of overflowing a u32", async () => { + expect( + await run( + `try { zlib.deflateSync(Buffer.from("hello"), { dictionary: new Uint8Array(2 ** 32) }); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + ), + ).toEqual({ + stdout: + 'threw ERR_OUT_OF_RANGE: The value of "dictionary.byteLength" is out of range. It must be <= 4294967295. Received 4294967296', + exitCode: 0, + }); + }); +}); From 0b84ca02310e790278774caa348c8fe31af15f2e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 26 Jun 2026 13:27:07 +0000 Subject: [PATCH 2/4] review: drain the child stderr pipe; reword the throw_if_closed doc comment --- src/runtime/node/node_zlib_binding.rs | 4 ++-- test/js/node/zlib/zlib-handle-bounds-check.test.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index a7f46ade01c5..b93aa9260412 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -280,8 +280,8 @@ pub(crate) trait CompressionStreamImpl: Sized + Taskable + 'static { impl CompressionStream { /// Rejects a call on a handle whose `Context` was already torn down by - /// `close()`. A closed `Context` is in `NodeMode::NONE` with its native - /// state freed, which none of its `mode` matches handle. + /// `close()`: its native state is freed and its `mode` is `NodeMode::NONE`, + /// which `Context::do_work` and `Context::init` have no arm for. pub(crate) fn throw_if_closed(this: &T, global_this: &JSGlobalObject) -> JsResult<()> { if this.closed().get() { return Err(global_this diff --git a/test/js/node/zlib/zlib-handle-bounds-check.test.ts b/test/js/node/zlib/zlib-handle-bounds-check.test.ts index fe12bb48f991..9d6d41ab0769 100644 --- a/test/js/node/zlib/zlib-handle-bounds-check.test.ts +++ b/test/js/node/zlib/zlib-handle-bounds-check.test.ts @@ -163,7 +163,9 @@ describe.concurrent("zlib native handle driven outside the zlib.ts lifecycle", ( env: bunEnv, stderr: "pipe", }); - const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]); + // stderr is drained so a large diagnostic can't fill the pipe and block + // the child, but it isn't asserted on (debug/ASAN builds write to it). + const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); return { stdout: stdout.trim(), exitCode }; } From cec22804c81caccf8144806af2c794da78825e69 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 26 Jun 2026 14:09:16 +0000 Subject: [PATCH 3/4] node:zlib: reject init/params on a busy handle; survive finalizing a never-initialized one The ASAN CI lane tears the VM down at exit, running every GC finalizer. Once write/reset/init stop aborting early, a handle whose native state was never created reaches the finalizer, which had its own aborts: - Brotli/Zstd Context::close() on a never-created `state`: brotli's deinit_state unwraps the `None`; zstd hands the null CCtx straight to ZSTD_CCtx_reset. Both now early-return. This also makes close() idempotent (the user-called double-close #32382 fixes). - NativeZlib Context::close() asserted that deflateEnd accepted the stream; on a z_stream whose init never ran it returns Z_STREAM_ERROR with nothing to free, so the assertion allows it. Two more gaps in the same surface from review: - init() (all three classes) and NativeZlib::params() only rejected a *closed* handle, so calling them while an async write() still held the Context on a worker thread mutated the same native stream from two threads. `throw_if_closed` becomes `throw_unless_idle` (write_in_progress / pending_close / closed); write()/writeSync() use it too instead of carrying their own copies of the first two checks. - A brotli/zstd init() whose parameter setup fails tears the Context down without marking the handle closed, so the next writeSync()/init() still aborted. That path now sets `closed`. The test children run with BUN_DESTRUCT_VM_ON_EXIT=1 so every finalizer runs deterministically, not only on the lanes that destroy the VM. --- src/runtime/node/node_zlib_binding.rs | 51 ++++------- src/runtime/node/zlib/NativeBrotli.rs | 15 ++- src/runtime/node/zlib/NativeZlib.rs | 18 +++- src/runtime/node/zlib/NativeZstd.rs | 15 ++- .../zlib/zlib-handle-bounds-check.test.ts | 91 ++++++++++++++++++- 5 files changed, 146 insertions(+), 44 deletions(-) diff --git a/src/runtime/node/node_zlib_binding.rs b/src/runtime/node/node_zlib_binding.rs index b93aa9260412..eb44d50f7033 100644 --- a/src/runtime/node/node_zlib_binding.rs +++ b/src/runtime/node/node_zlib_binding.rs @@ -279,10 +279,23 @@ pub(crate) trait CompressionStreamImpl: Sized + Taskable + 'static { } impl CompressionStream { - /// Rejects a call on a handle whose `Context` was already torn down by - /// `close()`: its native state is freed and its `mode` is `NodeMode::NONE`, - /// which `Context::do_work` and `Context::init` have no arm for. - pub(crate) fn throw_if_closed(this: &T, global_this: &JSGlobalObject) -> JsResult<()> { + /// Rejects a call on a handle that cannot accept a new operation: an async + /// write still holds `&mut Context` on a worker thread, a pending close is + /// about to tear it down, or a closed one already did (`mode` is `NONE`). + pub(crate) fn throw_unless_idle(this: &T, global_this: &JSGlobalObject) -> JsResult<()> { + if this.write_in_progress().get() { + return Err(global_this + .err( + ErrorCode::INVALID_STATE, + format_args!("Write already in progress"), + ) + .throw()); + } + if this.pending_close().get() { + return Err(global_this + .err(ErrorCode::INVALID_STATE, format_args!("Pending close")) + .throw()); + } if this.closed().get() { return Err(global_this .err( @@ -390,20 +403,7 @@ impl CompressionStream { } let _ = (in_off, in_len, out_off, out_len); - if this.write_in_progress().get() { - return Err(global_this - .err( - ErrorCode::INVALID_STATE, - format_args!("Write already in progress"), - ) - .throw()); - } - if this.pending_close().get() { - return Err(global_this - .err(ErrorCode::INVALID_STATE, format_args!("Pending close")) - .throw()); - } - Self::throw_if_closed(this, global_this)?; + Self::throw_unless_idle(this, global_this)?; // Pin both buffers before mutating any state: materializing a // FastTypedArray's backing store can fail on OOM, and failing here // leaves nothing to unwind. @@ -682,20 +682,7 @@ impl CompressionStream { ); let _ = (in_off, in_len, out_off, out_len); - if this.write_in_progress().get() { - return Err(global_this - .err( - ErrorCode::INVALID_STATE, - format_args!("Write already in progress"), - ) - .throw()); - } - if this.pending_close().get() { - return Err(global_this - .err(ErrorCode::INVALID_STATE, format_args!("Pending close")) - .throw()); - } - Self::throw_if_closed(this, global_this)?; + Self::throw_unless_idle(this, global_this)?; this.write_in_progress().set(true); this.ref_(); diff --git a/src/runtime/node/zlib/NativeBrotli.rs b/src/runtime/node/zlib/NativeBrotli.rs index 2520b5a1946b..53c8540d894a 100644 --- a/src/runtime/node/zlib/NativeBrotli.rs +++ b/src/runtime/node/zlib/NativeBrotli.rs @@ -189,9 +189,9 @@ mod _impl { ) .throw()); } - // A closed `Context` is in `NodeMode::NONE`, which `Context::init` - // cannot re-initialize. - CompressionStream::::throw_if_closed(self, global_this)?; + // Racing an in-flight async write would alias `&mut Context` + // across threads; a closed `Context` cannot be re-initialized. + CompressionStream::::throw_unless_idle(self, global_this)?; // `flush_write_result` writes two u32s into this array, so the // caller-supplied array must hold at least 2 elements. @@ -268,6 +268,9 @@ mod _impl { if err.is_error() { // impl.emitError(this, globalThis, this_value, err); //XXX: onerror isn't set yet self.stream.with_mut(|s| s.close()); + // The Context is torn down (`mode` is `NONE`); reject any + // further operation the way `close()` does. + self.closed.set(true); return Ok(JSValue::FALSE); } } @@ -529,7 +532,11 @@ mod _impl { } pub fn close(&mut self) { - self.deinit_state(); + // Idempotent: a handle that was never (successfully) initialized, + // or that was already closed, has no encoder/decoder to free. + if self.state.is_some() { + self.deinit_state(); + } self.mode = bun_zlib::NodeMode::NONE; } diff --git a/src/runtime/node/zlib/NativeZlib.rs b/src/runtime/node/zlib/NativeZlib.rs index dbe661a0907c..7bd40219cd82 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -130,9 +130,9 @@ mod _impl { ) .throw()); } - // A closed `Context` is in `NodeMode::NONE`, which `Context::init` - // cannot re-initialize. - CompressionStream::::throw_if_closed(self, global)?; + // Racing an in-flight async write would alias `&mut Context` + // across threads; a closed `Context` cannot be re-initialized. + CompressionStream::::throw_unless_idle(self, global)?; let window_bits = validators::validate_int32(global, arguments.ptr[0], "windowBits", None, None)?; @@ -226,6 +226,9 @@ mod _impl { ) .throw()); } + // `set_params` calls `deflateParams` on the same `z_stream` an + // in-flight async write's `deflate()` is using. + CompressionStream::::throw_unless_idle(self, global)?; let level = validators::validate_int32(global, arguments.ptr[0], "level", None, None)?; let strategy = @@ -624,7 +627,14 @@ impl Context { BROTLI_ENCODE | BROTLI_DECODE => {} ZSTD_COMPRESS | ZSTD_DECOMPRESS => {} } - debug_assert!(status == c::ReturnCode::Ok || status == c::ReturnCode::DataError); + // Ok: normal. DataError: pending output discarded by inflateEnd. + // StreamError: a handle whose init() threw before deflateInit2_/ + // inflateInit2_ ran, so the zeroed z_stream has nothing to free. + debug_assert!( + status == c::ReturnCode::Ok + || status == c::ReturnCode::DataError + || status == c::ReturnCode::StreamError + ); self.mode = NONE; } } diff --git a/src/runtime/node/zlib/NativeZstd.rs b/src/runtime/node/zlib/NativeZstd.rs index 5df21f63ede3..287f782dcc37 100644 --- a/src/runtime/node/zlib/NativeZstd.rs +++ b/src/runtime/node/zlib/NativeZstd.rs @@ -152,9 +152,9 @@ mod _impl { ) .throw()); } - // A closed `Context` is in `NodeMode::NONE`, which `Context::init` - // cannot re-initialize. - CompressionStream::::throw_if_closed(self, global)?; + // Racing an in-flight async write would alias `&mut Context` + // across threads; a closed `Context` cannot be re-initialized. + CompressionStream::::throw_unless_idle(self, global)?; let init_params_array_value = arguments[0]; let pledged_src_size_value = arguments[1]; @@ -236,6 +236,9 @@ mod _impl { .with_mut(|s| s.set_params(c_uint::try_from(i).expect("int cast"), x)); if err_.is_error() { self.stream.with_mut(|s| s.close()); + // The Context is torn down (`mode` is `NONE`); reject any + // further operation the way `close()` does. + self.closed.set(true); // SAFETY: is_error() ⇔ msg is non-null; it points at a NUL-terminated C string. let msg = unsafe { bun_core::ffi::cstr(err_.msg) }.to_bytes(); return Err(global @@ -522,6 +525,12 @@ mod _impl { } pub fn close(&mut self) { + // Idempotent: a handle that was never (successfully) initialized, + // or that was already closed, has no CCtx/DCtx to reset or free. + if self.state.is_none() { + self.mode = NodeMode::NONE; + return; + } let _ = match self.mode { // SAFETY: state is a valid CCtx/DCtx for this mode. NodeMode::ZSTD_COMPRESS => unsafe { diff --git a/test/js/node/zlib/zlib-handle-bounds-check.test.ts b/test/js/node/zlib/zlib-handle-bounds-check.test.ts index 9d6d41ab0769..f18f7d5ff1ac 100644 --- a/test/js/node/zlib/zlib-handle-bounds-check.test.ts +++ b/test/js/node/zlib/zlib-handle-bounds-check.test.ts @@ -160,7 +160,11 @@ describe.concurrent("zlib native handle driven outside the zlib.ts lifecycle", ( async function run(body: string) { await using proc = Bun.spawn({ cmd: [bunExe(), "-e", `const zlib = require("node:zlib");\n${body}`], - env: bunEnv, + // BUN_DESTRUCT_VM_ON_EXIT makes exit run `lastChanceToFinalize`, so the + // finalizer of every handle the case leaves behind runs deterministically + // (the ASAN CI lanes do this on every exit; without it the child "passes" + // and then aborts only on those lanes). + env: { ...bunEnv, BUN_DESTRUCT_VM_ON_EXIT: "1" }, stderr: "pipe", }); // stderr is drained so a large diagnostic can't fill the pipe and block @@ -254,6 +258,91 @@ describe.concurrent("zlib native handle driven outside the zlib.ts lifecycle", ( console.log("handled");`, "handled", ], + // A constructed-but-never-initialized handle reaching the GC finalizer: + // brotli/zstd Context::close() tried to free/reset a state that was never + // created, and zlib's asserted that deflateEnd accepted the zeroed stream. + [ + "zlib: a never-initialized handle finalizes cleanly", + `const C = zlib.createDeflate()._handle.constructor; + new C(1); console.log("handled");`, + "handled", + ], + [ + "brotli: a never-initialized handle finalizes cleanly", + `const C = zlib.createBrotliCompress()._handle.constructor; + new C(8); console.log("handled");`, + "handled", + ], + [ + "zstd: a never-initialized handle finalizes cleanly", + `const C = zlib.createZstdCompress()._handle.constructor; + new C(10); console.log("handled");`, + "handled", + ], + // init()/params() while an async write() is still running on the thread + // pool: both sides would mutate the same native stream concurrently. + [ + "zlib: init() while an async write is in flight throws", + `const C = zlib.createDeflate()._handle.constructor; + const h = new C(1); + h.init(15, 6, 8, 0, new Uint32Array(2), () => {}, undefined); + h.write(0, null, 0, 0, new Uint8Array(64), 0, 64); + try { h.init(15, 6, 8, 0, new Uint32Array(2), () => {}, undefined); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + "threw ERR_INVALID_STATE: Write already in progress", + ], + [ + "zlib: params() while an async write is in flight throws", + `const C = zlib.createDeflate()._handle.constructor; + const h = new C(1); + h.init(15, 6, 8, 0, new Uint32Array(2), () => {}, undefined); + h.write(0, null, 0, 0, new Uint8Array(64), 0, 64); + try { h.params(1, 0); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + "threw ERR_INVALID_STATE: Write already in progress", + ], + [ + "brotli: init() while an async write is in flight throws", + `const C = zlib.createBrotliCompress()._handle.constructor; + const h = new C(8); + h.init(new Uint32Array(0), new Uint32Array(2), () => {}); + h.write(0, null, 0, 0, new Uint8Array(64), 0, 64); + try { h.init(new Uint32Array(0), new Uint32Array(2), () => {}); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + "threw ERR_INVALID_STATE: Write already in progress", + ], + [ + "zstd: init() while an async write is in flight throws", + `const C = zlib.createZstdCompress()._handle.constructor; + const h = new C(10); + h.init(new Uint32Array(0), undefined, new Uint32Array(2), () => {}); + h.write(0, null, 0, 0, new Uint8Array(64), 0, 64); + try { h.init(new Uint32Array(0), undefined, new Uint32Array(2), () => {}); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + "threw ERR_INVALID_STATE: Write already in progress", + ], + // init() that creates the native state but then fails on a bad parameter + // key tears the Context down; the handle has to reject further use. + [ + "brotli: a handle whose init() parameters were rejected is closed", + `const C = zlib.createBrotliCompress()._handle.constructor; + const h = new C(8); + const p = new Uint32Array(50).fill(0xffffffff); p[49] = 0; + const r = h.init(p, new Uint32Array(2), () => {}); + try { h.writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64); console.log("handled " + r); } + catch (e) { console.log("threw " + e.code + ": " + e.message + " " + r); }`, + "threw ERR_INVALID_STATE: zlib binding closed false", + ], + [ + "zstd: a handle whose init() parameters were rejected is closed", + `const C = zlib.createZstdCompress()._handle.constructor; + const h = new C(10); + const p = new Uint32Array(50).fill(0xffffffff); p[49] = 0; + try { h.init(p, undefined, new Uint32Array(2), () => {}); } catch {} + try { h.writeSync(0, null, 0, 0, new Uint8Array(64), 0, 64); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + "threw ERR_INVALID_STATE: zlib binding closed", + ], ]; for (const [name, body, expected] of cases) { From fa7abd9ce47488a61e150fa6019828561540459c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:39:22 +0000 Subject: [PATCH 4/4] node:zlib: mark a handle closed when deflateInit2_/inflateInit2_ rejects init() `Context::init` leaves `mode` at `NONE` and returns silently when the zlib init call fails (e.g. `windowBits` outside 8..=15), so nothing set the wrapper's `closed` flag: the same `{ closed: false, mode: NONE }` state the brotli/zstd parameter-failure branches were just taught to avoid. The next `init()` then passed `throw_unless_idle` and reached `Context::init`'s `NONE => unreachable!()` arm, aborting the process. --- src/runtime/node/zlib/NativeZlib.rs | 6 ++++++ test/js/node/zlib/zlib-handle-bounds-check.test.ts | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/runtime/node/zlib/NativeZlib.rs b/src/runtime/node/zlib/NativeZlib.rs index 7bd40219cd82..66c9555bc5a4 100644 --- a/src/runtime/node/zlib/NativeZlib.rs +++ b/src/runtime/node/zlib/NativeZlib.rs @@ -210,6 +210,12 @@ mod _impl { self.stream .with_mut(|s| s.init(level, window_bits, mem_level, strategy, dictionary)); + // `Context::init` leaves `mode` at `NONE` when `deflateInit2_` / + // `inflateInit2_` rejects its arguments; mark the wrapper closed so + // the next operation is rejected instead of re-entering `NONE`. + if self.stream.with_mut(|s| s.mode == c::NodeMode::NONE) { + self.closed.set(true); + } Ok(JSValue::UNDEFINED) } diff --git a/test/js/node/zlib/zlib-handle-bounds-check.test.ts b/test/js/node/zlib/zlib-handle-bounds-check.test.ts index f18f7d5ff1ac..96edd1cc111d 100644 --- a/test/js/node/zlib/zlib-handle-bounds-check.test.ts +++ b/test/js/node/zlib/zlib-handle-bounds-check.test.ts @@ -321,8 +321,18 @@ describe.concurrent("zlib native handle driven outside the zlib.ts lifecycle", ( catch (e) { console.log("threw " + e.code + ": " + e.message); }`, "threw ERR_INVALID_STATE: Write already in progress", ], - // init() that creates the native state but then fails on a bad parameter - // key tears the Context down; the handle has to reject further use. + // init() that fails partway (zlib: deflateInit2_ rejects the arguments; + // brotli/zstd: a bad parameter key after the state was created) tears the + // Context down; the handle has to reject further use. + [ + "zlib: a handle whose init() arguments were rejected is closed", + `const C = zlib.createDeflate()._handle.constructor; + const h = new C(1); + h.init(100, 6, 8, 0, new Uint32Array(2), () => {}, undefined); + try { h.init(15, 6, 8, 0, new Uint32Array(2), () => {}, undefined); console.log("handled"); } + catch (e) { console.log("threw " + e.code + ": " + e.message); }`, + CLOSED, + ], [ "brotli: a handle whose init() parameters were rejected is closed", `const C = zlib.createBrotliCompress()._handle.constructor;