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

src/runtime/node/node_zlib_binding.rs

@@ -279,6 +279,21 @@ pub(crate) trait CompressionStreamImpl: Sized + Taskable + 'static {
 }
 
 impl<T: CompressionStreamImpl> CompressionStream<T> {
+    /// 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<T: CompressionStreamImpl> CompressionStream<T> {
                 .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<T: CompressionStreamImpl> CompressionStream<T> {
         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<T: CompressionStreamImpl> CompressionStream<T> {
                 .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<T: CompressionStreamImpl> CompressionStream<T> {
             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);

src/runtime/node/zlib/NativeBrotli.rs

@@ -189,6 +189,9 @@
                     )
                     .throw());
             }
+            // A closed `Context` is in `NodeMode::NONE`, which `Context::init`
+            // cannot re-initialize.
+            CompressionStream::<Self>::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 @@
         }
 
         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;

src/runtime/node/zlib/NativeZlib.rs

@@ -130,6 +130,9 @@
                 )
                 .throw());
             }
+            // A closed `Context` is in `NodeMode::NONE`, which `Context::init`
+            // cannot re-initialize.
+            CompressionStream::<Self>::throw_if_closed(self, global)?;
 
             let window_bits =
                 validators::validate_int32(global, arguments.ptr[0], "windowBits", None, None)?;
@@ -183,6 +186,18 @@
                         ));
                     }
                 };
+                // `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())
             };
 

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::<Self>::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 {

test/js/node/zlib/zlib-handle-bounds-check.test.ts

@@ -146,3 +146,134 @@
     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,
+    });
+  });
+});