Merge pull request #1023 from napi-rs/fix/get-str-from-object

fix(napi): invalid memory address in FromNapiValue for &str

.cirrus.yml

@@ -11,7 +11,9 @@ task:
     RUSTUP_IO_THREADS: 1
   setup_script:
     - pkg update
-    - pkg install -y -f curl node yarn npm libnghttp2
+    - pkg install -y -f curl node16 libnghttp2
+    - curl -qL https://www.npmjs.com/install.sh | sh
+    - npm install -g yarn
     - curl https://sh.rustup.rs -sSf --output rustup.sh
     - sh rustup.sh -y --profile minimal --default-toolchain stable
     - |

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

@@ -1,6 +1,6 @@
 use crate::{bindgen_prelude::*, check_status, sys, Error, Result, Status};
 
-use std::ffi::CStr;
+use std::ffi::{c_void, CStr};
 use std::fmt::Display;
 #[cfg(feature = "latin1")]
 use std::mem;
@@ -84,7 +84,6 @@ impl FromNapiValue for &str {
     len += 1;
     let mut ret = Vec::with_capacity(len);
     let buf_ptr = ret.as_mut_ptr();
-
     let mut written_char_count = 0;
 
     check_status!(
@@ -92,6 +91,22 @@ impl FromNapiValue for &str {
       "Failed to convert napi `string` into rust type `String`"
     )?;
 
+    // The `&str` should only be accepted from function arguments.
+    // We shouldn't implement `FromNapiValue` for it before.
+    // When it's used with `Object.get` scenario, the memory which `&str` point to will be invalid.
+    // For this scenario, we create a temporary empty `Object` here and assign the `Vec<u8>` under `&str` to it.
+    // So we can safely forget the `Vec<u8>` here which could fix the memory issue here.
+    // FIXME: This implementation should be removed in next major release.
+    let mut temporary_external_object = ptr::null_mut();
+    check_status!(sys::napi_create_external(
+      env,
+      buf_ptr as *mut c_void,
+      Some(release_string),
+      Box::into_raw(Box::new(len)) as *mut c_void,
+      &mut temporary_external_object,
+    ))?;
+
+    std::mem::forget(ret);
     match CStr::from_ptr(buf_ptr).to_str() {
       Err(e) => Err(Error::new(
         Status::InvalidArg,
@@ -279,3 +294,8 @@ pub mod latin1_string {
     }
   }
 }
+
+unsafe extern "C" fn release_string(_env: sys::napi_env, data: *mut c_void, len: *mut c_void) {
+  let len = *Box::from_raw(len as *mut usize);
+  Vec::from_raw_parts(data as *mut u8, len, len);
+}

examples/napi/__test__/typegen.spec.ts.md

@@ -100,6 +100,7 @@ Generated by [AVA](https://avajs.dev).
       name: string␊
     }␊
     export function receiveStrictObject(strictObject: StrictObject): void␊
+    export function getStrFromObject(): void␊
     export function asyncPlus100(p: Promise<number>): Promise<number>␊
     /** This is an interface for package.json */␊
     export interface PackageJson {␊

examples/napi/__test__/values.spec.ts

@@ -76,6 +76,7 @@ import {
   receiveClassOrNumber,
   JsClassForEither,
   receiveMutClassOrNumber,
+  getStrFromObject,
 } from '../'
 
 test('export const', (t) => {
@@ -197,6 +198,10 @@ test('object', (t) => {
   t.deepEqual(createObj(), { test: 1 })
 })
 
+test('get str from object', (t) => {
+  t.notThrows(() => getStrFromObject())
+})
+
 test('global', (t) => {
   t.is(getGlobal(), global)
 })

examples/napi/index.d.ts

@@ -90,6 +90,7 @@ export interface StrictObject {
   name: string
 }
 export function receiveStrictObject(strictObject: StrictObject): void
+export function getStrFromObject(): void
 export function asyncPlus100(p: Promise<number>): Promise<number>
 /** This is an interface for package.json */
 export interface PackageJson {

examples/napi/src/object.rs

@@ -69,3 +69,10 @@ pub struct StrictObject {
 pub fn receive_strict_object(strict_object: StrictObject) {
   assert_eq!(strict_object.name, "strict");
 }
+
+#[napi]
+pub fn get_str_from_object(env: Env) {
+  let mut obj = env.create_object().unwrap();
+  obj.set("name", "value").unwrap();
+  assert_eq!(obj.get("name").unwrap(), Some("value"));
+}