Skip to content

Commit a81788c

Browse files
authored
node-api: type tag external values without v8::Private
v8::External can not have any properties and private properties. Type tag v8::External with a wrapper struct without setting a private property on the v8::External. PR-URL: #51149 Fixes: nodejs/node-v8#273 Reviewed-By: Vladimir Morozov <vmorozov@microsoft.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Gabriel Schulhof <gabrielschulhof@gmail.com>
1 parent 9ba5df3 commit a81788c

File tree

2 files changed

+88
-13
lines changed

2 files changed

+88
-13
lines changed

src/js_native_api_v8.cc

+82-8
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,58 @@ void Reference::WeakCallback(const v8::WeakCallbackInfo<Reference>& data) {
825825
reference->env_->InvokeFinalizerFromGC(reference);
826826
}
827827

828+
/**
829+
* A wrapper for `v8::External` to support type-tagging. `v8::External` doesn't
830+
* support defining any properties and private properties on it, even though it
831+
* is an object. This wrapper is used to store the type tag and the data of the
832+
* external value.
833+
*/
834+
class ExternalWrapper {
835+
private:
836+
explicit ExternalWrapper(void* data) : data_(data) {}
837+
838+
static void WeakCallback(const v8::WeakCallbackInfo<ExternalWrapper>& data) {
839+
ExternalWrapper* wrapper = data.GetParameter();
840+
delete wrapper;
841+
}
842+
843+
public:
844+
static v8::Local<v8::External> New(napi_env env, void* data) {
845+
ExternalWrapper* wrapper = new ExternalWrapper(data);
846+
v8::Local<v8::External> external = v8::External::New(env->isolate, wrapper);
847+
wrapper->persistent_.Reset(env->isolate, external);
848+
wrapper->persistent_.SetWeak(
849+
wrapper, WeakCallback, v8::WeakCallbackType::kParameter);
850+
851+
return external;
852+
}
853+
854+
static ExternalWrapper* From(v8::Local<v8::External> external) {
855+
return static_cast<ExternalWrapper*>(external->Value());
856+
}
857+
858+
void* Data() { return data_; }
859+
860+
bool TypeTag(const napi_type_tag* type_tag) {
861+
if (type_tag_ != nullptr) {
862+
return false;
863+
}
864+
type_tag_ = type_tag;
865+
return true;
866+
}
867+
868+
bool CheckTypeTag(const napi_type_tag* type_tag) {
869+
return type_tag == type_tag_ ||
870+
(type_tag_ && type_tag->lower == type_tag_->lower &&
871+
type_tag->upper == type_tag_->upper);
872+
}
873+
874+
private:
875+
v8impl::Persistent<v8::Value> persistent_;
876+
void* data_;
877+
const napi_type_tag* type_tag_ = nullptr;
878+
};
879+
828880
} // end of namespace v8impl
829881

830882
// Warning: Keep in-sync with napi_status enum
@@ -2525,9 +2577,8 @@ napi_create_external(napi_env env,
25252577
NAPI_PREAMBLE(env);
25262578
CHECK_ARG(env, result);
25272579

2528-
v8::Isolate* isolate = env->isolate;
2529-
2530-
v8::Local<v8::Value> external_value = v8::External::New(isolate, data);
2580+
v8::Local<v8::External> external_value =
2581+
v8impl::ExternalWrapper::New(env, data);
25312582

25322583
if (finalize_cb) {
25332584
// The Reference object will delete itself after invoking the finalizer
@@ -2547,12 +2598,24 @@ napi_create_external(napi_env env,
25472598
}
25482599

25492600
napi_status NAPI_CDECL napi_type_tag_object(napi_env env,
2550-
napi_value object,
2601+
napi_value object_or_external,
25512602
const napi_type_tag* type_tag) {
25522603
NAPI_PREAMBLE(env);
25532604
v8::Local<v8::Context> context = env->context();
2605+
2606+
CHECK_ARG(env, object_or_external);
2607+
v8::Local<v8::Value> val =
2608+
v8impl::V8LocalValueFromJsValue(object_or_external);
2609+
if (val->IsExternal()) {
2610+
v8impl::ExternalWrapper* wrapper =
2611+
v8impl::ExternalWrapper::From(val.As<v8::External>());
2612+
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
2613+
env, wrapper->TypeTag(type_tag), napi_invalid_arg);
2614+
return GET_RETURN_STATUS(env);
2615+
}
2616+
25542617
v8::Local<v8::Object> obj;
2555-
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
2618+
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
25562619
CHECK_ARG_WITH_PREAMBLE(env, type_tag);
25572620

25582621
auto key = NAPI_PRIVATE_KEY(context, type_tag);
@@ -2574,13 +2637,24 @@ napi_status NAPI_CDECL napi_type_tag_object(napi_env env,
25742637
}
25752638

25762639
napi_status NAPI_CDECL napi_check_object_type_tag(napi_env env,
2577-
napi_value object,
2640+
napi_value object_or_external,
25782641
const napi_type_tag* type_tag,
25792642
bool* result) {
25802643
NAPI_PREAMBLE(env);
25812644
v8::Local<v8::Context> context = env->context();
2645+
2646+
CHECK_ARG(env, object_or_external);
2647+
v8::Local<v8::Value> obj_val =
2648+
v8impl::V8LocalValueFromJsValue(object_or_external);
2649+
if (obj_val->IsExternal()) {
2650+
v8impl::ExternalWrapper* wrapper =
2651+
v8impl::ExternalWrapper::From(obj_val.As<v8::External>());
2652+
*result = wrapper->CheckTypeTag(type_tag);
2653+
return GET_RETURN_STATUS(env);
2654+
}
2655+
25822656
v8::Local<v8::Object> obj;
2583-
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
2657+
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
25842658
CHECK_ARG_WITH_PREAMBLE(env, type_tag);
25852659
CHECK_ARG_WITH_PREAMBLE(env, result);
25862660

@@ -2625,7 +2699,7 @@ napi_status NAPI_CDECL napi_get_value_external(napi_env env,
26252699
RETURN_STATUS_IF_FALSE(env, val->IsExternal(), napi_invalid_arg);
26262700

26272701
v8::Local<v8::External> external_value = val.As<v8::External>();
2628-
*result = external_value->Value();
2702+
*result = v8impl::ExternalWrapper::From(external_value)->Data();
26292703

26302704
return napi_clear_last_error(env);
26312705
}

test/cctest/test_linked_binding.cc

+6-5
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ napi_value NapiLinkedWithInstanceData(napi_env env, napi_value exports) {
250250
napi_value key, value;
251251
CHECK_EQ(napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key),
252252
napi_ok);
253-
CHECK_EQ(napi_create_external(env, instance_data, nullptr, nullptr, &value),
253+
CHECK_EQ(napi_create_external_arraybuffer(
254+
env, instance_data, 1, nullptr, nullptr, &value),
254255
napi_ok);
255256
CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok);
256257
return nullptr;
@@ -289,9 +290,9 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) {
289290
.ToLocalChecked();
290291
v8::Local<v8::Value> completion_value =
291292
script->Run(context).ToLocalChecked();
292-
CHECK(completion_value->IsExternal());
293+
CHECK(completion_value->IsArrayBuffer());
293294
instance_data =
294-
static_cast<int*>(completion_value.As<v8::External>()->Value());
295+
static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
295296
CHECK_NE(instance_data, nullptr);
296297
CHECK_EQ(*instance_data, 0);
297298
}
@@ -327,9 +328,9 @@ TEST_F(LinkedBindingTest,
327328
.ToLocalChecked();
328329
v8::Local<v8::Value> completion_value =
329330
script->Run(context).ToLocalChecked();
330-
CHECK(completion_value->IsExternal());
331+
CHECK(completion_value->IsArrayBuffer());
331332
instance_data =
332-
static_cast<int*>(completion_value.As<v8::External>()->Value());
333+
static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
333334
CHECK_NE(instance_data, nullptr);
334335
CHECK_EQ(*instance_data, 0);
335336
}

0 commit comments

Comments
 (0)