From 63f9abf2ca13e8d4539e57dd9f502944035bcb0d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 29 May 2021 20:23:05 +0200 Subject: [PATCH 1/2] lib: implement safe alternatives to `Promise` static methods --- lib/internal/modules/esm/module_job.js | 11 +++-- lib/internal/per_context/primordials.js | 55 +++++++++++++++++++++++ lib/internal/vm/module.js | 4 +- test/parallel/test-primordials-promise.js | 14 ++++++ 4 files changed, 76 insertions(+), 8 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index f91084a60f4609..7c71512cfcb32f 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -8,13 +8,12 @@ const { FunctionPrototype, ObjectCreate, ObjectSetPrototypeOf, - PromiseAll, PromiseResolve, PromisePrototypeCatch, ReflectApply, RegExpPrototypeExec, RegExpPrototypeSymbolReplace, - SafeArrayIterator, + SafePromiseAll, SafeSet, StringPrototypeIncludes, StringPrototypeSplit, @@ -82,9 +81,9 @@ class ModuleJob { }); if (promises !== undefined) - await PromiseAll(new SafeArrayIterator(promises)); + await SafePromiseAll(promises); - return PromiseAll(new SafeArrayIterator(dependencyJobs)); + return SafePromiseAll(dependencyJobs); }; // Promise for the list of all dependencyJobs. this.linked = link(); @@ -112,8 +111,8 @@ class ModuleJob { } jobsInGraph.add(moduleJob); const dependencyJobs = await moduleJob.linked; - return PromiseAll(new SafeArrayIterator( - ArrayPrototypeMap(dependencyJobs, addJobsToDependencyGraph))); + return SafePromiseAll( + ArrayPrototypeMap(dependencyJobs, addJobsToDependencyGraph)); }; await addJobsToDependencyGraph(this); diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index bbaad4ea760767..94515ca117e3ca 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -262,6 +262,7 @@ function copyPrototype(src, dest, prefix) { const { ArrayPrototypeForEach, + ArrayPrototypeMap, FinalizationRegistry, FunctionPrototypeCall, Map, @@ -434,5 +435,59 @@ primordials.AsyncIteratorPrototype = primordials.ReflectGetPrototypeOf( async function* () {}).prototype); +const arrayToSafePromiseIterable = (promises) => + new primordials.SafeArrayIterator( + ArrayPrototypeMap( + promises, + (promise) => + new SafePromise((a, b) => PromisePrototypeThen(promise, a, b)) + ) + ); + +/** + * @param {Promise[]} promises + * @returns {Promise} + */ +primordials.SafePromiseAll = (promises) => + // Wrapping on a new Promise is necessary to not expose the SafePromise + // prototype to user-land. + new Promise((a, b) => + SafePromise.all(arrayToSafePromiseIterable(promises)).then(a, b) + ); + +/** + * @param {Promise[]} promises + * @returns {Promise} + */ +primordials.SafePromiseAllSettled = (promises) => + // Wrapping on a new Promise is necessary to not expose the SafePromise + // prototype to user-land. + new Promise((a, b) => + SafePromise.allSettled(arrayToSafePromiseIterable(promises)).then(a, b) + ); + +/** + * @param {Promise[]} promises + * @returns {Promise} + */ +primordials.SafePromiseAny = (promises) => + // Wrapping on a new Promise is necessary to not expose the SafePromise + // prototype to user-land. + new Promise((a, b) => + SafePromise.any(arrayToSafePromiseIterable(promises)).then(a, b) + ); + +/** + * @param {Promise[]} promises + * @returns {Promise} + */ +primordials.SafePromiseRace = (promises) => + // Wrapping on a new Promise is necessary to not expose the SafePromise + // prototype to user-land. + new Promise((a, b) => + SafePromise.race(arrayToSafePromiseIterable(promises)).then(a, b) + ); + + ObjectSetPrototypeOf(primordials, null); ObjectFreeze(primordials); diff --git a/lib/internal/vm/module.js b/lib/internal/vm/module.js index d3818e6fd6c27c..fe72c16a97665d 100644 --- a/lib/internal/vm/module.js +++ b/lib/internal/vm/module.js @@ -10,8 +10,8 @@ const { ObjectDefineProperty, ObjectGetPrototypeOf, ObjectSetPrototypeOf, - PromiseAll, ReflectApply, + SafePromiseAll, SafeWeakMap, Symbol, SymbolToStringTag, @@ -330,7 +330,7 @@ class SourceTextModule extends Module { try { if (promises !== undefined) { - await PromiseAll(promises); + await SafePromiseAll(promises); } } catch (e) { this.#error = e; diff --git a/test/parallel/test-primordials-promise.js b/test/parallel/test-primordials-promise.js index 6165192938458f..7ff29fc0f5d407 100644 --- a/test/parallel/test-primordials-promise.js +++ b/test/parallel/test-primordials-promise.js @@ -7,9 +7,18 @@ const assert = require('assert'); const { PromisePrototypeCatch, PromisePrototypeThen, + SafePromiseAll, + SafePromiseAllSettled, + SafePromiseAny, SafePromisePrototypeFinally, + SafePromiseRace, } = require('internal/test/binding').primordials; +Array.prototype[Symbol.iterator] = common.mustNotCall(); +Promise.all = common.mustNotCall(); +Promise.allSettled = common.mustNotCall(); +Promise.any = common.mustNotCall(); +Promise.race = common.mustNotCall(); Promise.prototype.catch = common.mustNotCall(); Promise.prototype.finally = common.mustNotCall(); Promise.prototype.then = common.mustNotCall(); @@ -18,6 +27,11 @@ assertIsPromise(PromisePrototypeCatch(Promise.reject(), common.mustCall())); assertIsPromise(PromisePrototypeThen(test(), common.mustCall())); assertIsPromise(SafePromisePrototypeFinally(test(), common.mustCall())); +assertIsPromise(SafePromiseAll([test()])); +assertIsPromise(SafePromiseAllSettled([test()])); +assertIsPromise(SafePromiseAny([test()])); +assertIsPromise(SafePromiseRace([test()])); + async function test() { const catchFn = common.mustCall(); const finallyFn = common.mustCall(); From 418864eb20bee4b71f641e809a43511dde8b3af2 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 8 Jul 2022 11:47:26 +0200 Subject: [PATCH 2/2] fixup! lib: implement safe alternatives to `Promise` static methods --- lib/internal/modules/esm/module_job.js | 4 +--- lib/internal/per_context/primordials.js | 28 ++++++++++++++----------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 7c71512cfcb32f..3a2ec7343a1d53 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -2,7 +2,6 @@ const { ArrayPrototypeJoin, - ArrayPrototypeMap, ArrayPrototypePush, ArrayPrototypeSome, FunctionPrototype, @@ -111,8 +110,7 @@ class ModuleJob { } jobsInGraph.add(moduleJob); const dependencyJobs = await moduleJob.linked; - return SafePromiseAll( - ArrayPrototypeMap(dependencyJobs, addJobsToDependencyGraph)); + return SafePromiseAll(dependencyJobs, addJobsToDependencyGraph); }; await addJobsToDependencyGraph(this); diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 94515ca117e3ca..7c6f513d9ccebd 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -435,57 +435,61 @@ primordials.AsyncIteratorPrototype = primordials.ReflectGetPrototypeOf( async function* () {}).prototype); -const arrayToSafePromiseIterable = (promises) => +const arrayToSafePromiseIterable = (promises, mapFn) => new primordials.SafeArrayIterator( ArrayPrototypeMap( promises, - (promise) => - new SafePromise((a, b) => PromisePrototypeThen(promise, a, b)) + (promise, i) => + new SafePromise((a, b) => PromisePrototypeThen(mapFn == null ? promise : mapFn(promise, i), a, b)) ) ); /** * @param {Promise[]} promises + * @param {(v: Promise, k: number) => Promise} [mapFn] * @returns {Promise} */ -primordials.SafePromiseAll = (promises) => +primordials.SafePromiseAll = (promises, mapFn) => // Wrapping on a new Promise is necessary to not expose the SafePromise // prototype to user-land. new Promise((a, b) => - SafePromise.all(arrayToSafePromiseIterable(promises)).then(a, b) + SafePromise.all(arrayToSafePromiseIterable(promises, mapFn)).then(a, b) ); /** * @param {Promise[]} promises - * @returns {Promise} + * @param {(v: Promise, k: number) => Promise} [mapFn] + * @returns {Promise[]>} */ -primordials.SafePromiseAllSettled = (promises) => +primordials.SafePromiseAllSettled = (promises, mapFn) => // Wrapping on a new Promise is necessary to not expose the SafePromise // prototype to user-land. new Promise((a, b) => - SafePromise.allSettled(arrayToSafePromiseIterable(promises)).then(a, b) + SafePromise.allSettled(arrayToSafePromiseIterable(promises, mapFn)).then(a, b) ); /** * @param {Promise[]} promises + * @param {(v: Promise, k: number) => Promise} [mapFn] * @returns {Promise} */ -primordials.SafePromiseAny = (promises) => +primordials.SafePromiseAny = (promises, mapFn) => // Wrapping on a new Promise is necessary to not expose the SafePromise // prototype to user-land. new Promise((a, b) => - SafePromise.any(arrayToSafePromiseIterable(promises)).then(a, b) + SafePromise.any(arrayToSafePromiseIterable(promises, mapFn)).then(a, b) ); /** * @param {Promise[]} promises + * @param {(v: Promise, k: number) => Promise} [mapFn] * @returns {Promise} */ -primordials.SafePromiseRace = (promises) => +primordials.SafePromiseRace = (promises, mapFn) => // Wrapping on a new Promise is necessary to not expose the SafePromise // prototype to user-land. new Promise((a, b) => - SafePromise.race(arrayToSafePromiseIterable(promises)).then(a, b) + SafePromise.race(arrayToSafePromiseIterable(promises, mapFn)).then(a, b) );