lib: implement safe alternatives to Promise static methods

PR-URL: nodejs#43728 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
DeeDeeG · Jul 10, 2022 · 3fb5784 · 3fb5784
1 parent 2f93a28
commit 3fb5784
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 9 deletions.
diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
@@ -2,19 +2,17 @@
 
 const {
   ArrayPrototypeJoin,
-  ArrayPrototypeMap,
   ArrayPrototypePush,
   ArrayPrototypeSome,
   FunctionPrototype,
   ObjectCreate,
   ObjectSetPrototypeOf,
-  PromiseAll,
   PromiseResolve,
   PromisePrototypeCatch,
   ReflectApply,
   RegExpPrototypeExec,
   RegExpPrototypeSymbolReplace,
-  SafeArrayIterator,
+  SafePromiseAll,
   SafeSet,
   StringPrototypeIncludes,
   StringPrototypeSplit,
@@ -82,9 +80,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 +110,7 @@ class ModuleJob {
       }
       jobsInGraph.add(moduleJob);
       const dependencyJobs = await moduleJob.linked;
-      return PromiseAll(new SafeArrayIterator(
-        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
@@ -262,6 +262,7 @@ function copyPrototype(src, dest, prefix) {
 
 const {
   ArrayPrototypeForEach,
+  ArrayPrototypeMap,
   FinalizationRegistry,
   FunctionPrototypeCall,
   Map,
@@ -434,5 +435,63 @@ primordials.AsyncIteratorPrototype =
     primordials.ReflectGetPrototypeOf(
       async function* () {}).prototype);
 
+const arrayToSafePromiseIterable = (promises, mapFn) =>
+  new primordials.SafeArrayIterator(
+    ArrayPrototypeMap(
+      promises,
+      (promise, i) =>
+        new SafePromise((a, b) => PromisePrototypeThen(mapFn == null ? promise : mapFn(promise, i), a, b))
+    )
+  );
+
+/**
+ * @param {Promise<any>[]} promises
+ * @param {(v: Promise<any>, k: number) => Promise<any>} [mapFn]
+ * @returns {Promise<any[]>}
+ */
+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, mapFn)).then(a, b)
+  );
+
+/**
+ * @param {Promise<any>[]} promises
+ * @param {(v: Promise<any>, k: number) => Promise<any>} [mapFn]
+ * @returns {Promise<PromiseSettledResult<any>[]>}
+ */
+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, mapFn)).then(a, b)
+  );
+
+/**
+ * @param {Promise<any>[]} promises
+ * @param {(v: Promise<any>, k: number) => Promise<any>} [mapFn]
+ * @returns {Promise<any>}
+ */
+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, mapFn)).then(a, b)
+  );
+
+/**
+ * @param {Promise<any>[]} promises
+ * @param {(v: Promise<any>, k: number) => Promise<any>} [mapFn]
+ * @returns {Promise<any>}
+ */
+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, mapFn)).then(a, b)
+  );
+
+
 ObjectSetPrototypeOf(primordials, null);
 ObjectFreeze(primordials);
diff --git 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
@@ -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();