Merge pull request #937 from neon-bindings/deprecate-external-buffers

Disable external buffers by default

.cargo/config.toml

@@ -1,7 +1,7 @@
 [alias]
 # Neon defines mutually exclusive feature flags which prevents using `cargo clippy --all-features`
 # The following aliases simplify linting the entire workspace
-neon-check = " check  --all --all-targets --features napi-experimental,futures"
-neon-clippy = "clippy --all --all-targets --features napi-experimental,futures -- -A clippy::missing_safety_doc"
-neon-test = "  test   --all               --features=doc-comment,napi-experimental,futures"
-neon-doc = "   rustdoc -p neon            --features=doc-dependencies,napi-experimental,futures -- --cfg docsrs"
+neon-check = " check  --all --all-targets --features napi-experimental,futures,external-buffers"
+neon-clippy = "clippy --all --all-targets --features napi-experimental,futures,external-buffers -- -A clippy::missing_safety_doc"
+neon-test = "  test   --all               --features=doc-comment,napi-experimental,futures,external-buffers"
+neon-doc = "   rustdoc -p neon            --features=doc-dependencies,napi-experimental,futures,external-buffers -- --cfg docsrs"

crates/neon/Cargo.toml

@@ -38,6 +38,11 @@ optional = true
 [features]
 default = ["napi-1"]
 
+# Enable the creation of external binary buffers. This is disabled by default
+# since these APIs fail at runtime in environments that enable the V8 memory
+# cage (such as Electron: https://www.electronjs.org/blog/v8-memory-cage).
+external-buffers = []
+
 # Experimental Rust Futures API
 # https://github.com/neon-bindings/rfcs/pull/46
 futures = ["tokio"]

crates/neon/src/sys/arraybuffer.rs

@@ -1,4 +1,6 @@
-use std::{mem::MaybeUninit, os::raw::c_void, ptr::null_mut, slice};
+#[cfg(feature = "external-buffers")]
+use std::os::raw::c_void;
+use std::{mem::MaybeUninit, ptr::null_mut, slice};
 
 use super::{
     bindings as napi,
@@ -18,6 +20,7 @@ pub unsafe fn new(env: Env, len: usize) -> Result<Local, napi::Status> {
     Ok(buf.assume_init())
 }
 
+#[cfg(feature = "external-buffers")]
 pub unsafe fn new_external<T>(env: Env, data: T) -> Local
 where
     T: AsMut<[u8]> + Send,
@@ -43,6 +46,7 @@ where
     result.assume_init()
 }
 
+#[cfg(feature = "external-buffers")]
 unsafe extern "C" fn drop_external<T>(_env: Env, _data: *mut c_void, hint: *mut c_void) {
     Box::<T>::from_raw(hint as *mut _);
 }

crates/neon/src/sys/bindings/functions.rs

@@ -198,6 +198,7 @@ mod napi1 {
 
             fn strict_equals(env: Env, lhs: Value, rhs: Value, result: *mut bool) -> Status;
 
+            #[cfg(feature = "external-buffers")]
             fn create_external_arraybuffer(
                 env: Env,
                 data: *mut c_void,
@@ -207,6 +208,7 @@ mod napi1 {
                 result: *mut Value,
             ) -> Status;
 
+            #[cfg(feature = "external-buffers")]
             fn create_external_buffer(
                 env: Env,
                 length: usize,

crates/neon/src/sys/bindings/mod.rs

@@ -106,7 +106,7 @@ macro_rules! napi_name {
 /// ```
 macro_rules! generate {
     (extern "C" {
-        $(fn $name:ident($($param:ident: $ptype:ty$(,)?)*)$( -> $rtype:ty)?;)+
+        $($(#[$attr:meta])? fn $name:ident($($param:ident: $ptype:ty$(,)?)*)$( -> $rtype:ty)?;)+
     }) => {
         struct Napi {
             $(
@@ -167,7 +167,7 @@ macro_rules! generate {
         }
 
         $(
-            #[inline]
+            $(#[$attr])? #[inline]
             pub(crate) unsafe fn $name($($param: $ptype,)*)$( -> $rtype)* {
                 (NAPI.$name)($($param,)*)
             }

crates/neon/src/sys/buffer.rs

@@ -1,4 +1,6 @@
-use std::{mem::MaybeUninit, os::raw::c_void, slice};
+#[cfg(feature = "external-buffers")]
+use std::os::raw::c_void;
+use std::{mem::MaybeUninit, slice};
 
 use super::{
     bindings as napi,
@@ -27,6 +29,7 @@ pub unsafe fn uninitialized(env: Env, len: usize) -> Result<(Local, *mut u8), na
     Ok((buf.assume_init(), bytes.assume_init().cast()))
 }
 
+#[cfg(feature = "external-buffers")]
 pub unsafe fn new_external<T>(env: Env, data: T) -> Local
 where
     T: AsMut<[u8]> + Send,
@@ -52,6 +55,7 @@ where
     result.assume_init()
 }
 
+#[cfg(feature = "external-buffers")]
 unsafe extern "C" fn drop_external<T>(_env: Env, _data: *mut c_void, hint: *mut c_void) {
     Box::<T>::from_raw(hint as *mut _);
 }

crates/neon/src/types_impl/buffer/types.rs

@@ -82,7 +82,21 @@ impl JsBuffer {
         }
     }
 
-    /// Construct a new `Buffer` from bytes allocated by Rust
+    #[cfg(feature = "external-buffers")]
+    #[cfg_attr(docsrs, doc(cfg(feature = "external-buffers")))]
+    /// Construct a new `Buffer` from bytes allocated by Rust.
+    ///
+    /// # Compatibility Note
+    ///
+    /// Some Node environments are built using V8's _sandboxed pointers_ functionality, which
+    /// [disallows the use of external buffers](https://www.electronjs.org/blog/v8-memory-cage).
+    /// In those environments, calling the underlying
+    /// [runtime function](https://nodejs.org/api/n-api.html#napi_create_external_buffer)
+    /// used by this method results in an immediate termination of the Node VM.
+    ///
+    /// As a result, this API is disabled by default. If you are confident that your code will
+    /// only be used in environments that disable sandboxed pointers, you can make use of this
+    /// method by enabling the **`external-buffers`** feature flag.
     pub fn external<'a, C, T>(cx: &mut C, data: T) -> Handle<'a, Self>
     where
         C: Context<'a>,
@@ -238,7 +252,21 @@ impl JsArrayBuffer {
         <JsArrayBuffer as TypedArray>::from_slice(cx, slice)
     }
 
-    /// Construct a new `JsArrayBuffer` from bytes allocated by Rust
+    #[cfg(feature = "external-buffers")]
+    #[cfg_attr(docsrs, doc(cfg(feature = "external-buffers")))]
+    /// Construct a new `JsArrayBuffer` from bytes allocated by Rust.
+    ///
+    /// # Compatibility Note
+    ///
+    /// Some Node environments are built using V8's _sandboxed pointers_ functionality, which
+    /// [disallows the use of external buffers](https://www.electronjs.org/blog/v8-memory-cage).
+    /// In those environments, calling the underlying
+    /// [runtime function](https://nodejs.org/api/n-api.html#napi_create_external_arraybuffer)
+    /// used by this method results in an immediate termination of the Node VM.
+    ///
+    /// As a result, this API is disabled by default. If you are confident that your code will
+    /// only be used in environments that disable sandboxed pointers, you can make use of this
+    /// method by enabling the **`external-buffers`** feature flag.
     pub fn external<'a, C, T>(cx: &mut C, data: T) -> Handle<'a, Self>
     where
         C: Context<'a>,

test/napi/Cargo.toml

@@ -16,4 +16,4 @@ tokio = { version = "1", features = ["rt-multi-thread"] }
 [dependencies.neon]
 version = "1.0.0-alpha.1"
 path = "../../crates/neon"
-features = ["futures", "napi-experimental"]
+features = ["futures", "napi-experimental", "external-buffers"]