Merge pull request #584 from napi-rs/fix-js-string-leak

fix(napi): JsString* leak in from_js_value

napi/src/js_values/string/mod.rs

@@ -58,13 +58,9 @@ impl JsString {
       )
     })?;
 
-    mem::forget(result);
-
     Ok(JsStringUtf8 {
       inner: self,
-      buf: mem::ManuallyDrop::new(unsafe {
-        Vec::from_raw_parts(buf_ptr, written_char_count, written_char_count)
-      }),
+      buf: result,
     })
   }
 
@@ -83,13 +79,10 @@ impl JsString {
         &mut written_char_count,
       )
     })?;
-    mem::forget(result);
 
     Ok(JsStringUtf16 {
       inner: self,
-      buf: mem::ManuallyDrop::new(unsafe {
-        Vec::from_raw_parts(buf_ptr, written_char_count, written_char_count)
-      }),
+      buf: result,
     })
   }
 

napi/src/js_values/string/utf16.rs

@@ -1,12 +1,11 @@
-use std::mem;
+use std::convert::TryFrom;
 use std::ops::Deref;
-use std::{convert::TryFrom, mem::ManuallyDrop};
 
 use crate::{Error, JsString, Result, Status};
 
 pub struct JsStringUtf16 {
   pub(crate) inner: JsString,
-  pub(crate) buf: mem::ManuallyDrop<Vec<u16>>,
+  pub(crate) buf: Vec<u16>,
 }
 
 impl JsStringUtf16 {
@@ -33,7 +32,6 @@ impl JsStringUtf16 {
 
   #[inline]
   pub fn into_value(self) -> JsString {
-    ManuallyDrop::into_inner(self.buf);
     self.inner
   }
 }
@@ -62,6 +60,6 @@ impl AsRef<Vec<u16>> for JsStringUtf16 {
 
 impl From<JsStringUtf16> for Vec<u16> {
   fn from(value: JsStringUtf16) -> Self {
-    ManuallyDrop::into_inner(value.buf)
+    value.as_slice().to_vec()
   }
 }

napi/src/js_values/string/utf8.rs

@@ -1,6 +1,5 @@
 use std::convert::TryFrom;
 use std::ffi::CStr;
-use std::mem::ManuallyDrop;
 use std::os::raw::c_char;
 use std::str;
 
@@ -8,7 +7,7 @@ use crate::{Error, JsString, Result, Status};
 
 pub struct JsStringUtf8 {
   pub(crate) inner: JsString,
-  pub(crate) buf: ManuallyDrop<Vec<c_char>>,
+  pub(crate) buf: Vec<c_char>,
 }
 
 impl JsStringUtf8 {
@@ -36,24 +35,16 @@ impl JsStringUtf8 {
 
   #[inline]
   pub fn into_owned(self) -> Result<String> {
-    let buffer = ManuallyDrop::into_inner(self.buf);
-
-    let s = unsafe { CStr::from_ptr(buffer.as_ptr()) }
-      .to_str()
-      .map_err(|e| Error::new(Status::InvalidArg, format!("{}", e)))?;
-
-    Ok(s.to_owned())
+    Ok(self.as_str()?.to_owned())
   }
 
   #[inline]
   pub fn take(self) -> Vec<u8> {
-    let bytes = unsafe { CStr::from_ptr(ManuallyDrop::into_inner(self.buf).as_ptr()) }.to_bytes();
-    bytes.to_vec()
+    self.as_slice().to_vec()
   }
 
   #[inline]
   pub fn into_value(self) -> JsString {
-    ManuallyDrop::into_inner(self.buf);
     self.inner
   }
 }