From 2de500f33b85de74b4ff7f0c0e79f49af21143fe Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 12 Jan 2022 15:19:05 +0800 Subject: [PATCH] fix(napi): invalid memory address in FromNapiValue for &str --- .../src/bindgen_runtime/js_values/string.rs | 24 ++++++++++++++++-- examples/napi/__test__/typegen.spec.ts.md | 1 + examples/napi/__test__/typegen.spec.ts.snap | Bin 2283 -> 2294 bytes examples/napi/__test__/values.spec.ts | 5 ++++ examples/napi/index.d.ts | 1 + examples/napi/src/object.rs | 7 +++++ 6 files changed, 36 insertions(+), 2 deletions(-) diff --git a/crates/napi/src/bindgen_runtime/js_values/string.rs b/crates/napi/src/bindgen_runtime/js_values/string.rs index c60c1bc1..d6439f11 100644 --- a/crates/napi/src/bindgen_runtime/js_values/string.rs +++ b/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` under `&str` to it. + // So we can safely forget the `Vec` 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); +} diff --git a/examples/napi/__test__/typegen.spec.ts.md b/examples/napi/__test__/typegen.spec.ts.md index ec3e0649..d698d541 100644 --- a/examples/napi/__test__/typegen.spec.ts.md +++ b/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): Promise␊ /** This is an interface for package.json */␊ export interface PackageJson {␊ diff --git a/examples/napi/__test__/typegen.spec.ts.snap b/examples/napi/__test__/typegen.spec.ts.snap index a29f5f016c95807cc3c9f12af1eadc54f783b05b..5fc4cad16f0549b5f9fd9ed7e3a70b8b8c125bc5 100644 GIT binary patch literal 2294 zcmVDUyk-`Y(cZ!>Et{ESQ^s2g8WVDbS-(aM|t{1XwZN0me4O?zD zFpMl`hLUKF$jOjnTX75|e~{j1w zRzLs7G}jTk|En6wGz!U(36@aJNFbAh1)At-%t*)vWWXcFm7p;|YW4H4VMsOo_Ot)| z>Diy3;m;pFdG_fa|NYrl&xuwM7rGU4Mg548=U=VCCNrDs&L-~CRLk|8XGr`h>9x;} z*Uv8u0K1Q6qDc@@rO45}W{IHDsec1b5W6^}p+!Ui4c;q!XVd zIm93pf#y^M=?kS3E{6Mt#cQS&QF6jEbmcH{N%Yc*L&3uCmjG2d zF@SKc7GN8-0-|-gaFuN}6@eF1JpywW5dr5ckrS@ii-UA9sNzTw=_HLb@83y}Cc~VK zgIg!9&xhau?J)FcD_b;)pMN&;j}l%05^HwAhsQ#DPXjyN+g$<)_U+t-UO=PBr@^>I zP?62T3gqQJxs^Nw0{F+=8+bh@FUbSBg*!tUFUfY341i;rFa@itFBD5cwtl0e_~M#8 zR^4lE`7-CDB#{aE5u6nQjSChw2^?&_%4Kqo;9I0oR1$NM8{>M!k_&@>n)J-2cwEG( zcBmGYSL-aGpg1z5a^wPMfl7@8J2gztTiu8{hjMkBJWICdvyV76INc;JjH)tPH-($ySk)0PsB#M zhjwK`J9&M9T3(VWGGieR%RcHvsn>myYFDvUgqr9J|G@J?`AU0fbgu)QqUmao>&bWaizgA z-m; z+EHY$Gk15BxX^!>RdoQNL$>VpEAmrTUrHR&j6kX^Nu^J@iaN=+{qDGZVPtRT#o+)G zUzew^bT){l6$p2IDwNIAzqL8V+MPeE4^6lQD^=(T`>Q)sgmvx-7M z(NoZ~3+Q@w0DP$jTYFCje@CI0+IkAS0+%)O@3FwNX=mYlCazDSHqly4w0bsDFM{1c z>nc;-_F=aJc1vUwL9<&^IaUpgNbM=4nndxTX)+A-g;_j-K8|A8H_}ki0qbVl?u53n zu>qG1W(60$x;A4MeAPB7C0hMiCm8s@1HCTQG&+$X`<=;sw1^M@wxdF)uwUim7h~{e zV17Dqny?xSaB4_D`f_L6l>fYUyMzRnZLNGBneUDJQ1(Mht1(BXrhj$dGv3`uQipl| zIIu>ZlMkk1Vxf}B2O?uz)gVoOu)7~1ZGE3|d@BIYg2sqNFjZ+ud`P*eAMv1xWF$^~ zci^v(ScZg8VgSX;K^*c3_m?Ytkno({#`T&zJedcxw%`*Q6_cJB`ykCzyKORj@l-a3 z!)MlmGtfpFf#>6z^^% z>4eX*b#=1RL%7T-1Jc4MZPaNLHnm;$DPFJ{^6?0uDo(xuaqN`YamAlf^| znZ2UAyB9QXo&|iQ`Tr3xx6zTQeT-Xd>C9{KK}5%F+y6UNpc6EUc70&irmeY{$T4=n znn9n5abobYcVIr((PVPEB#gkW=dh_aX46*A7}kx}0@q3j0dOWBEGl^-&;wbp8gxnI`vtPvG7&Pd19*Qf?T6E=2bX^H0LS09>~E&jN^unRgsR4b7&ul9M7q` zT^nWD-PK=qa}KcNJ3TILXg^JK#OgZCj!D5}ijOe@!HG8TpF^1a^4nb02zGZ);}I(9 z5Vr1iLe;nhjE4 zR&cCiX#T%)M)jyFtTLy|(?d7Qq!m_oe5zMUj;#_2n@6i4HRQNO0Tv%9D|9I^kerHf z61l%0?HI_p=@N#y*S(XYUf*%Vinp7c@Ah`<9&W6U0npa=*E1yJR27r}XgQ(xD5q7_ zrD1p8=Sq|>&tj)>h^}rhNwzBSGd%;IhkOXB&vQk1tg+p;Ydcq=ZJZ`oJjqns^!>^= QvB$Ol0scH^#3CF300+5Hw*UYD literal 2283 zcmVEj6cVjwrHS z?p`fbupf&E00000000B68QX5-#&z4GDB!m|=Vh9r2(^$M-(aM|t{1XwZ@s&g4O?zD zFc#S%Ig-XAXBf`Vw&ECQ{-Do&36c-!xAhZx&J0P7L{f4WF$|b9bMAAl<6nk;B!h|i z=~t$?j@bQQ)mUaxNJdPslxju-nWikzL{Ae&LN+8r9x<*2O#o7>pMD8Ls_D0%|K|_S z{`3rge)q>`pZ)&l&%b(3w2HXUoscW)M~poGY7I7-*<5!vahGOVt`|H*;!jDxdw#Nh zacKb9eIip$f`}?bj_x%}1&z-98<6Y8V4pm!5t1?*N)b)TmtS_w#==%6`1ctjY8v}8 zGRN0%JNfcMhYVoraqSUAc}@mnt_b{6&|N5v5)=ZM_2JQR_v+o?yWZL5phH9!`z$RW zhM5R7mjdjkzmaLEyjF*F)09qMD4lXK+BYm-Gp&e{G0V}F!^9=g&tivyh25_Jsw_5u zaIO|$Ta5yub&laGyJ{)|FQIx2<}e}x&Uq?huGou%Y&fjrND=8Yi!>kHNsp$Zf{laQ zB(2X!-~in)^k^qvw25DQwu+AmUH}qncECp`LVHgGJK5V^0txo*-GyF2qsXVhq(e}V z&C&|w5vADK&+dfzO8b5Y~Z)Mk{^5l z-DOG#jNh^zW}VV9;ZieppR>@c9V=T|g|B5DJWk;OTG&g58ypXUgicQ7>;V zQ(9uCJHizTekNEU3FILYjQBZD8Sd}k;#g)WeDC7-CBKKyJ^Vc4qcLjnH7@$x(D|D| zW*F34n{|E#YzOjWXWL_)d10FGT#r+EXTsWR3Q(J{imK)*{sVJ@YoXdTCDnN%HrhS3 zD-+tu>kHJ1l2nr!OL8f6y_#tlO?sdK(ghy9&d@&XmXa| zmj|)%%E=0q%)jKUA%XYd$TN72AXf#YMt`NYa$DY}a<8f$L+(pKgMfQtHE_h02FG~I zrplB4O@~P1C6zX+J^}y}1-CeLH~Kn~J~c+bhE@*A$ZXVst~Q0~(5y+{sJ>&=S>@_R zk-g5s-A&>`|6Nwq0fdg&vfHo8PkDW*a6~f#sj4KkKIJOvB;W44lkTOFy;~HA15A`N zIb59~%2)_lD9iN)e=!8$v-Fo54Tj>v>Ja86JnUy(YHh-DQVXKaxMT7Q^n$SqTQ{4{ zMe~%i=*&@Uj0)_C&!bFjZEku=r3EMedgBH&EDs-!L|ru+528nWBNPeQG`4H`PSVQ4wa zx(IZR`&br|*?JDQEDqJw0d^L80bs0cmjQ#u&^DZp`t@}oNv2>*T%*MTr!vyT=e>b znWo^YvG=IZ8q6BJ!2b>C9aBxCQyH?~nA|6e2mxRlC3Fh=RZV^g27eCbXG5pAYQO-e zhV-MacD7AbFIuuINO1Yq%2$v1mUjTvJhZf0b95ToQU^ZgeH=;ZFfV5QHS(N%G=&iB zk4!%jnb=|lY5Jqx{RnC6`;6lg0C*PkJ1l}>NlW5G%FXXTzIew&!Y0Yk?3YP?i8;N@4~XBD9oX2-Q3=VU8pmYIwayQh)*%Gqk;yQ;Srw zS;H0I?;O^48RK)*dW5rFAmbPYAG?ptvB0ue-f;5Ypa1<&vybB4jU|ow99vf>D?NnE zoH8ISOvpx^R%uh)JfGnOo7o+Y0IKHX8xY41k3GlC7%4De&kCZwV;se6s=IqZ^X6H= zN1Fd10dpHYnb|kC#ZJvU0v|+l!nXatQw2If4`|m1c5T|4%UDjZvDFOPM~oALmp||F zxsE26(|ejmMGegM7c?28k`7_(Zl_dDdNQ(M zP%^;bYCb8%uB`x*|Os=b5y|>W)wKE6K4_Az|}q9i)aFw~v-92FZqs{=^)Ue2+WvZmWSpvo5&$hH^d9B3in=uH&ig`%>g8GN zED6!o4JOG~6@I2?!1IugAoY2!2#*c6+jec|DzuH$ { @@ -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) }) diff --git a/examples/napi/index.d.ts b/examples/napi/index.d.ts index d6e712b8..09c4b881 100644 --- a/examples/napi/index.d.ts +++ b/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): Promise /** This is an interface for package.json */ export interface PackageJson { diff --git a/examples/napi/src/object.rs b/examples/napi/src/object.rs index 6ae0fa5b..bab1e228 100644 --- a/examples/napi/src/object.rs +++ b/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")); +}