diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 895f7b9d096166..b5c906e7bd9066 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -290,18 +290,6 @@ BaseObjectPtr ContextifyContext::New( Local wrapper; { Context::Scope context_scope(v8_context); - Local ctor_name = sandbox_obj->GetConstructorName(); - if (!ctor_name->Equals(v8_context, env->object_string()).FromMaybe(false) && - new_context_global - ->DefineOwnProperty( - v8_context, - v8::Symbol::GetToStringTag(env->isolate()), - ctor_name, - static_cast(v8::DontEnum)) - .IsNothing()) { - return BaseObjectPtr(); - } - // Assign host_defined_options_id to the global object so that in the // callback of ImportModuleDynamically, we can get the // host_defined_options_id from the v8::Context without accessing the @@ -742,19 +730,25 @@ Intercepted ContextifyContext::PropertyDeleterCallback( // static void ContextifyContext::PropertyEnumeratorCallback( const PropertyCallbackInfo& args) { + // Named enumerator will be invoked on Object.keys, + // Object.getOwnPropertyNames, Object.getOwnPropertySymbols, + // Object.getOwnPropertyDescriptors, for...in, etc. operations. + // Named enumerator should return all own non-indices property names, + // including string properties and symbol properties. V8 will filter the + // result array to match the expected symbol-only, enumerable-only with + // NamedPropertyQueryCallback. ContextifyContext* ctx = ContextifyContext::Get(args); // Still initializing if (IsStillInitializing(ctx)) return; Local properties; - // Only get named properties, exclude symbols and indices. + // Only get own named properties, exclude indices. if (!ctx->sandbox() ->GetPropertyNames( ctx->context(), - KeyCollectionMode::kIncludePrototypes, - static_cast(PropertyFilter::ONLY_ENUMERABLE | - PropertyFilter::SKIP_SYMBOLS), + KeyCollectionMode::kOwnOnly, + static_cast(PropertyFilter::ALL_PROPERTIES), IndexFilter::kSkipIndices) .ToLocal(&properties)) return; @@ -765,6 +759,12 @@ void ContextifyContext::PropertyEnumeratorCallback( // static void ContextifyContext::IndexedPropertyEnumeratorCallback( const PropertyCallbackInfo& args) { + // Indexed enumerator will be invoked on Object.keys, + // Object.getOwnPropertyNames, Object.getOwnPropertyDescriptors, for...in, + // etc. operations. Indexed enumerator should return all own non-indices index + // properties. V8 will filter the result array to match the expected + // enumerable-only with IndexedPropertyQueryCallback. + Isolate* isolate = args.GetIsolate(); HandleScope scope(isolate); ContextifyContext* ctx = ContextifyContext::Get(args); @@ -775,9 +775,15 @@ void ContextifyContext::IndexedPropertyEnumeratorCallback( Local properties; - // By default, GetPropertyNames returns string and number property names, and - // doesn't convert the numbers to strings. - if (!ctx->sandbox()->GetPropertyNames(context).ToLocal(&properties)) return; + // Only get own index properties. + if (!ctx->sandbox() + ->GetPropertyNames( + context, + KeyCollectionMode::kOwnOnly, + static_cast(PropertyFilter::SKIP_SYMBOLS), + IndexFilter::kIncludeIndices) + .ToLocal(&properties)) + return; std::vector> properties_vec; if (FromV8Array(context, properties, &properties_vec).IsNothing()) { diff --git a/test/parallel/test-vm-global-property-enumerator.js b/test/parallel/test-vm-global-property-enumerator.js index 7b37c2af41094d..10d7b6720b49eb 100644 --- a/test/parallel/test-vm-global-property-enumerator.js +++ b/test/parallel/test-vm-global-property-enumerator.js @@ -1,5 +1,6 @@ 'use strict'; require('../common'); +const globalNames = require('../common/globals'); const vm = require('vm'); const assert = require('assert'); @@ -39,11 +40,57 @@ const cases = [ key: 'value', }, }, + (() => { + const obj = { + __proto__: { + [Symbol.toStringTag]: 'proto', + }, + }; + Object.defineProperty(obj, '1', { + value: 'value', + enumerable: false, + configurable: true, + }); + Object.defineProperty(obj, 'key', { + value: 'value', + enumerable: false, + configurable: true, + }); + Object.defineProperty(obj, Symbol('symbol'), { + value: 'value', + enumerable: false, + configurable: true, + }); + Object.defineProperty(obj, Symbol('symbol-enumerable'), { + value: 'value', + enumerable: true, + configurable: true, + }); + return obj; + })(), ]; for (const [idx, obj] of cases.entries()) { const ctx = vm.createContext(obj); const globalObj = vm.runInContext('this', ctx); - const keys = Object.keys(globalObj); - assert.deepStrictEqual(keys, Object.keys(obj), `Case ${idx} failed`); + assert.deepStrictEqual(Object.keys(globalObj), Object.keys(obj), `Case ${idx} failed: Object.keys`); + + const ownPropertyNamesInner = difference(Object.getOwnPropertyNames(globalObj), globalNames.intrinsics); + const ownPropertyNamesOuter = Object.getOwnPropertyNames(obj); + assert.deepStrictEqual( + ownPropertyNamesInner, + ownPropertyNamesOuter, + `Case ${idx} failed: Object.getOwnPropertyNames` + ); + + assert.deepStrictEqual( + Object.getOwnPropertySymbols(globalObj), + Object.getOwnPropertySymbols(obj), + `Case ${idx} failed: Object.getOwnPropertySymbols` + ); } + +function difference(arrA, arrB) { + const setB = new Set(arrB); + return arrA.filter((x) => !setB.has(x)); +};