fix(napi): use create_buffer/arrary_buffer if provided data is empty

crates/backend/src/typegen/fn.rs

@@ -66,7 +66,7 @@ impl ToTypeDef for NapiFn {
         .ts_generic_types
         .as_ref()
         .map(|g| format!("<{}>", g))
-        .unwrap_or("".to_string()),
+        .unwrap_or_else(|| "".to_string()),
       args = self
         .ts_args_type
         .clone()

crates/napi/src/bindgen_runtime/js_values/arraybuffer.rs

@@ -177,22 +177,24 @@ macro_rules! impl_typed_array {
           val.finalizer_notify,
         )));
         check_status!(
-          unsafe {
+          if length == 0 {
-            sys::napi_create_external_arraybuffer(
+            // Rust uses 0x1 as the data pointer for empty buffers,
-              env,
+            // but NAPI/V8 only allows multiple buffers to have
-              if length == 0 {
+            // the same data pointer if it's 0x0.
-                // Rust uses 0x1 as the data pointer for empty buffers,
+            unsafe {
-                // but NAPI/V8 only allows multiple buffers to have
+              sys::napi_create_arraybuffer(env, length, ptr::null_mut(), &mut arraybuffer_value)
-                // the same data pointer if it's 0x0.
+            }
-                ptr::null_mut()
+          } else {
-              } else {
+            unsafe {
-                val.data as *mut c_void
+              sys::napi_create_external_arraybuffer(
-              },
+                env,
-              length,
+                val.data as *mut c_void,
-              Some(finalizer::<$rust_type>),
+                length,
-              hint_ptr as *mut c_void,
+                Some(finalizer::<$rust_type>),
-              &mut arraybuffer_value,
+                hint_ptr as *mut c_void,
-            )
+                &mut arraybuffer_value,
+              )
+            }
           },
           "Create external arraybuffer failed"
         )?;

crates/napi/src/bindgen_runtime/js_values/buffer.rs

@@ -172,22 +172,22 @@ impl ToNapiValue for Buffer {
     let len = val.inner.len();
     let mut ret = ptr::null_mut();
     check_status!(
-      unsafe {
+      if len == 0 {
-        sys::napi_create_external_buffer(
+        // Rust uses 0x1 as the data pointer for empty buffers,
-          env,
+        // but NAPI/V8 only allows multiple buffers to have
-          len,
+        // the same data pointer if it's 0x0.
-          if len == 0 {
+        unsafe { sys::napi_create_buffer(env, len, ptr::null_mut(), &mut ret) }
-            // Rust uses 0x1 as the data pointer for empty buffers,
+      } else {
-            // but NAPI/V8 only allows multiple buffers to have
+        unsafe {
-            // the same data pointer if it's 0x0.
+          sys::napi_create_external_buffer(
-            ptr::null_mut()
+            env,
-          } else {
+            len,
-            val.inner.as_mut_ptr() as *mut _
+            val.inner.as_mut_ptr() as *mut _,
-          },
+            Some(drop_buffer),
-          Some(drop_buffer),
+            Box::into_raw(Box::new((len, val.capacity))) as *mut _,
-          Box::into_raw(Box::new((len, val.capacity))) as *mut _,
+            &mut ret,
-          &mut ret,
+          )
-        )
+        }
       },
       "Failed to create napi buffer"
     )?;

crates/napi/src/env.rs

@@ -35,6 +35,8 @@ use serde::Serialize;
 
 pub type Callback = unsafe extern "C" fn(sys::napi_env, sys::napi_callback_info) -> sys::napi_value;
 
+pub(crate) static EMPTY_VEC: Vec<u8> = vec![];
+
 #[derive(Clone, Copy)]
 /// `Env` is used to represent a context that the underlying N-API implementation can use to persist VM-specific state.
 ///
@@ -261,21 +263,21 @@ impl Env {
     let mut raw_value = ptr::null_mut();
     let data_ptr = data.as_mut_ptr();
     check_status!(unsafe {
-      sys::napi_create_external_buffer(
+      if length == 0 {
-        self.0,
+        // Rust uses 0x1 as the data pointer for empty buffers,
-        length,
+        // but NAPI/V8 only allows multiple buffers to have
-        if length == 0 {
+        // the same data pointer if it's 0x0.
-          // Rust uses 0x1 as the data pointer for empty buffers,
+        sys::napi_create_buffer(self.0, length, ptr::null_mut(), &mut raw_value)
-          // but NAPI/V8 only allows multiple buffers to have
+      } else {
-          // the same data pointer if it's 0x0.
+        sys::napi_create_external_buffer(
-          ptr::null_mut()
+          self.0,
-        } else {
+          length,
-          data_ptr as *mut c_void
+          data_ptr as *mut c_void,
-        },
+          Some(drop_buffer),
-        Some(drop_buffer),
+          Box::into_raw(Box::new((length, data.capacity()))) as *mut c_void,
-        Box::into_raw(Box::new((length, data.capacity()))) as *mut c_void,
+          &mut raw_value,
-        &mut raw_value,
+        )
-      )
+      }
     })?;
     Ok(JsBufferValue::new(
       JsBuffer(Value {
@@ -304,18 +306,17 @@ impl Env {
     Finalize: FnOnce(Hint, Env),
   {
     let mut raw_value = ptr::null_mut();
+    if data.is_null() || data == EMPTY_VEC.as_ptr() {
+      return Err(Error::new(
+        Status::InvalidArg,
+        "Borrowed data should not be null".to_owned(),
+      ));
+    }
     check_status!(unsafe {
       sys::napi_create_external_buffer(
         self.0,
         length,
-        if length == 0 {
+        data as *mut c_void,
-          // Rust uses 0x1 as the data pointer for empty buffers,
-          // but NAPI/V8 only allows multiple buffers to have
-          // the same data pointer if it's 0x0.
-          ptr::null_mut()
-        } else {
-          data as *mut c_void
-        },
         Some(
           raw_finalize_with_custom_callback::<Hint, Finalize>
             as unsafe extern "C" fn(
@@ -393,26 +394,26 @@ impl Env {
     ))
   }
 
-  pub fn create_arraybuffer_with_data(&self, data: Vec<u8>) -> Result<JsArrayBufferValue> {
+  pub fn create_arraybuffer_with_data(&self, mut data: Vec<u8>) -> Result<JsArrayBufferValue> {
     let length = data.len();
     let mut raw_value = ptr::null_mut();
-    let data_ptr = data.as_ptr();
+    let data_ptr = data.as_mut_ptr();
     check_status!(unsafe {
-      sys::napi_create_external_arraybuffer(
+      if length == 0 {
-        self.0,
+        // Rust uses 0x1 as the data pointer for empty buffers,
-        if length == 0 {
+        // but NAPI/V8 only allows multiple buffers to have
-          // Rust uses 0x1 as the data pointer for empty buffers,
+        // the same data pointer if it's 0x0.
-          // but NAPI/V8 only allows multiple buffers to have
+        sys::napi_create_arraybuffer(self.0, length, ptr::null_mut(), &mut raw_value)
-          // the same data pointer if it's 0x0.
+      } else {
-          ptr::null_mut()
+        sys::napi_create_external_arraybuffer(
-        } else {
+          self.0,
-          data_ptr as *mut c_void
+          data_ptr as *mut c_void,
-        },
+          length,
-        length,
+          Some(drop_buffer),
-        Some(drop_buffer),
+          Box::into_raw(Box::new((length, data.capacity()))) as *mut c_void,
-        Box::into_raw(Box::new((length, data.capacity()))) as *mut c_void,
+          &mut raw_value,
-        &mut raw_value,
+        )
-      )
+      }
     })?;
 
     mem::forget(data);
@@ -609,7 +610,7 @@ impl Env {
     // `&'static dyn Fn…` in Rust parlance, in that thanks to `Box::into_raw()`
     // we are sure the context won't be freed, and thus the callback may use
     // it to call the actual method thanks to the trampoline…
-    // But we thus have a data leak: there is nothing yet reponsible for
+    // But we thus have a data leak: there is nothing yet responsible for
     // running the `drop(Box::from_raw(…))` cleanup code.
     //
     // To solve that, according to the docs, we need to attach a finalizer:

examples/napi-compat-mode/__test__/buffer.spec.ts

@@ -34,8 +34,12 @@ test('should create borrowed buffer with finalize', (t) => {
 })
 
 test('should create empty borrowed buffer with finalize', (t) => {
-  t.is(bindings.createEmptyBorrowedBufferWithFinalize().toString(), '')
+  t.throws(() => bindings.createEmptyBorrowedBufferWithFinalize().toString(), {
-  t.is(bindings.createEmptyBorrowedBufferWithFinalize().toString(), '')
+    message: 'Borrowed data should not be null',
+  })
+  t.throws(() => bindings.createEmptyBorrowedBufferWithFinalize().toString(), {
+    message: 'Borrowed data should not be null',
+  })
 })
 
 test('should create empty buffer', (t) => {

examples/napi/__test__/values.spec.ts

@@ -377,6 +377,16 @@ test('buffer', (t) => {
   t.is(b.toString(), '')
 })
 
+test('reset empty buffer', (t) => {
+  const empty = getEmptyBuffer()
+
+  const shared = new ArrayBuffer(0)
+  const buffer = Buffer.from(shared)
+  t.notThrows(() => {
+    buffer.set(empty)
+  })
+})
+
 test('convert typedarray to vec', (t) => {
   const input = new Uint32Array([1, 2, 3, 4, 5])
   t.deepEqual(convertU32Array(input), Array.from(input))