From 2086877e81040afb6dc0c476c731d00c9d2ae940 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 18 Feb 2025 22:23:01 +0100 Subject: [PATCH] src: port `defineLazyProperties` to native code This allows us to have getters not observable from JS side. PR-URL: https://github.com/nodejs/node/pull/57081 Reviewed-By: James M Snell Reviewed-By: Chengzhong Wu Reviewed-By: Joyee Cheung --- .../bootstrap/web/exposed-wildcard.js | 14 +--- lib/internal/util.js | 41 +----------- src/node_messaging.cc | 36 ++++++++++ src/node_util.cc | 66 +++++++++++++++++++ test/common/wpt.js | 27 -------- test/wpt/test-domexception.js | 2 - 6 files changed, 105 insertions(+), 81 deletions(-) diff --git a/lib/internal/bootstrap/web/exposed-wildcard.js b/lib/internal/bootstrap/web/exposed-wildcard.js index d8f451edb01205..4b3d6be0701ec4 100644 --- a/lib/internal/bootstrap/web/exposed-wildcard.js +++ b/lib/internal/bootstrap/web/exposed-wildcard.js @@ -12,12 +12,11 @@ const { const { exposeInterface, - lazyDOMExceptionClass, exposeLazyInterfaces, - exposeGetterAndSetter, exposeNamespace, } = require('internal/util'); const config = internalBinding('config'); +const { exposeLazyDOMExceptionProperty } = internalBinding('messaging'); // https://console.spec.whatwg.org/#console-namespace exposeNamespace(globalThis, 'console', @@ -28,16 +27,7 @@ const { URL, URLSearchParams } = require('internal/url'); exposeInterface(globalThis, 'URL', URL); // https://url.spec.whatwg.org/#urlsearchparams exposeInterface(globalThis, 'URLSearchParams', URLSearchParams); -exposeGetterAndSetter(globalThis, - 'DOMException', - () => { - const DOMException = lazyDOMExceptionClass(); - exposeInterface(globalThis, 'DOMException', DOMException); - return DOMException; - }, - (value) => { - exposeInterface(globalThis, 'DOMException', value); - }); +exposeLazyDOMExceptionProperty(globalThis); // https://dom.spec.whatwg.org/#interface-abortcontroller // Lazy ones. diff --git a/lib/internal/util.js b/lib/internal/util.js index 887ae8874e15fc..e690d7f4323bcc 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -55,6 +55,7 @@ const { const { signals } = internalBinding('constants').os; const { guessHandleType: _guessHandleType, + defineLazyProperties, privateSymbols: { arrow_message_private_symbol, decorated_private_symbol, @@ -610,46 +611,6 @@ function exposeGetterAndSetter(target, name, getter, setter = undefined) { }); } -function defineLazyProperties(target, id, keys, enumerable = true) { - const descriptors = { __proto__: null }; - let mod; - for (let i = 0; i < keys.length; i++) { - const key = keys[i]; - let lazyLoadedValue; - function set(value) { - ObjectDefineProperty(target, key, { - __proto__: null, - writable: true, - value, - }); - } - ObjectDefineProperty(set, 'name', { - __proto__: null, - value: `set ${key}`, - }); - function get() { - mod ??= require(id); - if (lazyLoadedValue === undefined) { - lazyLoadedValue = mod[key]; - set(lazyLoadedValue); - } - return lazyLoadedValue; - } - ObjectDefineProperty(get, 'name', { - __proto__: null, - value: `get ${key}`, - }); - descriptors[key] = { - __proto__: null, - configurable: true, - enumerable, - get, - set, - }; - } - ObjectDefineProperties(target, descriptors); -} - function defineReplaceableLazyAttribute(target, id, keys, writable = true, check) { let mod; for (let i = 0; i < keys.length; i++) { diff --git a/src/node_messaging.cc b/src/node_messaging.cc index bae1387da5d897..9c854df7a3d1f3 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1645,6 +1645,36 @@ static void BroadcastChannel(const FunctionCallbackInfo& args) { } } +static void ExposeLazyDOMExceptionPropertyGetter( + Local name, const v8::PropertyCallbackInfo& info) { + auto context = info.GetIsolate()->GetCurrentContext(); + Local domexception; + if (!GetDOMException(context).ToLocal(&domexception)) { + // V8 will have scheduled an error to be thrown. + return; + } + info.GetReturnValue().Set(domexception); +} +static void ExposeLazyDOMExceptionProperty( + const FunctionCallbackInfo& args) { + CHECK_GE(args.Length(), 1); + CHECK(args[0]->IsObject()); + + Isolate* isolate = args.GetIsolate(); + auto target = args[0].As(); + + if (target + ->SetLazyDataProperty(isolate->GetCurrentContext(), + FIXED_ONE_BYTE_STRING(isolate, "DOMException"), + ExposeLazyDOMExceptionPropertyGetter, + Local(), + v8::DontEnum) + .IsNothing()) { + // V8 will have scheduled an error to be thrown. + return; + } +} + static void CreatePerIsolateProperties(IsolateData* isolate_data, Local target) { Isolate* isolate = isolate_data->isolate(); @@ -1669,6 +1699,10 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data, isolate_data->message_port_constructor_string(), GetMessagePortConstructorTemplate(isolate_data)); + SetMethod(isolate, + target, + "exposeLazyDOMExceptionProperty", + ExposeLazyDOMExceptionProperty); // These are not methods on the MessagePort prototype, because // the browser equivalents do not provide them. SetMethod(isolate, target, "stopMessagePort", MessagePort::Stop); @@ -1714,6 +1748,8 @@ static void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(MessagePort::MoveToContext); registry->Register(SetDeserializerCreateObjectFunction); registry->Register(StructuredClone); + registry->Register(ExposeLazyDOMExceptionProperty); + registry->Register(ExposeLazyDOMExceptionPropertyGetter); } } // anonymous namespace diff --git a/src/node_util.cc b/src/node_util.cc index c855201d8938fd..844cdb0f440577 100644 --- a/src/node_util.cc +++ b/src/node_util.cc @@ -348,6 +348,69 @@ static void IsInsideNodeModules(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(result); } +static void DefineLazyPropertiesGetter( + Local name, const v8::PropertyCallbackInfo& info) { + Realm* realm = Realm::GetCurrent(info); + Isolate* isolate = realm->isolate(); + auto context = isolate->GetCurrentContext(); + Local arg = info.Data(); + Local require_result; + if (!realm->builtin_module_require() + ->Call(context, Null(isolate), 1, &arg) + .ToLocal(&require_result)) { + // V8 will have scheduled an error to be thrown. + return; + } + Local ret; + if (!require_result.As()->Get(context, name).ToLocal(&ret)) { + // V8 will have scheduled an error to be thrown. + return; + } + info.GetReturnValue().Set(ret); +} +static void DefineLazyProperties(const FunctionCallbackInfo& args) { + // target: object, id: string, keys: string[][, enumerable = true] + CHECK_GE(args.Length(), 3); + // target: Object where to define the lazy properties. + CHECK(args[0]->IsObject()); + // id: Internal module to lazy-load where the API to expose are implemented. + CHECK(args[1]->IsString()); + // keys: Keys to map from `require(id)` and `target`. + CHECK(args[2]->IsArray()); + // enumerable: Whether the property should be enumerable. + CHECK(args.Length() == 3 || args[3]->IsBoolean()); + + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + auto context = isolate->GetCurrentContext(); + + auto target = args[0].As(); + Local id = args[1]; + v8::PropertyAttribute attribute = + args.Length() == 3 || args[3]->IsTrue() ? v8::None : v8::DontEnum; + + const Local keys = args[2].As(); + size_t length = keys->Length(); + for (size_t i = 0; i < length; i++) { + Local key; + if (!keys->Get(context, i).ToLocal(&key)) { + // V8 will have scheduled an error to be thrown. + return; + } + CHECK(key->IsString()); + if (target + ->SetLazyDataProperty(context, + key.As(), + DefineLazyPropertiesGetter, + id, + attribute) + .IsNothing()) { + // V8 will have scheduled an error to be thrown. + return; + }; + } +} + void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetPromiseDetails); registry->Register(GetProxyDetails); @@ -364,6 +427,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(fast_guess_handle_type_.GetTypeInfo()); registry->Register(ParseEnv); registry->Register(IsInsideNodeModules); + registry->Register(DefineLazyProperties); + registry->Register(DefineLazyPropertiesGetter); } void Initialize(Local target, @@ -448,6 +513,7 @@ void Initialize(Local target, } SetMethod(context, target, "isInsideNodeModules", IsInsideNodeModules); + SetMethod(context, target, "defineLazyProperties", DefineLazyProperties); SetMethodNoSideEffect( context, target, "getPromiseDetails", GetPromiseDetails); SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails); diff --git a/test/common/wpt.js b/test/common/wpt.js index 3ead9cb23b9c46..74b182e5b84296 100644 --- a/test/common/wpt.js +++ b/test/common/wpt.js @@ -595,7 +595,6 @@ class WPTRunner { switch (name) { case 'Window': { this.globalThisInitScripts.push('globalThis.Window = Object.getPrototypeOf(globalThis).constructor;'); - this.loadLazyGlobals(); break; } @@ -609,32 +608,6 @@ class WPTRunner { } } - loadLazyGlobals() { - const lazyProperties = [ - 'DOMException', - 'Performance', 'PerformanceEntry', 'PerformanceMark', 'PerformanceMeasure', - 'PerformanceObserver', 'PerformanceObserverEntryList', 'PerformanceResourceTiming', - 'Blob', 'atob', 'btoa', - 'MessageChannel', 'MessagePort', 'MessageEvent', - 'EventTarget', 'Event', - 'AbortController', 'AbortSignal', - 'performance', - 'TransformStream', 'TransformStreamDefaultController', - 'WritableStream', 'WritableStreamDefaultController', 'WritableStreamDefaultWriter', - 'ReadableStream', 'ReadableStreamDefaultReader', - 'ReadableStreamBYOBReader', 'ReadableStreamBYOBRequest', - 'ReadableByteStreamController', 'ReadableStreamDefaultController', - 'ByteLengthQueuingStrategy', 'CountQueuingStrategy', - 'TextEncoder', 'TextDecoder', 'TextEncoderStream', 'TextDecoderStream', - 'CompressionStream', 'DecompressionStream', - ]; - if (Boolean(process.versions.openssl) && !process.env.NODE_SKIP_CRYPTO) { - lazyProperties.push('crypto', 'Crypto', 'CryptoKey', 'SubtleCrypto'); - } - const script = lazyProperties.map((name) => `globalThis.${name};`).join('\n'); - this.globalThisInitScripts.push(script); - } - // TODO(joyeecheung): work with the upstream to port more tests in .html // to .js. async runJsTests() { diff --git a/test/wpt/test-domexception.js b/test/wpt/test-domexception.js index 7900d1ca9a79c6..8fa93bc59bdf44 100644 --- a/test/wpt/test-domexception.js +++ b/test/wpt/test-domexception.js @@ -4,6 +4,4 @@ const { WPTRunner } = require('../common/wpt'); const runner = new WPTRunner('webidl/ecmascript-binding/es-exceptions'); -runner.loadLazyGlobals(); - runner.runJsTests();