Skip to content

Commit

Permalink
Check hasOwnProperty after conversion to string
Browse files Browse the repository at this point in the history
I thought I could be clever and micro-optimize this a bit, but
`has_own_property` really does require a string argument, so we have to
do the conversion first.
  • Loading branch information
goto-bus-stop committed Apr 8, 2020
1 parent f6b1c4f commit 9b338f9
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 13 deletions.
24 changes: 12 additions & 12 deletions crates/neon-runtime/src/napi/object.rs
Expand Up @@ -32,7 +32,7 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec
}

let raw_names = raw_names.assume_init();
let mut fixed_names = fixed_names.assume_init();
let fixed_names = fixed_names.assume_init();

*out = fixed_names;

Expand All @@ -47,17 +47,6 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec
continue;
}

let mut is_own_property = false;
// May return a non-OK status if `key` is not a string or a Symbol, but here it is always
// a string.
if napi::napi_has_own_property(env, object, property_name, &mut is_own_property as *mut _) != napi::napi_status::napi_ok {
return false;
}

if !is_own_property {
continue;
}

// Before https://github.com/nodejs/node/pull/27524, `napi_get_property_names` would return
// numbers for numeric indices instead of strings.
// Make sure we always return strings.
Expand All @@ -72,6 +61,17 @@ pub unsafe extern "C" fn get_own_property_names(out: &mut Local, env: Env, objec
property_name
};

let mut is_own_property = false;
// May return a non-OK status if `key` is not a string or a Symbol, but here it is always
// a string.
if napi::napi_has_own_property(env, object, property_name, &mut is_own_property as *mut _) != napi::napi_status::napi_ok {
return false;
}

if !is_own_property {
continue;
}

let mut dummy = false;
// If we can't convert assign to this array, something went wrong.
if !set_index(&mut dummy, env, fixed_names, fixed_len, property_name) {
Expand Down
2 changes: 1 addition & 1 deletion test/napi/lib/hello.js
Expand Up @@ -25,6 +25,6 @@ describe('hello', function() {
0: 1,
a: 1,
whatever: true
})
});
});
});

0 comments on commit 9b338f9

Please sign in to comment.