Skip to content

Commit

Permalink
node-api: type tag external values without v8::Private
Browse files Browse the repository at this point in the history 
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: nodejs#51149
Fixes: nodejs/node-v8#273
Reviewed-By: Vladimir Morozov <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
  • Loading branch information
@legendecas
legendecas authored Dec 22, 2023
1 parent 9ba5df3 commit a81788c
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 13 deletions.
90 changes: 82 additions & 8 deletions src/js_native_api_v8.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,58 @@ void Reference::WeakCallback(const v8::WeakCallbackInfo<Reference>& data) {
reference->env_->InvokeFinalizerFromGC(reference);
}

/**
* A wrapper for `v8::External` to support type-tagging. `v8::External` doesn't
* support defining any properties and private properties on it, even though it
* is an object. This wrapper is used to store the type tag and the data of the
* external value.
*/
class ExternalWrapper {
private:
explicit ExternalWrapper(void* data) : data_(data) {}

static void WeakCallback(const v8::WeakCallbackInfo<ExternalWrapper>& data) {
ExternalWrapper* wrapper = data.GetParameter();
delete wrapper;
}

public:
static v8::Local<v8::External> New(napi_env env, void* data) {
ExternalWrapper* wrapper = new ExternalWrapper(data);
v8::Local<v8::External> external = v8::External::New(env->isolate, wrapper);
wrapper->persistent_.Reset(env->isolate, external);
wrapper->persistent_.SetWeak(
wrapper, WeakCallback, v8::WeakCallbackType::kParameter);

return external;
}

static ExternalWrapper* From(v8::Local<v8::External> external) {
return static_cast<ExternalWrapper*>(external->Value());
}

void* Data() { return data_; }

bool TypeTag(const napi_type_tag* type_tag) {
if (type_tag_ != nullptr) {
return false;
}
type_tag_ = type_tag;
return true;
}

bool CheckTypeTag(const napi_type_tag* type_tag) {
return type_tag == type_tag_ ||
(type_tag_ && type_tag->lower == type_tag_->lower &&
type_tag->upper == type_tag_->upper);
}

private:
v8impl::Persistent<v8::Value> persistent_;
void* data_;
const napi_type_tag* type_tag_ = nullptr;
};

} // end of namespace v8impl

// Warning: Keep in-sync with napi_status enum
Expand Down Expand Up @@ -2525,9 +2577,8 @@ napi_create_external(napi_env env,
NAPI_PREAMBLE(env);
CHECK_ARG(env, result);

v8::Isolate* isolate = env->isolate;

v8::Local<v8::Value> external_value = v8::External::New(isolate, data);
v8::Local<v8::External> external_value =
v8impl::ExternalWrapper::New(env, data);

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

napi_status NAPI_CDECL napi_type_tag_object(napi_env env,
napi_value object,
napi_value object_or_external,
const napi_type_tag* type_tag) {
NAPI_PREAMBLE(env);
v8::Local<v8::Context> context = env->context();

CHECK_ARG(env, object_or_external);
v8::Local<v8::Value> val =
v8impl::V8LocalValueFromJsValue(object_or_external);
if (val->IsExternal()) {
v8impl::ExternalWrapper* wrapper =
v8impl::ExternalWrapper::From(val.As<v8::External>());
RETURN_STATUS_IF_FALSE_WITH_PREAMBLE(
env, wrapper->TypeTag(type_tag), napi_invalid_arg);
return GET_RETURN_STATUS(env);
}

v8::Local<v8::Object> obj;
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
CHECK_ARG_WITH_PREAMBLE(env, type_tag);

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

napi_status NAPI_CDECL napi_check_object_type_tag(napi_env env,
napi_value object,
napi_value object_or_external,
const napi_type_tag* type_tag,
bool* result) {
NAPI_PREAMBLE(env);
v8::Local<v8::Context> context = env->context();

CHECK_ARG(env, object_or_external);
v8::Local<v8::Value> obj_val =
v8impl::V8LocalValueFromJsValue(object_or_external);
if (obj_val->IsExternal()) {
v8impl::ExternalWrapper* wrapper =
v8impl::ExternalWrapper::From(obj_val.As<v8::External>());
*result = wrapper->CheckTypeTag(type_tag);
return GET_RETURN_STATUS(env);
}

v8::Local<v8::Object> obj;
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object);
CHECK_TO_OBJECT_WITH_PREAMBLE(env, context, obj, object_or_external);
CHECK_ARG_WITH_PREAMBLE(env, type_tag);
CHECK_ARG_WITH_PREAMBLE(env, result);

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

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

return napi_clear_last_error(env);
}
Expand Down
11 changes: 6 additions & 5 deletions test/cctest/test_linked_binding.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ napi_value NapiLinkedWithInstanceData(napi_env env, napi_value exports) {
napi_value key, value;
CHECK_EQ(napi_create_string_utf8(env, "hello", NAPI_AUTO_LENGTH, &key),
napi_ok);
CHECK_EQ(napi_create_external(env, instance_data, nullptr, nullptr, &value),
CHECK_EQ(napi_create_external_arraybuffer(
env, instance_data, 1, nullptr, nullptr, &value),
napi_ok);
CHECK_EQ(napi_set_property(env, exports, key, value), napi_ok);
return nullptr;
Expand Down Expand Up @@ -289,9 +290,9 @@ TEST_F(LinkedBindingTest, LocallyDefinedLinkedBindingNapiInstanceDataTest) {
.ToLocalChecked();
v8::Local<v8::Value> completion_value =
script->Run(context).ToLocalChecked();
CHECK(completion_value->IsExternal());
CHECK(completion_value->IsArrayBuffer());
instance_data =
static_cast<int*>(completion_value.As<v8::External>()->Value());
static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
CHECK_NE(instance_data, nullptr);
CHECK_EQ(*instance_data, 0);
}
Expand Down Expand Up @@ -327,9 +328,9 @@ TEST_F(LinkedBindingTest,
.ToLocalChecked();
v8::Local<v8::Value> completion_value =
script->Run(context).ToLocalChecked();
CHECK(completion_value->IsExternal());
CHECK(completion_value->IsArrayBuffer());
instance_data =
static_cast<int*>(completion_value.As<v8::External>()->Value());
static_cast<int*>(completion_value.As<v8::ArrayBuffer>()->Data());
CHECK_NE(instance_data, nullptr);
CHECK_EQ(*instance_data, 0);
}
Expand Down

0 comments on commit a81788c

Please sign in to comment.