Repository: denoland/deno
Pull request: fix(ext/node): fix multiple node:zlib compatibility issues

These fixes bring back number of failing `node:zlib` tests down to 8.

Changed files:
filename: ext/node/ops/zlib/mod.rs
patch: @@ -357,6 +357,22 @@ impl Zlib {
     Ok(zlib.err)
   }
 
+  #[fast]
+  pub fn params(
+    &self,
+    #[smi] level: i32,
+    #[smi] strategy: i32,
+  ) -> Result<(), ZlibError> {
+    let mut zlib = self.inner.borrow_mut();
+    let zlib = zlib.as_mut().ok_or(ZlibError::NotInitialized)?;
+
+    zlib.err = zlib.strm.deflate_params(level, strategy);
+    zlib.level = level;
+    zlib.strategy = strategy;
+
+    Ok(())
+  }
+
   #[smi]
   pub fn init(
     &self,
@@ -611,7 +627,9 @@ impl BrotliEncoder {
         if value == 0xFFFFFFFF {
           continue; // Skip setting the parameter, same as C API.
         }
-        state.set_parameter(encoder_param(i as u32), value);
+        if !state.set_parameter(encoder_param(i as u32), value) {
+          return false;
+        }
       }
 
       state
@@ -832,7 +850,7 @@ impl BrotliDecoder {
     #[smi] out_off: u32,
     #[smi] out_len: u32,
   ) -> Result<(), JsErrorBox> {
-    let callback = {
+    let (error_info, callback) = {
       let ctx = self.ctx.borrow();
       let ctx = ctx.as_ref().expect("BrotliDecoder not initialized");
 
@@ -849,8 +867,8 @@ impl BrotliDecoder {
       let mut avail_out = out_len as usize;
 
       // SAFETY: `inst`, `next_in`, `next_out`, `avail_in`, and `avail_out` are valid pointers.
-      unsafe {
-        ffi::decompressor::ffi::BrotliDecoderDecompressStream(
+      let error_info = unsafe {
+        let res = ffi::decompressor::ffi::BrotliDecoderDecompressStream(
           ctx.inst,
           &mut avail_in,
           &mut next_in,
@@ -863,13 +881,48 @@ impl BrotliDecoder {
         let result = std::slice::from_raw_parts_mut(ctx.write_result, 2);
         result[0] = avail_out as u32;
         result[1] = avail_in as u32;
-      }
 
-      v8::Local::new(scope, &ctx.callback)
+        if matches!(
+          res,
+          ffi::decompressor::ffi::interface::BrotliDecoderResult::BROTLI_DECODER_RESULT_ERROR
+        ) {
+          let error_code =
+            ffi::decompressor::ffi::BrotliDecoderGetErrorCode(ctx.inst);
+          let error_str =
+            ffi::decompressor::ffi::BrotliDecoderErrorString(error_code);
+          let msg = if error_str.is_null() {
+            "Decompression failed".to_string()
+          } else {
+            let c_str = std::ffi::CStr::from_ptr(error_str as *const _);
+            format!(
+              "ERR_{}",
+              c_str.to_str().unwrap_or("Decompression failed")
+            )
+          };
+          Some((error_code as i32, msg))
+        } else {
+          None
+        }
+      };
+
+      (error_info, v8::Local::new(scope, &ctx.callback))
     };
 
     let this = v8::Local::new(scope, &this);
-    let _ = callback.call(scope, this.into(), &[]);
+
+    if let Some((err, msg)) = error_info {
+      v8_static_strings! {
+        ONERROR_STR = "onerror",
+      }
+      let onerror_str = ONERROR_STR.v8_string(scope).unwrap();
+      let onerror = this.get(scope, onerror_str.into()).unwrap();
+      let cb = v8::Local::<v8::Function>::try_from(onerror).unwrap();
+      let msg = v8::String::new(scope, &msg).unwrap();
+      let err = v8::Integer::new(scope, err);
+      cb.call(scope, this.into(), &[msg.into(), err.into()]);
+    } else {
+      let _ = callback.call(scope, this.into(), &[]);
+    }
 
     Ok(())
   }

filename: ext/node/ops/zlib/stream.rs
patch: @@ -123,6 +123,11 @@ impl StreamWrapper {
     unsafe { zlib::inflate(&mut self.strm, flush as _) }
   }
 
+  pub fn deflate_params(&mut self, level: c_int, strategy: c_int) -> c_int {
+    // SAFETY: `self.strm` is an initialized `zlib::z_stream`.
+    unsafe { zlib::deflateParams(&mut self.strm, level, strategy) }
+  }
+
   pub fn inflate_set_dictionary(&mut self, dictionary: &[u8]) -> c_int {
     // SAFETY: `self.strm` is an initialized `zlib::z_stream`.
     unsafe {

filename: ext/node/polyfills/internal/util.mjs
patch: @@ -25,6 +25,9 @@ const {
   Promise,
   ReflectApply,
   ReflectConstruct,
+  SafeSet,
+  SetPrototypeAdd,
+  SetPrototypeHas,
   SafeWeakRef,
   StringPrototypeReplace,
   SymbolFor,
@@ -159,9 +162,17 @@ export function convertToValidSignal(signal) {
   throw new ERR_UNKNOWN_SIGNAL(signal);
 }
 
+const codesWarned = new SafeSet();
+
 export function deprecateInstantiation(Constructor, deprecationCode, ...args) {
-  // DEP0184: Calling zlib classes without new is deprecated
-  // For now, just create and return the new instance
+  if (!SetPrototypeHas(codesWarned, deprecationCode)) {
+    SetPrototypeAdd(codesWarned, deprecationCode);
+    globalThis.process.emitWarning(
+      `Instantiating ${Constructor.name} without the 'new' keyword has been deprecated.`,
+      "DeprecationWarning",
+      deprecationCode,
+    );
+  }
   return ReflectConstruct(Constructor, args);
 }
 

filename: ext/node/polyfills/zlib.js
patch: @@ -124,12 +124,12 @@ const {
   ZSTD_e_end,
 } = zlibConstants;
 
-export const constants = zlibConstants;
+export const constants = ObjectFreeze(zlibConstants);
 
 // Translation table for return codes.
 export const codes = {
   Z_OK: constants.Z_OK,
-  Z_STREAM_END: constants.Z_STREAM_EBROTLI_COMPRESSND,
+  Z_STREAM_END: constants.Z_STREAM_END,
   Z_NEED_DICT: constants.Z_NEED_DICT,
   Z_ERRNO: constants.Z_ERRNO,
   Z_STREAM_ERROR: constants.Z_STREAM_ERROR,
@@ -143,6 +143,8 @@ for (const ckey of ObjectKeys(codes)) {
   codes[codes[ckey]] = ckey;
 }
 
+ObjectFreeze(codes);
+
 function zlibBuffer(engine, buffer, callback) {
   validateFunction(callback, "callback");
   // Streams do not support non-Uint8Array ArrayBufferViews yet. Convert it to a
@@ -956,7 +958,14 @@ function Brotli(opts, mode) {
     : new binding.BrotliEncoder(mode);
 
   this._writeState = new Uint32Array(2);
-  handle.init(brotliInitParamsArray, this._writeState, processCallback);
+  const success = handle.init(
+    brotliInitParamsArray,
+    this._writeState,
+    processCallback,
+  );
+  if (!success) {
+    throw new ERR_ZLIB_INITIALIZATION_FAILED("Initialization failed");
+  }
 
   ReflectApply(ZlibBase, this, [opts, mode, handle, brotliDefaultOpts]);
 }

filename: tests/node_compat/config.jsonc
patch: @@ -1568,11 +1568,11 @@
     "parallel/test-zlib-brotli-from-brotli.js": {},
     "parallel/test-zlib-brotli-from-string.js": {},
     // "parallel/test-zlib-brotli-kmaxlength-rangeerror.js": {},
-    // "parallel/test-zlib-brotli.js": {},
+    "parallel/test-zlib-brotli.js": {},
     "parallel/test-zlib-close-after-error.js": {},
     "parallel/test-zlib-close-after-write.js": {},
     "parallel/test-zlib-close-in-ondata.js": {},
-    // "parallel/test-zlib-const.js": {},
+    "parallel/test-zlib-const.js": {},
     "parallel/test-zlib-convenience-methods.js": {},
     "parallel/test-zlib-crc32.js": {},
     "parallel/test-zlib-create-raw.js": {},
@@ -1595,23 +1595,23 @@
     "parallel/test-zlib-from-string.js": {},
     "parallel/test-zlib-invalid-arg-value-brotli-compress.js": {},
     // "parallel/test-zlib-invalid-input-memory.js": {},
-    // "parallel/test-zlib-invalid-input.js": {},
+    "parallel/test-zlib-invalid-input.js": {},
     // "parallel/test-zlib-kmaxlength-rangeerror.js": {},
     "parallel/test-zlib-maxOutputLength.js": {},
     "parallel/test-zlib-no-stream.js": {},
     "parallel/test-zlib-not-string-or-buffer.js": {},
     "parallel/test-zlib-object-write.js": {},
-    // "parallel/test-zlib-params.js": {},
+    "parallel/test-zlib-params.js": {},
     "parallel/test-zlib-premature-end.js": {},
     "parallel/test-zlib-random-byte-pipes.js": {},
-    // "parallel/test-zlib-reset-before-write.js": {},
+    "parallel/test-zlib-reset-before-write.js": {},
     "parallel/test-zlib-sync-no-event.js": {},
     "parallel/test-zlib-truncated.js": {},
     "parallel/test-zlib-type-error.js": {},
     // "parallel/test-zlib-unused-weak.js": {},
     "parallel/test-zlib-unzip-one-byte-chunks.js": {},
     "parallel/test-zlib-write-after-close.js": {},
-    // "parallel/test-zlib-write-after-end.js": {},
+    "parallel/test-zlib-write-after-end.js": {},
     "parallel/test-zlib-write-after-flush.js": {},
     "parallel/test-zlib-zero-byte.js": {},
     "parallel/test-zlib-zero-windowBits.js": {},
@@ -1622,7 +1622,7 @@
     // "parallel/test-zlib-zstd-kmaxlength-rangeerror.js": {},
     "parallel/test-zlib-zstd-pledged-src-size.js": {},
     "parallel/test-zlib-zstd.js": {},
-    // "parallel/test-zlib.js": {},
+    "parallel/test-zlib.js": {},
     "pseudo-tty/test-set-raw-mode-reset-process-exit.js": {},
     "pseudo-tty/test-set-raw-mode-reset.js": {},
     "pseudo-tty/test-tty-color-support-warning-2.js": {},