From ac4b5de656d318b98badc13e226f986e6c0b55eb Mon Sep 17 00:00:00 2001
From: Divy Srivastava <dj.srivastava23@gmail.com>
Date: Tue, 13 Dec 2022 06:26:53 -0800
Subject: [PATCH] feat(napi): improve napi coverage (#16198)

---
 cli/napi/js_native_api.rs | 366 +++++++++++++++++++++-----------------
 ext/napi/lib.rs           |   2 +
 2 files changed, 202 insertions(+), 166 deletions(-)

diff --git a/cli/napi/js_native_api.rs b/cli/napi/js_native_api.rs
index 1c3b02fa91..dedea8f799 100644
--- a/cli/napi/js_native_api.rs
+++ b/cli/napi/js_native_api.rs
@@ -3,6 +3,7 @@
 #![allow(non_upper_case_globals)]
 
 use deno_runtime::deno_napi::*;
+use libc::INT_MAX;
 use v8::BackingStore;
 use v8::UniqueRef;
 
@@ -36,9 +37,7 @@ macro_rules! check_arg_option {
 fn napi_create_array(env: *mut Env, result: *mut napi_value) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
   check_arg!(result);
-
-  let value = v8::Array::new(&mut env.scope(), 0);
-  *result = value.into();
+  *result = v8::Array::new(&mut env.scope(), 0).into();
   Ok(())
 }
 
@@ -50,9 +49,7 @@ fn napi_create_array_with_length(
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
   check_arg!(result);
-
-  let value = v8::Array::new(&mut env.scope(), len);
-  *result = value.into();
+  *result = v8::Array::new(&mut env.scope(), len).into();
   Ok(())
 }
 
@@ -83,9 +80,7 @@ fn napi_create_bigint_int64(
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
   check_arg!(result);
-
-  let value = v8::BigInt::new_from_i64(&mut env.scope(), value);
-  *result = value.into();
+  *result = v8::BigInt::new_from_i64(&mut env.scope(), value).into();
   Ok(())
 }
 
@@ -96,9 +91,8 @@ fn napi_create_bigint_uint64(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> =
-    v8::BigInt::new_from_u64(&mut env.scope(), value).into();
-  *result = value.into();
+  check_arg!(result);
+  *result = v8::BigInt::new_from_u64(&mut env.scope(), value).into();
   Ok(())
 }
 
@@ -111,14 +105,26 @@ fn napi_create_bigint_words(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> = v8::BigInt::new_from_words(
+  check_arg!(words);
+  check_arg!(result);
+
+  if word_count > INT_MAX as _ {
+    return Err(Error::InvalidArg);
+  }
+
+  match v8::BigInt::new_from_words(
     &mut env.scope(),
     sign_bit,
     std::slice::from_raw_parts(words, word_count),
-  )
-  .unwrap()
-  .into();
-  *result = value.into();
+  ) {
+    Some(value) => {
+      *result = value.into();
+    }
+    None => {
+      return Err(Error::InvalidArg);
+    }
+  }
+
   Ok(())
 }
 
@@ -272,9 +278,8 @@ fn napi_create_double(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> =
-    v8::Number::new(&mut env.scope(), value).into();
-  *result = value.into();
+  check_arg!(result);
+  *result = v8::Number::new(&mut env.scope(), value).into();
   Ok(())
 }
 
@@ -398,33 +403,29 @@ fn napi_create_external_buffer(
 fn napi_create_function(
   env_ptr: *mut Env,
   name: *const u8,
-  length: isize,
+  length: usize,
   cb: napi_callback,
   cb_info: napi_callback_info,
   result: *mut napi_value,
 ) -> Result {
   let _: &mut Env = env_ptr.as_mut().ok_or(Error::InvalidArg)?;
-  let name = match name.is_null() {
-    true => None,
-    false => Some(name),
-  };
-  let name = name.map(|name| {
-    if length == -1 {
-      std::ffi::CStr::from_ptr(name as *const _).to_str().unwrap()
-    } else {
-      let name = std::slice::from_raw_parts(name, length as usize);
-      // If ends with NULL
-      if name[name.len() - 1] == 0 {
-        std::str::from_utf8(&name[0..name.len() - 1]).unwrap()
-      } else {
-        std::str::from_utf8(name).unwrap()
-      }
-    }
-  });
+  check_arg!(result);
+  check_arg_option!(cb);
+  check_arg!(name);
 
-  let function = create_function(env_ptr, name, cb, cb_info);
-  let value: v8::Local<v8::Value> = function.into();
-  *result = value.into();
+  if length > INT_MAX as _ {
+    return Err(Error::InvalidArg);
+  }
+
+  let name = std::slice::from_raw_parts(name, length);
+  // If it ends with NULL
+  let name = if name[name.len() - 1] == 0 {
+    std::str::from_utf8_unchecked(&name[0..name.len() - 1])
+  } else {
+    std::str::from_utf8_unchecked(name)
+  };
+
+  *result = create_function(env_ptr, Some(name), cb, cb_info).into();
   Ok(())
 }
 
@@ -435,9 +436,20 @@ fn napi_create_int32(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> =
-    v8::Number::new(&mut env.scope(), value as f64).into();
-  *result = value.into();
+  check_arg!(result);
+  *result = v8::Integer::new(&mut env.scope(), value).into();
+  Ok(())
+}
+
+#[napi_sym::napi_sym]
+fn napi_create_uint32(
+  env: *mut Env,
+  value: u32,
+  result: *mut napi_value,
+) -> Result {
+  let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  check_arg!(result);
+  *result = v8::Integer::new_from_unsigned(&mut env.scope(), value).into();
   Ok(())
 }
 
@@ -448,9 +460,8 @@ fn napi_create_int64(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> =
-    v8::Number::new(&mut env.scope(), value as f64).into();
-  *result = value.into();
+  check_arg!(result);
+  *result = v8::Number::new(&mut env.scope(), value as f64).into();
   Ok(())
 }
 
@@ -519,12 +530,20 @@ fn napi_create_reference(
 fn napi_create_string_latin1(
   env: *mut Env,
   string: *const u8,
-  length: isize,
+  length: usize,
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  if length > 0 {
+    check_arg!(string);
+  }
+  check_arg!(result);
+  let safe_len = (length == NAPI_AUTO_LENGTH) || length <= INT_MAX as _;
+  if !safe_len {
+    return Err(Error::InvalidArg);
+  }
 
-  let string = if length == -1 {
+  let string = if length == NAPI_AUTO_LENGTH {
     std::ffi::CStr::from_ptr(string as *const _)
       .to_str()
       .unwrap()
@@ -538,8 +557,7 @@ fn napi_create_string_latin1(
     v8::NewStringType::Normal,
   ) {
     Some(v8str) => {
-      let value: v8::Local<v8::Value> = v8str.into();
-      *result = value.into();
+      *result = v8str.into();
     }
     None => return Err(Error::GenericFailure),
   }
@@ -555,15 +573,34 @@ fn napi_create_string_utf16(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let string = std::slice::from_raw_parts(string, length);
-  let v8str = v8::String::new_from_two_byte(
+  if length > 0 {
+    check_arg!(string);
+  }
+  check_arg!(result);
+  let safe_len = (length == NAPI_AUTO_LENGTH) || length <= INT_MAX as _;
+  if !safe_len {
+    return Err(Error::InvalidArg);
+  }
+
+  let string = if length == NAPI_AUTO_LENGTH {
+    let s = std::ffi::CStr::from_ptr(string as *const _)
+      .to_str()
+      .unwrap();
+    std::slice::from_raw_parts(s.as_ptr() as *const u16, s.len())
+  } else {
+    std::slice::from_raw_parts(string, length)
+  };
+
+  match v8::String::new_from_two_byte(
     &mut env.scope(),
     string,
     v8::NewStringType::Normal,
-  )
-  .unwrap();
-  let value: v8::Local<v8::Value> = v8str.into();
-  *result = value.into();
+  ) {
+    Some(v8str) => {
+      *result = v8str.into();
+    }
+    None => return Err(Error::GenericFailure),
+  }
   Ok(())
 }
 
@@ -571,12 +608,20 @@ fn napi_create_string_utf16(
 fn napi_create_string_utf8(
   env: *mut Env,
   string: *const u8,
-  length: isize,
+  length: usize,
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  if length > 0 {
+    check_arg!(string);
+  }
+  check_arg!(result);
+  let safe_len = (length == NAPI_AUTO_LENGTH) || length <= INT_MAX as _;
+  if !safe_len {
+    return Err(Error::InvalidArg);
+  }
 
-  let string = if length == -1 {
+  let string = if length == NAPI_AUTO_LENGTH {
     std::ffi::CStr::from_ptr(string as *const _)
       .to_str()
       .unwrap()
@@ -585,8 +630,7 @@ fn napi_create_string_utf8(
     std::str::from_utf8(string).unwrap()
   };
   let v8str = v8::String::new(&mut env.scope(), string).unwrap();
-  let value: v8::Local<v8::Value> = v8str.into();
-  *result = value.into();
+  *result = v8str.into();
 
   Ok(())
 }
@@ -598,16 +642,16 @@ fn napi_create_symbol(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let description = match description.is_none() {
-    true => None,
-    false => Some(
-      transmute::<napi_value, v8::Local<v8::Value>>(description)
-        .to_string(&mut env.scope())
-        .unwrap(),
-    ),
-  };
-  let sym = v8::Symbol::new(&mut env.scope(), description);
-  *result = sym.into();
+  check_arg!(result);
+
+  let scope = &mut env.scope();
+  let description = description
+    .map(|d| match d.to_string(scope) {
+      Some(s) => Ok(s),
+      None => Err(Error::StringExpected),
+    })
+    .transpose()?;
+  *result = v8::Symbol::new(scope, description).into();
   Ok(())
 }
 
@@ -707,19 +751,6 @@ fn napi_create_typedarray(
   Ok(())
 }
 
-#[napi_sym::napi_sym]
-fn napi_create_uint32(
-  env: *mut Env,
-  value: u32,
-  result: *mut napi_value,
-) -> Result {
-  let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> =
-    v8::Number::new(&mut env.scope(), value as f64).into();
-  *result = value.into();
-  Ok(())
-}
-
 #[napi_sym::napi_sym]
 fn napi_make_callback(
   env: *mut Env,
@@ -1082,7 +1113,7 @@ fn napi_define_class(
 ) -> Result {
   let env: &mut Env = env_ptr.as_mut().ok_or(Error::InvalidArg)?;
   check_arg!(result);
-  // check_arg!(constructor as *const c_void);
+  check_arg_option!(constructor);
 
   if property_count > 0 {
     check_arg!(properties);
@@ -1151,21 +1182,6 @@ fn napi_define_class(
         setter,
         accessor_property,
       );
-
-      // // TODO: use set_accessor & set_accessor_with_setter
-      // match (getter, setter) {
-      //   (Some(getter), None) => {
-      //     proto.set(name.into(), getter.into());
-      //   }
-      //   (Some(getter), Some(setter)) => {
-      //     proto.set(name.into(), getter.into());
-      //     proto.set(name.into(), setter.into());
-      //   }
-      //   (None, Some(setter)) => {
-      //     proto.set(name.into(), setter.into());
-      //   }
-      //   (None, None) => unreachable!(),
-      // }
     } else if method.is_some() {
       let function = create_function_template(env_ptr, None, p.method, p.data);
       let proto = tpl.prototype_template(scope);
@@ -1236,19 +1252,23 @@ fn napi_delete_element(
 #[napi_sym::napi_sym]
 fn napi_delete_property(
   env: *mut Env,
-  value: napi_value,
+  object: napi_value,
   key: napi_value,
   result: *mut bool,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value = transmute::<napi_value, v8::Local<v8::Value>>(value);
-  let obj = value.to_object(&mut env.scope()).unwrap();
-  *result = obj
-    .delete(
-      &mut env.scope(),
-      transmute::<napi_value, v8::Local<v8::Value>>(key),
-    )
-    .unwrap_or(false);
+  check_arg_option!(key);
+  check_arg!(result);
+
+  let scope = &mut env.scope();
+  let object = object
+    .map(|o| o.to_object(scope))
+    .flatten()
+    .ok_or(Error::InvalidArg)?;
+
+  *result = object
+    .delete(scope, key.unwrap_unchecked())
+    .ok_or(Error::GenericFailure)?;
   Ok(())
 }
 
@@ -1332,9 +1352,8 @@ fn napi_get_boolean(
   result: *mut napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> =
-    v8::Boolean::new(env.isolate(), value).into();
-  *result = value.into();
+  check_arg!(result);
+  *result = v8::Boolean::new(env.isolate(), value).into();
   Ok(())
 }
 
@@ -1362,13 +1381,16 @@ fn napi_get_buffer_info(
 
 #[napi_sym::napi_sym]
 fn napi_get_cb_info(
-  _env: *mut Env,
+  env: *mut Env,
   cbinfo: napi_callback_info,
   argc: *mut i32,
   argv: *mut napi_value,
   this_arg: *mut napi_value,
   cb_data: *mut *mut c_void,
 ) -> Result {
+  let _: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  check_arg!(cbinfo);
+
   let cbinfo: &CallbackInfo = &*(cbinfo as *const CallbackInfo);
   let args = &*(cbinfo.args as *const v8::FunctionCallbackArguments);
 
@@ -1519,9 +1541,8 @@ fn napi_get_new_target(
 #[napi_sym::napi_sym]
 fn napi_get_null(env: *mut Env, result: *mut napi_value) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-
-  let value: v8::Local<v8::Value> = v8::null(env.isolate()).into();
-  *result = value.into();
+  check_arg!(result);
+  *result = v8::null(env.isolate()).into();
   Ok(())
 }
 
@@ -1611,8 +1632,8 @@ fn napi_get_typedarray_info(
 #[napi_sym::napi_sym]
 fn napi_get_undefined(env: *mut Env, result: *mut napi_value) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value: v8::Local<v8::Value> = v8::undefined(env.isolate()).into();
-  *result = value.into();
+  check_arg!(result);
+  *result = v8::undefined(env.isolate()).into();
   Ok(())
 }
 
@@ -1662,25 +1683,23 @@ fn napi_has_own_property(
   result: *mut bool,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value = transmute::<napi_value, v8::Local<v8::Value>>(object);
-  let object = value.to_object(&mut env.scope()).unwrap();
+  check_arg_option!(key);
+  check_arg!(result);
 
-  let key = transmute::<napi_value, v8::Local<v8::Value>>(key);
-  if !key.is_name() {
-    return Err(Error::NameExpected);
-  }
+  let scope = &mut env.scope();
+  let object = object
+    .map(|o| o.to_object(scope))
+    .flatten()
+    .ok_or(Error::InvalidArg)?;
 
-  let maybe = object
-    .has_own_property(
-      &mut env.scope(),
-      v8::Local::<v8::Name>::try_from(key).unwrap(),
-    )
-    .unwrap_or(false);
+  let key = key
+    .map(v8::Local::<v8::Name>::try_from)
+    .ok_or(Error::InvalidArg)?
+    .map_err(|_| Error::NameExpected)?;
 
-  *result = maybe;
-  if !maybe {
-    return Err(Error::GenericFailure);
-  }
+  *result = object
+    .has_own_property(scope, key)
+    .ok_or(Error::GenericFailure)?;
 
   Ok(())
 }
@@ -1688,19 +1707,23 @@ fn napi_has_own_property(
 #[napi_sym::napi_sym]
 fn napi_has_property(
   env: *mut Env,
-  value: napi_value,
+  object: napi_value,
   key: napi_value,
   result: *mut bool,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let value = transmute::<napi_value, v8::Local<v8::Value>>(value);
-  let obj = value.to_object(&mut env.scope()).unwrap();
-  *result = obj
-    .has(
-      &mut env.scope(),
-      transmute::<napi_value, v8::Local<v8::Value>>(key),
-    )
-    .unwrap_or(false);
+  check_arg_option!(key);
+  check_arg!(result);
+
+  let scope = &mut env.scope();
+  let object = object
+    .map(|o| o.to_object(scope))
+    .flatten()
+    .ok_or(Error::InvalidArg)?;
+
+  *result = object
+    .has(scope, key.unwrap_unchecked())
+    .ok_or(Error::GenericFailure)?;
   Ok(())
 }
 
@@ -2068,28 +2091,38 @@ fn napi_set_named_property(
 fn napi_set_property(
   env: *mut Env,
   object: napi_value,
-  property: napi_value,
+  key: napi_value,
   value: napi_value,
 ) -> Result {
   let env: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
-  let object = transmute::<napi_value, v8::Local<v8::Value>>(object);
-  let object = object.to_object(&mut env.scope()).unwrap();
-  let property = transmute::<napi_value, v8::Local<v8::Value>>(property);
-  let value = transmute::<napi_value, v8::Local<v8::Value>>(value);
-  object.set(&mut env.scope(), property, value).unwrap();
+  check_arg_option!(key);
+  check_arg_option!(value);
+
+  let scope = &mut env.scope();
+  let object = object
+    .map(|o| o.to_object(scope))
+    .flatten()
+    .ok_or(Error::InvalidArg)?;
+
+  object
+    .set(scope, key.unwrap_unchecked(), value.unwrap_unchecked())
+    .ok_or(Error::GenericFailure)?;
+
   Ok(())
 }
 
 #[napi_sym::napi_sym]
 fn napi_strict_equals(
-  _env: *mut Env,
+  env: *mut Env,
   lhs: napi_value,
   rhs: napi_value,
   result: *mut bool,
 ) -> Result {
-  let lhs = transmute::<napi_value, v8::Local<v8::Value>>(lhs);
-  let rhs = transmute::<napi_value, v8::Local<v8::Value>>(rhs);
-  *result = lhs.strict_equals(rhs);
+  let _: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  check_arg_option!(lhs);
+  check_arg_option!(rhs);
+
+  *result = lhs.unwrap_unchecked().strict_equals(rhs.unwrap_unchecked());
   Ok(())
 }
 
@@ -2189,22 +2222,23 @@ pub fn get_value_type(value: v8::Local<v8::Value>) -> Option<napi_valuetype> {
 
 #[napi_sym::napi_sym]
 fn napi_typeof(
-  _env: *mut Env,
+  env: *mut Env,
   value: napi_value,
   result: *mut napi_valuetype,
 ) -> Result {
-  if value.is_none() {
-    *result = napi_undefined;
-    return Ok(());
-  }
-  let value = transmute::<napi_value, v8::Local<v8::Value>>(value);
-  let ty = get_value_type(value);
-  if let Some(ty) = ty {
-    *result = ty;
-    Ok(())
-  } else {
-    Err(Error::InvalidArg)
-  }
+  let _: &mut Env = env.as_mut().ok_or(Error::InvalidArg)?;
+  check_arg_option!(value);
+  check_arg!(result);
+  match get_value_type(value.unwrap()) {
+    Some(ty) => {
+      *result = ty;
+    }
+    None => {
+      return Err(Error::InvalidArg);
+    }
+  };
+
+  Ok(())
 }
 
 #[napi_sym::napi_sym]
diff --git a/ext/napi/lib.rs b/ext/napi/lib.rs
index 5fd3085ca9..11619398d7 100644
--- a/ext/napi/lib.rs
+++ b/ext/napi/lib.rs
@@ -75,6 +75,8 @@ pub const napi_arraybuffer_expected: napi_status = 19;
 pub const napi_detachable_arraybuffer_expected: napi_status = 20;
 pub const napi_would_deadlock: napi_status = 21;
 
+pub const NAPI_AUTO_LENGTH: usize = usize::MAX;
+
 thread_local! {
   pub static MODULE: RefCell<Option<*const NapiModule>> = RefCell::new(None);
   pub static ASYNC_WORK_SENDER: RefCell<Option<mpsc::UnboundedSender<PendingNapiAsyncWork>>> = RefCell::new(None);