From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: VerteDinde Date: Mon, 24 Jun 2024 21:48:40 -0700 Subject: fix: add property query interceptors This commit cherry-picks an upstream interceptor API change from node-v8/canary to accomodate V8's upstream changes to old interceptor APIs. Node PR: https://github.com/nodejs/node-v8/commit/d1f18b0bf16efbc1e54ba04a54735ce4683cb936 CL: https://chromium-review.googlesource.com/c/v8/v8/+/5630388 This patch can be removed when the node change is incorporated into main. diff --git a/src/node_contextify.cc b/src/node_contextify.cc index f6969e635f692b17b4efca1dfbee086a0199db6e..6ace71b04e9da761c77838dde2abd8088abcb6c4 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -49,6 +49,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; @@ -457,14 +458,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(); @@ -488,18 +490,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; @@ -517,8 +523,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 @@ -537,11 +544,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 && @@ -555,19 +567,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(); @@ -577,19 +593,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(); @@ -608,7 +628,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(); @@ -631,6 +651,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(); @@ -642,26 +665,32 @@ 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; } } // 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 @@ -681,76 +710,84 @@ 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 d42b5e0c544e726fc3f6d8392a554df9aa480fe9..ea2d513463057715127cb4b4f2d66b4cfcf71351 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -107,42 +107,39 @@ class ContextifyContext : public BaseObject { const v8::FunctionCallbackInfo& args); 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 bce7ae07214ddf970a530db29ed6970e14b7a5ed..85f82180d48d6cfd7738cd7b1e504f23b38153e8 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 9238f2d4d7376b22e264dbc9359b480937d29676..b5c1df6941616642075babdad81be00ce63ffd56 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -59,16 +59,17 @@ class ExternalReferenceRegistry { V(v8::FunctionCallback) \ 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) \