From 850d3578b6917049c1d956db42bee22a95453021 Mon Sep 17 00:00:00 2001 From: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> Date: Sun, 10 Jan 2021 13:10:00 +0100 Subject: [PATCH] =?UTF-8?q?lib:=20refactor=C2=A0`primordials.makeSafe`=20t?= =?UTF-8?q?o=20use=20more=20primordials?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/36865 Reviewed-By: Antoine du Hamel --- lib/internal/per_context/primordials.js | 235 +++++++++++++----------- 1 file changed, 129 insertions(+), 106 deletions(-) diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 2ab9550213ecca..ec1316d3245a41 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -6,6 +6,12 @@ // so that Node.js's builtin modules do not need to later look these up from // the global proxy, which can be mutated by users. +const { + defineProperty: ReflectDefineProperty, + getOwnPropertyDescriptor: ReflectGetOwnPropertyDescriptor, + ownKeys: ReflectOwnKeys, +} = Reflect; + // TODO(joyeecheung): we can restrict access to these globals in builtin // modules through the JS linter, for example: ban access such as `Object` // (which falls back to a lookup in the global proxy) in favor of @@ -19,17 +25,6 @@ const { bind, call } = Function.prototype; const uncurryThis = bind.bind(call); primordials.uncurryThis = uncurryThis; -function copyProps(src, dest) { - for (const key of Reflect.ownKeys(src)) { - if (!Reflect.getOwnPropertyDescriptor(dest, key)) { - Reflect.defineProperty( - dest, - key, - Reflect.getOwnPropertyDescriptor(src, key)); - } - } -} - function getNewKey(key) { return typeof key === 'symbol' ? `Symbol${key.description[7].toUpperCase()}${key.description.slice(8)}` : @@ -37,12 +32,12 @@ function getNewKey(key) { } function copyAccessor(dest, prefix, key, { enumerable, get, set }) { - Reflect.defineProperty(dest, `${prefix}Get${key}`, { + ReflectDefineProperty(dest, `${prefix}Get${key}`, { value: uncurryThis(get), enumerable }); if (set !== undefined) { - Reflect.defineProperty(dest, `${prefix}Set${key}`, { + ReflectDefineProperty(dest, `${prefix}Set${key}`, { value: uncurryThis(set), enumerable }); @@ -50,128 +45,46 @@ function copyAccessor(dest, prefix, key, { enumerable, get, set }) { } function copyPropsRenamed(src, dest, prefix) { - for (const key of Reflect.ownKeys(src)) { + for (const key of ReflectOwnKeys(src)) { const newKey = getNewKey(key); - const desc = Reflect.getOwnPropertyDescriptor(src, key); + const desc = ReflectGetOwnPropertyDescriptor(src, key); if ('get' in desc) { copyAccessor(dest, prefix, newKey, desc); } else { - Reflect.defineProperty(dest, `${prefix}${newKey}`, desc); + ReflectDefineProperty(dest, `${prefix}${newKey}`, desc); } } } function copyPropsRenamedBound(src, dest, prefix) { - for (const key of Reflect.ownKeys(src)) { + for (const key of ReflectOwnKeys(src)) { const newKey = getNewKey(key); - const desc = Reflect.getOwnPropertyDescriptor(src, key); + const desc = ReflectGetOwnPropertyDescriptor(src, key); if ('get' in desc) { copyAccessor(dest, prefix, newKey, desc); } else { if (typeof desc.value === 'function') { desc.value = desc.value.bind(src); } - Reflect.defineProperty(dest, `${prefix}${newKey}`, desc); + ReflectDefineProperty(dest, `${prefix}${newKey}`, desc); } } } function copyPrototype(src, dest, prefix) { - for (const key of Reflect.ownKeys(src)) { + for (const key of ReflectOwnKeys(src)) { const newKey = getNewKey(key); - const desc = Reflect.getOwnPropertyDescriptor(src, key); + const desc = ReflectGetOwnPropertyDescriptor(src, key); if ('get' in desc) { copyAccessor(dest, prefix, newKey, desc); } else { if (typeof desc.value === 'function') { desc.value = uncurryThis(desc.value); } - Reflect.defineProperty(dest, `${prefix}${newKey}`, desc); - } - } -} - -const createSafeIterator = (factory, next) => { - class SafeIterator { - constructor(iterable) { - this._iterator = factory(iterable); - } - next() { - return next(this._iterator); - } - [Symbol.iterator]() { - return this; - } - } - Object.setPrototypeOf(SafeIterator.prototype, null); - Object.freeze(SafeIterator.prototype); - Object.freeze(SafeIterator); - return SafeIterator; -}; - -function makeSafe(unsafe, safe) { - if (Symbol.iterator in unsafe.prototype) { - const dummy = new unsafe(); - let next; // We can reuse the same `next` method. - - for (const key of Reflect.ownKeys(unsafe.prototype)) { - if (!Reflect.getOwnPropertyDescriptor(safe.prototype, key)) { - const desc = Reflect.getOwnPropertyDescriptor(unsafe.prototype, key); - if ( - typeof desc.value === 'function' && - desc.value.length === 0 && - Symbol.iterator in (desc.value.call(dummy) ?? {}) - ) { - const createIterator = uncurryThis(desc.value); - next ??= uncurryThis(createIterator(dummy).next); - const SafeIterator = createSafeIterator(createIterator, next); - desc.value = function() { - return new SafeIterator(this); - }; - } - Reflect.defineProperty(safe.prototype, key, desc); - } + ReflectDefineProperty(dest, `${prefix}${newKey}`, desc); } - } else { - copyProps(unsafe.prototype, safe.prototype); } - copyProps(unsafe, safe); - - Object.setPrototypeOf(safe.prototype, null); - Object.freeze(safe.prototype); - Object.freeze(safe); - return safe; } -primordials.makeSafe = makeSafe; - -// Subclass the constructors because we need to use their prototype -// methods later. -// Defining the `constructor` is necessary here to avoid the default -// constructor which uses the user-mutable `%ArrayIteratorPrototype%.next`. -primordials.SafeMap = makeSafe( - Map, - class SafeMap extends Map { - constructor(i) { super(i); } // eslint-disable-line no-useless-constructor - } -); -primordials.SafeWeakMap = makeSafe( - WeakMap, - class SafeWeakMap extends WeakMap { - constructor(i) { super(i); } // eslint-disable-line no-useless-constructor - } -); -primordials.SafeSet = makeSafe( - Set, - class SafeSet extends Set { - constructor(i) { super(i); } // eslint-disable-line no-useless-constructor - } -); -primordials.SafeWeakSet = makeSafe( - WeakSet, - class SafeWeakSet extends WeakSet { - constructor(i) { super(i); } // eslint-disable-line no-useless-constructor - } -); // Create copies of the namespace objects [ @@ -256,6 +169,41 @@ primordials.SafeWeakSet = makeSafe( copyPrototype(original.prototype, primordials, `${name}Prototype`); }); +/* eslint-enable node-core/prefer-primordials */ + +const { + ArrayPrototypeForEach, + FunctionPrototypeCall, + Map, + ObjectFreeze, + ObjectSetPrototypeOf, + Set, + SymbolIterator, + WeakMap, + WeakSet, +} = primordials; + +// Because these functions are used by `makeSafe`, which is exposed +// on the `primordials` object, it's important to use const references +// to the primordials that they use: +const createSafeIterator = (factory, next) => { + class SafeIterator { + constructor(iterable) { + this._iterator = factory(iterable); + } + next() { + return next(this._iterator); + } + [SymbolIterator]() { + return this; + } + } + ObjectSetPrototypeOf(SafeIterator.prototype, null); + ObjectFreeze(SafeIterator.prototype); + ObjectFreeze(SafeIterator); + return SafeIterator; +}; + primordials.SafeArrayIterator = createSafeIterator( primordials.ArrayPrototypeSymbolIterator, primordials.ArrayIteratorPrototypeNext @@ -265,5 +213,80 @@ primordials.SafeStringIterator = createSafeIterator( primordials.StringIteratorPrototypeNext ); -Object.setPrototypeOf(primordials, null); -Object.freeze(primordials); +const copyProps = (src, dest) => { + ArrayPrototypeForEach(ReflectOwnKeys(src), (key) => { + if (!ReflectGetOwnPropertyDescriptor(dest, key)) { + ReflectDefineProperty( + dest, + key, + ReflectGetOwnPropertyDescriptor(src, key)); + } + }); +}; + +const makeSafe = (unsafe, safe) => { + if (SymbolIterator in unsafe.prototype) { + const dummy = new unsafe(); + let next; // We can reuse the same `next` method. + + ArrayPrototypeForEach(ReflectOwnKeys(unsafe.prototype), (key) => { + if (!ReflectGetOwnPropertyDescriptor(safe.prototype, key)) { + const desc = ReflectGetOwnPropertyDescriptor(unsafe.prototype, key); + if ( + typeof desc.value === 'function' && + desc.value.length === 0 && + SymbolIterator in (FunctionPrototypeCall(desc.value, dummy) ?? {}) + ) { + const createIterator = uncurryThis(desc.value); + next ??= uncurryThis(createIterator(dummy).next); + const SafeIterator = createSafeIterator(createIterator, next); + desc.value = function() { + return new SafeIterator(this); + }; + } + ReflectDefineProperty(safe.prototype, key, desc); + } + }); + } else { + copyProps(unsafe.prototype, safe.prototype); + } + copyProps(unsafe, safe); + + ObjectSetPrototypeOf(safe.prototype, null); + ObjectFreeze(safe.prototype); + ObjectFreeze(safe); + return safe; +}; +primordials.makeSafe = makeSafe; + +// Subclass the constructors because we need to use their prototype +// methods later. +// Defining the `constructor` is necessary here to avoid the default +// constructor which uses the user-mutable `%ArrayIteratorPrototype%.next`. +primordials.SafeMap = makeSafe( + Map, + class SafeMap extends Map { + constructor(i) { super(i); } // eslint-disable-line no-useless-constructor + } +); +primordials.SafeWeakMap = makeSafe( + WeakMap, + class SafeWeakMap extends WeakMap { + constructor(i) { super(i); } // eslint-disable-line no-useless-constructor + } +); +primordials.SafeSet = makeSafe( + Set, + class SafeSet extends Set { + constructor(i) { super(i); } // eslint-disable-line no-useless-constructor + } +); +primordials.SafeWeakSet = makeSafe( + WeakSet, + class SafeWeakSet extends WeakSet { + constructor(i) { super(i); } // eslint-disable-line no-useless-constructor + } +); + +ObjectSetPrototypeOf(primordials, null); +ObjectFreeze(primordials);