Skip to content

Commit

Permalink
n-api: Implement stricter wrapping
Browse files Browse the repository at this point in the history
Use a stronger criterion to identify objects in the prototype chain that store
pointers to native data that were added by previous calls to `napi_wrap()`.

Whereas the old criterion for identifying `napi_wrap()`-injected prototype
chain objects was to consider an object with an internal field
count of 1 to be such an object, the new criterion is to consider an object
with an internal field count of 2 such that the second field holds a
`v8::External` which itself contains a pointer to a global static string unique
to N-API to be a `napi_wrap()`-injected prototype chain object.

This greatly reduces the possibility of returning a pointer that was not
previously added with `napi_wrap()`, and it allows us to recognize that an
object has already undergone `napi_wrap()` and we can thus prevent a chain of
wrappers only the first of which is accessible from appearing in the prototype
chain, as would be the result of multiple calls to `napi_wrap()` using the same
object.

PR-URL: #13872
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
  • Loading branch information
Gabriel Schulhof authored and addaleax committed Jul 18, 2017
1 parent 3bc713e commit f2054f3
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 16 deletions.
8 changes: 6 additions & 2 deletions doc/api/n-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2876,8 +2876,8 @@ napi_status napi_wrap(napi_env env,
Returns `napi_ok` if the API succeeded.
Wraps a native instance in JavaScript object of the corresponding type.
The native instance can be retrieved later using `napi_unwrap()`.
Wraps a native instance in a JavaScript object. The native instance can be
retrieved later using `napi_unwrap()`.
When JavaScript code invokes a constructor for a class that was defined using
`napi_define_class()`, the `napi_callback` for the constructor is invoked.
Expand Down Expand Up @@ -2905,6 +2905,10 @@ required in order to enable correct proper of the reference.
Afterward, additional manipulation of the wrapper's prototype chain may cause
`napi_unwrap()` to fail.
*Note*: Calling `napi_wrap()` a second time on an object that already has a
native instance associated with it by virtue of a previous call to
`napi_wrap()` will cause an error to be returned.
### *napi_unwrap*
<!-- YAML
added: v8.0.0
Expand Down
64 changes: 50 additions & 14 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,38 @@ v8::Local<v8::Object> CreateAccessorCallbackData(napi_env env,
return cbdata;
}

// Pointer used to identify items wrapped by N-API. Used by FindWrapper and
// napi_wrap().
const char napi_wrap_name[] = "N-API Wrapper";

// Search the object's prototype chain for the wrapper object. Usually the
// wrapper would be the first in the chain, but it is OK for other objects to
// be inserted in the prototype chain.
bool FindWrapper(v8::Local<v8::Object> obj,
v8::Local<v8::Object>* result = nullptr) {
v8::Local<v8::Object> wrapper = obj;

do {
v8::Local<v8::Value> proto = wrapper->GetPrototype();
if (proto.IsEmpty() || !proto->IsObject()) {
return false;
}
wrapper = proto.As<v8::Object>();
if (wrapper->InternalFieldCount() == 2) {
v8::Local<v8::Value> external = wrapper->GetInternalField(1);
if (external->IsExternal() &&
external.As<v8::External>()->Value() == v8impl::napi_wrap_name) {
break;
}
}
} while (true);

if (result != nullptr) {
*result = wrapper;
}
return true;
}

} // end of namespace v8impl

// Intercepts the Node-V8 module registration callback. Converts parameters
Expand Down Expand Up @@ -2046,11 +2078,22 @@ napi_status napi_wrap(napi_env env,
RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
v8::Local<v8::Object> obj = value.As<v8::Object>();

// Create a wrapper object with an internal field to hold the wrapped pointer.
// If we've already wrapped this object, we error out.
RETURN_STATUS_IF_FALSE(env, !v8impl::FindWrapper(obj), napi_invalid_arg);

// Create a wrapper object with an internal field to hold the wrapped pointer
// and a second internal field to identify the owner as N-API.
v8::Local<v8::ObjectTemplate> wrapper_template;
ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, 1);
v8::Local<v8::Object> wrapper =
wrapper_template->NewInstance(context).ToLocalChecked();
ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, 2);

auto maybe_object = wrapper_template->NewInstance(context);
CHECK_MAYBE_EMPTY(env, maybe_object, napi_generic_failure);

v8::Local<v8::Object> wrapper = maybe_object.ToLocalChecked();
wrapper->SetInternalField(1, v8::External::New(isolate,
reinterpret_cast<void*>(const_cast<char*>(v8impl::napi_wrap_name))));

// Store the pointer as an external in the wrapper.
wrapper->SetInternalField(0, v8::External::New(isolate, native_object));

// Insert the wrapper into the object's prototype chain.
Expand Down Expand Up @@ -2087,16 +2130,9 @@ napi_status napi_unwrap(napi_env env, napi_value js_object, void** result) {
RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
v8::Local<v8::Object> obj = value.As<v8::Object>();

// Search the object's prototype chain for the wrapper with an internal field.
// Usually the wrapper would be the first in the chain, but it is OK for
// other objects to be inserted in the prototype chain.
v8::Local<v8::Object> wrapper = obj;
do {
v8::Local<v8::Value> proto = wrapper->GetPrototype();
RETURN_STATUS_IF_FALSE(
env, !proto.IsEmpty() && proto->IsObject(), napi_invalid_arg);
wrapper = proto.As<v8::Object>();
} while (wrapper->InternalFieldCount() != 1);
v8::Local<v8::Object> wrapper;
RETURN_STATUS_IF_FALSE(
env, v8impl::FindWrapper(obj, &wrapper), napi_invalid_arg);

v8::Local<v8::Value> unwrappedValue = wrapper->GetInternalField(0);
RETURN_STATUS_IF_FALSE(env, unwrappedValue->IsExternal(), napi_invalid_arg);
Expand Down
8 changes: 8 additions & 0 deletions test/addons-napi/test_general/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,11 @@ assert.strictEqual(test_general.testGetVersion(), 1);
// since typeof in js return object need to validate specific case
// for null
assert.strictEqual(test_general.testNapiTypeof(null), 'null');

const x = {};

// Assert that wrapping twice fails.
test_general.wrap(x, 25);
assert.throws(function() {
test_general.wrap(x, 'Blah');
}, Error);
19 changes: 19 additions & 0 deletions test/addons-napi/test_general/test_general.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,24 @@ napi_value testNapiTypeof(napi_env env, napi_callback_info info) {
return result;
}

static void deref_item(napi_env env, void* data, void* hint) {
(void) hint;

NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, (napi_ref)data));
}

napi_value wrap(napi_env env, napi_callback_info info) {
size_t argc = 2;
napi_value argv[2];
napi_ref payload;

NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL));
NAPI_CALL(env, napi_create_reference(env, argv[1], 1, &payload));
NAPI_CALL(env, napi_wrap(env, argv[0], payload, deref_item, NULL, NULL));

return NULL;
}

void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
napi_property_descriptor descriptors[] = {
DECLARE_NAPI_PROPERTY("testStrictEquals", testStrictEquals),
Expand All @@ -130,6 +148,7 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
DECLARE_NAPI_PROPERTY("createNapiError", createNapiError),
DECLARE_NAPI_PROPERTY("testNapiErrorCleanup", testNapiErrorCleanup),
DECLARE_NAPI_PROPERTY("testNapiTypeof", testNapiTypeof),
DECLARE_NAPI_PROPERTY("wrap", wrap),
};

NAPI_CALL_RETURN_VOID(env, napi_define_properties(
Expand Down

0 comments on commit f2054f3

Please sign in to comment.