Repository: denoland/deno
Pull request: fix(ext/node): bad compression caused by invalid brotli params

Fixes https://github.com/denoland/deno/issues/30306
Fixes https://github.com/denoland/deno/issues/30430
Fixes https://github.com/denoland/deno/issues/29755

Changed files:
filename: ext/node/ops/zlib/mod.rs
patch: @@ -4,6 +4,10 @@ use std::borrow::Cow;
 use std::cell::RefCell;
 use std::rc::Rc;
 
+use brotli::enc::StandardAlloc;
+use brotli::enc::encode::BrotliEncoderDestroyInstance;
+use brotli::enc::encode::BrotliEncoderOperation;
+use brotli::enc::encode::BrotliEncoderStateStruct;
 use brotli::ffi;
 use deno_core::op2;
 use deno_core::v8;
@@ -548,7 +552,7 @@ pub fn op_zlib_close_if_pending(
 }
 
 struct BrotliEncoderCtx {
-  inst: *mut ffi::compressor::BrotliEncoderState,
+  inst: BrotliEncoderStateStruct<StandardAlloc>,
   write_result: *mut u32,
   callback: v8::Global<v8::Function>,
 }
@@ -590,19 +594,14 @@ impl BrotliEncoder {
     #[buffer] write_result: &mut [u32],
     #[global] callback: v8::Global<v8::Function>,
   ) {
-    // SAFETY: creates new brotli encoder instance. `params` is a valid slice of u32 values.
-    let inst = unsafe {
-      let state = ffi::compressor::BrotliEncoderCreateInstance(
-        Some(brotli_alloc),
-        Some(brotli_free),
-        std::ptr::null_mut(),
-      );
+    let inst = {
+      let mut state = BrotliEncoderStateStruct::new(StandardAlloc::default());
+
       for (i, &value) in params.iter().enumerate() {
-        ffi::compressor::BrotliEncoderSetParameter(
-          state,
-          encoder_param(i as u32),
-          value,
-        );
+        if value == 0xFFFFFFFF {
+          continue; // Skip setting the parameter, same as C API.
+        }
+        state.set_parameter(encoder_param(i as u32), value);
       }
 
       state
@@ -629,41 +628,31 @@ impl BrotliEncoder {
     &self,
     #[this] this: v8::Global<v8::Object>,
     scope: &mut v8::HandleScope,
-    #[smi] flush: i32,
+    #[smi] flush: u8,
     #[buffer] input: &[u8],
     #[smi] in_off: u32,
     #[smi] in_len: u32,
     #[buffer] out: &mut [u8],
     #[smi] out_off: u32,
     #[smi] out_len: u32,
   ) -> Result<(), JsErrorBox> {
-    let ctx = self.ctx.borrow();
-    let ctx = ctx.as_ref().expect("BrotliDecoder not initialized");
-
-    let mut next_in = input
-      .get(in_off as usize..in_off as usize + in_len as usize)
-      .ok_or_else(|| JsErrorBox::type_error("invalid input range"))?
-      .as_ptr();
-    let mut next_out = out
-      .get_mut(out_off as usize..out_off as usize + out_len as usize)
-      .ok_or_else(|| JsErrorBox::type_error("invalid output range"))?
-      .as_mut_ptr();
-
     let mut avail_in = in_len as usize;
     let mut avail_out = out_len as usize;
     // SAFETY: `inst`, `next_in`, `next_out`, `avail_in`, and `avail_out` are valid pointers.
     let callback = unsafe {
-      ffi::compressor::BrotliEncoderCompressStream(
-        ctx.inst,
-        std::mem::transmute::<
-          i32,
-          brotli::ffi::compressor::BrotliEncoderOperation,
-        >(flush),
+      let mut ctx = self.ctx.borrow_mut();
+      let ctx = ctx.as_mut().expect("BrotliDecoder not initialized");
+
+      ctx.inst.compress_stream(
+        std::mem::transmute::<u8, BrotliEncoderOperation>(flush),
         &mut avail_in,
-        &mut next_in,
+        input,
+        &mut (in_off as usize),
         &mut avail_out,
-        &mut next_out,
-        std::ptr::null_mut(),
+        out,
+        &mut (out_off as usize),
+        &mut None,
+        &mut |_, _, _, _| (),
       );
 
       // SAFETY: `write_result` is a valid pointer to a mutable slice of u32 of length 2.
@@ -682,7 +671,7 @@ impl BrotliEncoder {
   #[fast]
   pub fn write_sync(
     &self,
-    #[smi] flush: i32,
+    #[smi] flush: u8,
     #[buffer] input: &[u8],
     #[smi] in_off: u32,
     #[smi] in_len: u32,
@@ -693,50 +682,36 @@ impl BrotliEncoder {
     let mut ctx = self.ctx.borrow_mut();
     let ctx = ctx.as_mut().expect("BrotliEncoder not initialized");
 
-    let mut next_in = input
-      .get(in_off as usize..in_off as usize + in_len as usize)
-      .ok_or_else(|| JsErrorBox::type_error("invalid input range"))?
-      .as_ptr();
-    let mut next_out = out
-      .get_mut(out_off as usize..out_off as usize + out_len as usize)
-      .ok_or_else(|| JsErrorBox::type_error("invalid output range"))?
-      .as_mut_ptr();
-
     let mut avail_in = in_len as usize;
     let mut avail_out = out_len as usize;
-
-    // SAFETY: `ctx.inst` is a valid pointer to a BrotliEncoderState.
+    // SAFETY: `inst`, `next_in`, `next_out`, `avail_in`, and `avail_out` are valid pointers.
     unsafe {
-      ffi::compressor::BrotliEncoderCompressStream(
-        ctx.inst,
-        std::mem::transmute::<
-          i32,
-          brotli::ffi::compressor::BrotliEncoderOperation,
-        >(flush),
+      ctx.inst.compress_stream(
+        std::mem::transmute::<u8, BrotliEncoderOperation>(flush),
         &mut avail_in,
-        &mut next_in,
+        input,
+        &mut (in_off as usize),
         &mut avail_out,
-        &mut next_out,
-        std::ptr::null_mut(),
+        out,
+        &mut (out_off as usize),
+        &mut None,
+        &mut |_, _, _, _| (),
       );
 
       // SAFETY: `ctx.write_result` is a valid pointer to a mutable slice of u32 of length 2.
       let result = std::slice::from_raw_parts_mut(ctx.write_result, 2);
       result[0] = avail_out as u32;
       result[1] = avail_in as u32;
-    }
+    };
 
     Ok(())
   }
 
   #[fast]
   fn close(&self) {
     let mut ctx = self.ctx.borrow_mut();
-    if let Some(ctx) = ctx.take() {
-      // SAFETY: `ctx.inst` is a valid pointer to a BrotliEncoderState.
-      unsafe {
-        ffi::compressor::BrotliEncoderDestroyInstance(ctx.inst);
-      }
+    if let Some(mut ctx) = ctx.take() {
+      BrotliEncoderDestroyInstance(&mut ctx.inst);
     }
   }
 }

filename: tests/unit_node/zlib_test.ts
patch: @@ -158,7 +158,7 @@ Deno.test("Brotli quality 10 doesn't panic", () => {
   });
   assertEquals(
     new Uint8Array(e.buffer, e.byteOffset, e.byteLength),
-    new Uint8Array([17, 10, 4, 0, 2, 97, 98, 99, 3]),
+    new Uint8Array([11, 1, 128, 97, 98, 99, 3]),
   );
 });