Harden nodex-api memory safety at N-API boundaries

crates/nodex-api/src/env.rs

@@ -659,7 +659,19 @@ impl NapiEnv {
     /// napi_set_instance_data(). If no data is set, the call will succeed and data will be set to
     /// NULL.
     #[inline]
-    pub fn get_instance_data<T>(&self) -> NapiResult<Option<&mut T>> {
+    pub fn get_instance_data<T>(&self) -> NapiResult<Option<&T>> {
+        let data = napi_call!(=napi_get_instance_data, *self) as *mut T;
+        if data.is_null() {
+            Ok(None)
+        } else {
+            unsafe { Ok(Some(&*(data as *const T))) }
+        }
+    }
+
+    #[cfg(feature = "v6")]
+    #[inline]
+    /// Mutable access variant of instance data; use when data must be updated in place.
+    pub fn get_instance_data_mut<T>(&mut self) -> NapiResult<Option<&mut T>> {
         let data = napi_call!(=napi_get_instance_data, *self) as *mut T;
         if data.is_null() {
             Ok(None)

crates/nodex-api/src/mac.rs

@@ -45,7 +45,8 @@ macro_rules! napi_call {
         unsafe {
             let mut result = std::mem::MaybeUninit::uninit();
             let status = $crate::api::$napi($($args),+, result.as_mut_ptr());
-            (status, result.assume_init())
+            let value = status.ok().then(|| result.assume_init());
+            (status, value)
         }
     };
 

crates/nodex-api/src/reference.rs

@@ -1,6 +1,6 @@
 use crate::{api, prelude::*};
 
-#[derive(Clone, Debug)]
+#[derive(Debug)]
 pub struct NapiRef(NapiEnv, napi_ref);
 
 impl NapiRef {

crates/nodex-api/src/value/external.rs

@@ -43,7 +43,13 @@ impl<T> JsExternal<T> {
     }
 
     /// Access the underlaying data.
-    pub fn get(&self) -> NapiResult<&mut T> {
+    pub fn get(&self) -> NapiResult<&T> {
+        let ext = napi_call!(=napi_get_value_external, self.env(), self.raw());
+        unsafe { Ok(&*(ext as *const T)) }
+    }
+
+    /// Mutable access to the underlying external data.
+    pub fn get_mut(&mut self) -> NapiResult<&mut T> {
         let ext = napi_call!(=napi_get_value_external, self.env(), self.raw());
         unsafe { Ok(&mut *(ext as *mut T)) }
     }

crates/nodex-api/src/value/mod.rs

@@ -441,10 +441,26 @@ pub trait NapiValueT: NapiValueCheck + Sized {
     ///
     /// NB: if a there is no wrap or the wrap is just removed by NapiValue::remove_wrap, return
     /// None.
-    fn unwrap<T>(&self) -> NapiResult<Option<&mut T>> {
+    fn unwrap<T>(&self) -> NapiResult<Option<&T>> {
         let (status, value) = napi_call!(?napi_unwrap, self.env(), self.raw());
         match status {
-            NapiStatus::Ok => unsafe { Ok(Some(&mut *(value as *mut T))) },
+            NapiStatus::Ok => match value {
+                Some(value) => unsafe { Ok(Some(&*(value as *const T))) },
+                None => Ok(None),
+            },
+            NapiStatus::InvalidArg => Ok(None),
+            err => Err(err),
+        }
+    }
+
+    /// Mutable variant of unwrap.
+    fn unwrap_mut<T>(&mut self) -> NapiResult<Option<&mut T>> {
+        let (status, value) = napi_call!(?napi_unwrap, self.env(), self.raw());
+        match status {
+            NapiStatus::Ok => match value {
+                Some(value) => unsafe { Ok(Some(&mut *(value as *mut T))) },
+                None => Ok(None),
+            },
             NapiStatus::InvalidArg => Ok(None),
             err => Err(err),
         }

crates/nodex-api/src/work.rs

@@ -61,11 +61,13 @@ impl<T> NapiAsyncWork<T> {
             unsafe {
                 let mut pair: Box<(
                     Box<dyn FnMut(&mut T)>,
-                    Box<dyn FnMut(NapiEnv, NapiStatus, T)>,
+                    Box<dyn FnMut(NapiEnv, NapiStatus, T) -> NapiResult<()>>,
                     T,
                 )> = Box::from_raw(data as _);
                 let mut complete = pair.1;
-                complete(env, status, pair.2);
+                if let Err(err) = complete(env, status, pair.2) {
+                    log::error!("NapiAsyncWork::complete: {}", err);
+                }
             }
         }
 

examples/demo/src/lib.rs

@@ -2,7 +2,7 @@ use nodex::prelude::*;
 
 nodex::napi_module!(init);
 
-fn init(env: NapiEnv, mut exports: JsObject) -> NapiResult<()> {
+fn init(mut env: NapiEnv, mut exports: JsObject) -> NapiResult<()> {
     nodex::napi_guard!(env.napi_version()?);
 
     let mut obj = env.object()?;
@@ -203,7 +203,7 @@ fn init(env: NapiEnv, mut exports: JsObject) -> NapiResult<()> {
 
     let value = env.get_instance_data::<usize>()?;
     println!("get instance data: {:?}", value);
-    if let Ok(Some(data)) = env.get_instance_data::<usize>() {
+    if let Ok(Some(data)) = env.get_instance_data_mut::<usize>() {
         *data = 200;
     }
     let value = env.get_instance_data::<usize>()?;