From d6b12f5b77e35c58a611d614cf0aac674ec2c3ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Wed, 1 May 2024 18:50:02 +0200 Subject: [PATCH] src: migrate to new V8 interceptors API Refs: https://github.com/v8/node/pull/180 PR-URL: https://github.com/nodejs/node/pull/52745 Reviewed-By: Yagiz Nizipli Reviewed-By: Stephen Belanger --- src/node_contextify.cc | 145 +++++++++++++++++++++------------- src/node_contextify.h | 37 ++++----- src/node_env_var.cc | 52 +++++++----- src/node_external_reference.h | 21 ++--- 4 files changed, 152 insertions(+), 103 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index a4c2eb354eb498..26aa4a206da37c 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -51,6 +51,7 @@ using v8::FunctionTemplate; using v8::HandleScope; using v8::IndexedPropertyHandlerConfiguration; using v8::Int32; +using v8::Intercepted; using v8::Isolate; using v8::Just; using v8::Local; @@ -458,14 +459,15 @@ bool ContextifyContext::IsStillInitializing(const ContextifyContext* ctx) { } // static -void ContextifyContext::PropertyGetterCallback( - Local property, - const PropertyCallbackInfo& args) { +Intercepted ContextifyContext::PropertyGetterCallback( + Local property, const PropertyCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } Local context = ctx->context(); Local sandbox = ctx->sandbox(); @@ -487,18 +489,22 @@ void ContextifyContext::PropertyGetterCallback( rv = ctx->global_proxy(); args.GetReturnValue().Set(rv); + return Intercepted::kYes; } + return Intercepted::kNo; } // static -void ContextifyContext::PropertySetterCallback( +Intercepted ContextifyContext::PropertySetterCallback( Local property, Local value, - const PropertyCallbackInfo& args) { + const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } Local context = ctx->context(); PropertyAttribute attributes = PropertyAttribute::None; @@ -516,8 +522,9 @@ void ContextifyContext::PropertySetterCallback( (static_cast(attributes) & static_cast(PropertyAttribute::ReadOnly)); - if (read_only) - return; + if (read_only) { + return Intercepted::kNo; + } // true for x = 5 // false for this.x = 5 @@ -536,11 +543,16 @@ void ContextifyContext::PropertySetterCallback( bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox; if (!is_declared && args.ShouldThrowOnError() && is_contextual_store && - !is_function) - return; + !is_function) { + return Intercepted::kNo; + } - if (!is_declared && property->IsSymbol()) return; - if (ctx->sandbox()->Set(context, property, value).IsNothing()) return; + if (!is_declared && property->IsSymbol()) { + return Intercepted::kNo; + } + if (ctx->sandbox()->Set(context, property, value).IsNothing()) { + return Intercepted::kNo; + } Local desc; if (is_declared_on_sandbox && @@ -554,19 +566,23 @@ void ContextifyContext::PropertySetterCallback( // We have to specify the return value for any contextual or get/set // property if (desc_obj->HasOwnProperty(context, env->get_string()).FromMaybe(false) || - desc_obj->HasOwnProperty(context, env->set_string()).FromMaybe(false)) + desc_obj->HasOwnProperty(context, env->set_string()).FromMaybe(false)) { args.GetReturnValue().Set(value); + return Intercepted::kYes; + } } + return Intercepted::kNo; } // static -void ContextifyContext::PropertyDescriptorCallback( - Local property, - const PropertyCallbackInfo& args) { +Intercepted ContextifyContext::PropertyDescriptorCallback( + Local property, const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } Local context = ctx->context(); @@ -576,19 +592,23 @@ void ContextifyContext::PropertyDescriptorCallback( Local desc; if (sandbox->GetOwnPropertyDescriptor(context, property).ToLocal(&desc)) { args.GetReturnValue().Set(desc); + return Intercepted::kYes; } } + return Intercepted::kNo; } // static -void ContextifyContext::PropertyDefinerCallback( +Intercepted ContextifyContext::PropertyDefinerCallback( Local property, const PropertyDescriptor& desc, - const PropertyCallbackInfo& args) { + const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } Local context = ctx->context(); Isolate* isolate = context->GetIsolate(); @@ -607,7 +627,7 @@ void ContextifyContext::PropertyDefinerCallback( // If the property is set on the global as neither writable nor // configurable, don't change it on the global or sandbox. if (is_declared && read_only && dont_delete) { - return; + return Intercepted::kNo; } Local sandbox = ctx->sandbox(); @@ -630,6 +650,9 @@ void ContextifyContext::PropertyDefinerCallback( desc.has_set() ? desc.set() : Undefined(isolate).As()); define_prop_on_sandbox(&desc_for_sandbox); + // TODO(https://github.com/nodejs/node/issues/52634): this should return + // kYes to behave according to the expected semantics. + return Intercepted::kNo; } else { Local value = desc.has_value() ? desc.value() : Undefined(isolate).As(); @@ -641,26 +664,33 @@ void ContextifyContext::PropertyDefinerCallback( PropertyDescriptor desc_for_sandbox(value); define_prop_on_sandbox(&desc_for_sandbox); } + // TODO(https://github.com/nodejs/node/issues/52634): this should return + // kYes to behave according to the expected semantics. + return Intercepted::kNo; } + return Intercepted::kNo; } // static -void ContextifyContext::PropertyDeleterCallback( - Local property, - const PropertyCallbackInfo& args) { +Intercepted ContextifyContext::PropertyDeleterCallback( + Local property, const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } Maybe success = ctx->sandbox()->Delete(ctx->context(), property); - if (success.FromMaybe(false)) - return; + if (success.FromMaybe(false)) { + return Intercepted::kNo; + } // Delete failed on the sandbox, intercept and do not delete on // the global object. args.GetReturnValue().Set(false); + return Intercepted::kYes; } // static @@ -680,76 +710,83 @@ void ContextifyContext::PropertyEnumeratorCallback( } // static -void ContextifyContext::IndexedPropertyGetterCallback( - uint32_t index, - const PropertyCallbackInfo& args) { +Intercepted ContextifyContext::IndexedPropertyGetterCallback( + uint32_t index, const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } - ContextifyContext::PropertyGetterCallback( + return ContextifyContext::PropertyGetterCallback( Uint32ToName(ctx->context(), index), args); } - -void ContextifyContext::IndexedPropertySetterCallback( +Intercepted ContextifyContext::IndexedPropertySetterCallback( uint32_t index, Local value, - const PropertyCallbackInfo& args) { + const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } - ContextifyContext::PropertySetterCallback( + return ContextifyContext::PropertySetterCallback( Uint32ToName(ctx->context(), index), value, args); } // static -void ContextifyContext::IndexedPropertyDescriptorCallback( - uint32_t index, - const PropertyCallbackInfo& args) { +Intercepted ContextifyContext::IndexedPropertyDescriptorCallback( + uint32_t index, const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } - ContextifyContext::PropertyDescriptorCallback( + return ContextifyContext::PropertyDescriptorCallback( Uint32ToName(ctx->context(), index), args); } - -void ContextifyContext::IndexedPropertyDefinerCallback( +Intercepted ContextifyContext::IndexedPropertyDefinerCallback( uint32_t index, const PropertyDescriptor& desc, - const PropertyCallbackInfo& args) { + const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } - ContextifyContext::PropertyDefinerCallback( + return ContextifyContext::PropertyDefinerCallback( Uint32ToName(ctx->context(), index), desc, args); } // static -void ContextifyContext::IndexedPropertyDeleterCallback( - uint32_t index, - const PropertyCallbackInfo& args) { +Intercepted ContextifyContext::IndexedPropertyDeleterCallback( + uint32_t index, const PropertyCallbackInfo& args) { ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing - if (IsStillInitializing(ctx)) return; + if (IsStillInitializing(ctx)) { + return Intercepted::kNo; + } Maybe success = ctx->sandbox()->Delete(ctx->context(), index); - if (success.FromMaybe(false)) - return; + if (success.FromMaybe(false)) { + return Intercepted::kNo; + } // Delete failed on the sandbox, intercept and do not delete on // the global object. args.GetReturnValue().Set(false); + return Intercepted::kYes; } void ContextifyScript::CreatePerIsolateProperties( diff --git a/src/node_contextify.h b/src/node_contextify.h index b5cc4e546f4ab7..afefe54cbbe4b9 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -96,42 +96,39 @@ class ContextifyContext : public BaseObject { const errors::TryCatchScope& try_catch); static void WeakCallback( const v8::WeakCallbackInfo& data); - static void PropertyGetterCallback( + static v8::Intercepted PropertyGetterCallback( v8::Local property, const v8::PropertyCallbackInfo& args); - static void PropertySetterCallback( + static v8::Intercepted PropertySetterCallback( v8::Local property, v8::Local value, - const v8::PropertyCallbackInfo& args); - static void PropertyDescriptorCallback( + const v8::PropertyCallbackInfo& args); + static v8::Intercepted PropertyDescriptorCallback( v8::Local property, const v8::PropertyCallbackInfo& args); - static void PropertyDefinerCallback( + static v8::Intercepted PropertyDefinerCallback( v8::Local property, const v8::PropertyDescriptor& desc, - const v8::PropertyCallbackInfo& args); - static void PropertyDeleterCallback( + const v8::PropertyCallbackInfo& args); + static v8::Intercepted PropertyDeleterCallback( v8::Local property, const v8::PropertyCallbackInfo& args); static void PropertyEnumeratorCallback( const v8::PropertyCallbackInfo& args); - static void IndexedPropertyGetterCallback( - uint32_t index, - const v8::PropertyCallbackInfo& args); - static void IndexedPropertySetterCallback( + static v8::Intercepted IndexedPropertyGetterCallback( + uint32_t index, const v8::PropertyCallbackInfo& args); + static v8::Intercepted IndexedPropertySetterCallback( uint32_t index, v8::Local value, - const v8::PropertyCallbackInfo& args); - static void IndexedPropertyDescriptorCallback( - uint32_t index, - const v8::PropertyCallbackInfo& args); - static void IndexedPropertyDefinerCallback( + const v8::PropertyCallbackInfo& args); + static v8::Intercepted IndexedPropertyDescriptorCallback( + uint32_t index, const v8::PropertyCallbackInfo& args); + static v8::Intercepted IndexedPropertyDefinerCallback( uint32_t index, const v8::PropertyDescriptor& desc, - const v8::PropertyCallbackInfo& args); - static void IndexedPropertyDeleterCallback( - uint32_t index, - const v8::PropertyCallbackInfo& args); + const v8::PropertyCallbackInfo& args); + static v8::Intercepted IndexedPropertyDeleterCallback( + uint32_t index, const v8::PropertyCallbackInfo& args); v8::Global context_; std::unique_ptr microtask_queue_; diff --git a/src/node_env_var.cc b/src/node_env_var.cc index bce7ae07214ddf..85f82180d48d6c 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -16,6 +16,7 @@ using v8::DontEnum; using v8::FunctionTemplate; using v8::HandleScope; using v8::Integer; +using v8::Intercepted; using v8::Isolate; using v8::Just; using v8::Local; @@ -336,24 +337,27 @@ Maybe KVStore::AssignToObject(v8::Isolate* isolate, return Just(true); } -static void EnvGetter(Local property, - const PropertyCallbackInfo& info) { +static Intercepted EnvGetter(Local property, + const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); if (property->IsSymbol()) { - return info.GetReturnValue().SetUndefined(); + info.GetReturnValue().SetUndefined(); + return Intercepted::kYes; } CHECK(property->IsString()); MaybeLocal value_string = env->env_vars()->Get(env->isolate(), property.As()); if (!value_string.IsEmpty()) { info.GetReturnValue().Set(value_string.ToLocalChecked()); + return Intercepted::kYes; } + return Intercepted::kNo; } -static void EnvSetter(Local property, - Local value, - const PropertyCallbackInfo& info) { +static Intercepted EnvSetter(Local property, + Local value, + const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); // calling env->EmitProcessEnvWarning() sets a variable indicating that @@ -369,35 +373,40 @@ static void EnvSetter(Local property, "the " "value to a string before setting process.env with it.", "DEP0104") - .IsNothing()) - return; + .IsNothing()) { + return Intercepted::kNo; + } } Local key; Local value_string; if (!property->ToString(env->context()).ToLocal(&key) || !value->ToString(env->context()).ToLocal(&value_string)) { - return; + return Intercepted::kNo; } env->env_vars()->Set(env->isolate(), key, value_string); - // Whether it worked or not, always return value. - info.GetReturnValue().Set(value); + return Intercepted::kYes; } -static void EnvQuery(Local property, - const PropertyCallbackInfo& info) { +static Intercepted EnvQuery(Local property, + const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { int32_t rc = env->env_vars()->Query(env->isolate(), property.As()); - if (rc != -1) info.GetReturnValue().Set(rc); + if (rc != -1) { + // Return attributes for the property. + info.GetReturnValue().Set(v8::None); + return Intercepted::kYes; + } } + return Intercepted::kNo; } -static void EnvDeleter(Local property, - const PropertyCallbackInfo& info) { +static Intercepted EnvDeleter(Local property, + const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); CHECK(env->has_run_bootstrapping_code()); if (property->IsString()) { @@ -407,6 +416,7 @@ static void EnvDeleter(Local property, // process.env never has non-configurable properties, so always // return true like the tc39 delete operator. info.GetReturnValue().Set(true); + return Intercepted::kYes; } static void EnvEnumerator(const PropertyCallbackInfo& info) { @@ -417,9 +427,9 @@ static void EnvEnumerator(const PropertyCallbackInfo& info) { env->env_vars()->Enumerate(env->isolate())); } -static void EnvDefiner(Local property, - const PropertyDescriptor& desc, - const PropertyCallbackInfo& info) { +static Intercepted EnvDefiner(Local property, + const PropertyDescriptor& desc, + const PropertyCallbackInfo& info) { Environment* env = Environment::GetCurrent(info); if (desc.has_value()) { if (!desc.has_writable() || @@ -430,6 +440,7 @@ static void EnvDefiner(Local property, "configurable, writable," " and enumerable " "data descriptor"); + return Intercepted::kYes; } else if (!desc.configurable() || !desc.enumerable() || !desc.writable()) { @@ -438,6 +449,7 @@ static void EnvDefiner(Local property, "configurable, writable," " and enumerable " "data descriptor"); + return Intercepted::kYes; } else { return EnvSetter(property, desc.value(), info); } @@ -447,12 +459,14 @@ static void EnvDefiner(Local property, "'process.env' does not accept an" " accessor(getter/setter)" " descriptor"); + return Intercepted::kYes; } else { THROW_ERR_INVALID_OBJECT_DEFINE_PROPERTY(env, "'process.env' only accepts a " "configurable, writable," " and enumerable " "data descriptor"); + return Intercepted::kYes; } } diff --git a/src/node_external_reference.h b/src/node_external_reference.h index eec8a0e3f07ae9..599db7352d2851 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -67,16 +67,17 @@ class ExternalReferenceRegistry { V(v8::AccessorSetterCallback) \ V(v8::AccessorNameGetterCallback) \ V(v8::AccessorNameSetterCallback) \ - V(v8::GenericNamedPropertyDefinerCallback) \ - V(v8::GenericNamedPropertyDeleterCallback) \ - V(v8::GenericNamedPropertyEnumeratorCallback) \ - V(v8::GenericNamedPropertyQueryCallback) \ - V(v8::GenericNamedPropertySetterCallback) \ - V(v8::IndexedPropertySetterCallback) \ - V(v8::IndexedPropertyDefinerCallback) \ - V(v8::IndexedPropertyDeleterCallback) \ - V(v8::IndexedPropertyQueryCallback) \ - V(v8::IndexedPropertyDescriptorCallback) \ + V(v8::NamedPropertyGetterCallback) \ + V(v8::NamedPropertyDefinerCallback) \ + V(v8::NamedPropertyDeleterCallback) \ + V(v8::NamedPropertyEnumeratorCallback) \ + V(v8::NamedPropertyQueryCallback) \ + V(v8::NamedPropertySetterCallback) \ + V(v8::IndexedPropertyGetterCallbackV2) \ + V(v8::IndexedPropertySetterCallbackV2) \ + V(v8::IndexedPropertyDefinerCallbackV2) \ + V(v8::IndexedPropertyDeleterCallbackV2) \ + V(v8::IndexedPropertyQueryCallbackV2) \ V(const v8::String::ExternalStringResourceBase*) #define V(ExternalReferenceType) \