8000 n-api: Implement stricter wrapping · nodejs/node@7973bd3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7973bd3

Browse files
Gabriel SchulhofMylesBorins
Gabriel Schulhof
authored and
MylesBorins
committed
n-api: Implement stricter wrapping
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. Backport-PR-URL: #19447 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>
1 parent 5b0c57c commit 7973bd3

File tree

4 files changed

+83
-16
lines changed

4 files changed

+83
-16
lines changed

‎doc/api/n-api.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2876,8 +2876,8 @@ napi_status napi_wrap(napi_env env,
28762876
28772877
Returns `napi_ok` if the API succeeded.
28782878
2879-
Wraps a native instance in JavaScript object of the corresponding type.
2880-
The native instance can be retrieved later using `napi_unwrap()`.
2879+
Wraps a native instance in a JavaScript object. The native instance can be
2880+
retrieved later using `napi_unwrap()`.
28812881
28822882
When JavaScript code invokes a constructor for a class that was defined using
28832883
`napi_define_class()`, the `napi_callback` for the constructor is invoked.
@@ -2905,6 +2905,10 @@ required in order to enable correct proper of the reference.
29052905
Afterward, additional manipulation of the wrapper's prototype chain may cause
29062906
`napi_unwrap()` to fail.
29072907
2908+
*Note*: Calling `napi_wrap()` a second time on an object that already has a
2909+
native instance associated with it by virtue of a previous call to
2910+
`napi_wrap()` will cause an error to be returned.
2911+
29082912
### *napi_unwrap*
29092913
<!-- YAML
29102914
added: v8.0.0

‎src/node_api.cc‎

Lines changed: 50 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,38 @@ v8::Local<v8::Object> CreateAccessorCallbackData(napi_env env,
678678
return cbdata;
679679
}
680680

681+
// Pointer used to identify items wrapped by N-API. Used by FindWrapper and
682+
// napi_wrap().
683+
const char napi_wrap_name[] = "N-API Wrapper";
684+
685+
// Search the object's prototype chain for the wrapper object. Usually the
686+
// wrapper would be the first in the chain, but it is OK for other objects to
687+
// be inserted in the prototype chain.
688+
bool FindWrapper(v8::Local<v8::Object> obj,
689+
v8::Local<v8::Object>* result = nullptr) {
690+
v8::Local<v8::Object> wrapper = obj;
691+
692+
do {
693+
v8::Local<v8::Value> proto = wrapper->GetPrototype();
694+
if (proto.IsEmpty() || !proto->IsObject()) {
695+
return false;
696+
}
697+
wrapper = proto.As<v8::Object>();
698+
if (wrapper->InternalFieldCount() == 2) {
699+
v8::Local<v8::Value> external = wrapper->GetInternalField(1);
700+
if (external->IsExternal() &&
701+
external.As<v8::External>()->Value() == v8impl::napi_wrap_name) {
702+
break;
703+
}
704+
}
705+
} while (true);
706+
707+
if (result != nullptr) {
708+
*result = wrapper;
709+
}
710+
return true;
711+
}
712+
681713
} // end of namespace v8impl
682714

683715
// Intercepts the Node-V8 module registration callback. Converts parameters
@@ -2049,11 +2081,22 @@ napi_status napi_wrap(napi_env env,
20492081
RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
20502082
v8::Local<v8::Object> obj = value.As<v8::Object>();
20512083

2052-
// Create a wrapper object with an internal field to hold the wrapped pointer.
2084+
// If we've already wrapped this object, we error out.
2085+
RETURN_STATUS_IF_FALSE(env, !v8impl::FindWrapper(obj), napi_invalid_arg);
2086+
2087+
// Create a wrapper object with an internal field to hold the wrapped pointer
2088+
// and a second internal field to identify the owner as N-API.
20532089
v8::Local<v8::ObjectTemplate> wrapper_template;
2054-
ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, 1);
2055-
v8::Local<v8::Object> wrapper =
2056-
wrapper_template->NewInstance(context).ToLocalChecked();
2090+
ENV_OBJECT_TEMPLATE(env, wrap, wrapper_template, 2);
2091+
2092+
auto maybe_object = wrapper_template->NewInstance(context);
2093+
CHECK_MAYBE_EMPTY(env, maybe_object, napi_generic_failure);
2094+
2095+
v8::Local<v8::Object> wrapper = maybe_object.ToLocalChecked();
2096+
wrapper->SetInternalField(1, v8::External::New(isolate,
2097+
reinterpret_cast<void*>(const_cast<char*>(v8impl::napi_wrap_name))));
2098+
2099+
// Store the pointer as an external in the wrapper.
20572100
wrapper->SetInternalField(0, v8::External::New(isolate, native_object));
20582101

20592102
// Insert the wrapper into the object's prototype chain.
@@ -2090,16 +2133,9 @@ napi_status napi_unwrap(napi_env env, napi_value js_object, void** result) {
20902133
RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
20912134
v8::Local<v8::Object> obj = value.As<v8::Object>();
20922135

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

21042140
v8::Local<v8::Value> unwrappedValue = wrapper->GetInternalField(0);
21052141
RETURN_STATUS_IF_FALSE(env, unwrappedValue->IsExternal(), napi_invalid_arg);

‎test/addons-napi/test_general/test.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,11 @@ assert.strictEqual(test_general.testGetVersion(), 1);
5050
// since typeof in js return object need to validate specific case
5151
// for null
5252
assert.strictEqual(test_general.testNapiTypeof(null), 'null');
53+
54+
const x = {};
55+
56+
// Assert that wrapping twice fails.
57+
test_general.wrap(x, 25);
58+
assert.throws(function() {
59+
test_general.wrap(x, 'Blah');
60+
}, Error);

‎test/addons-napi/test_general/test_general.c‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,24 @@ napi_value testNapiTypeof(napi_env env, napi_callback_info info) {
119119
return result;
120120
}
121121

122+
static void deref_item(napi_env env, void* data, void* hint) {
123+
(void) hint;
124+
125+
NAPI_CALL_RETURN_VOID(env, napi_delete_reference(env, (napi_ref)data));
126+
}
127+
128+
napi_value wrap(napi_env env, napi_callback_info info) {
129+
size_t argc = 2;
130+
napi_value argv[2];
131+
napi_ref payload;
132+
133+
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, argv, NULL, NULL));
134+
NAPI_CALL(env, napi_create_reference(env, argv[1], 1, &payload));
135+
NAPI_CALL(env, napi_wrap(env, argv[0], payload, deref_item, NULL, NULL));
136+
137+
return NULL;
138+
}
139+
122140
void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
123141
napi_property_descriptor descriptors[] = {
124142
DECLARE_NAPI_PROPERTY("testStrictEquals", testStrictEquals),
@@ -130,6 +148,7 @@ void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
130148
DECLARE_NAPI_PROPERTY("createNapiError", createNapiError),
131149
DECLARE_NAPI_PROPERTY("testNapiErrorCleanup", testNapiErro 534A rCleanup),
132150
DECLARE_NAPI_PROPERTY("testNapiTypeof", testNapiTypeof),
151+
DECLARE_NAPI_PROPERTY("wrap", wrap),
133152
};
134153

135154
NAPI_CALL_RETURN_VOID(env, napi_define_properties(

0 commit comments

Comments
 (0)
0