Fix safe promise checks (#1264)

endojs · Aug 24, 2022 · 8e8d41c · 8e8d41c
2 parents e896ac2 + 94cbce7
commit 8e8d41c
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 23 deletions.
diff --git a/packages/marshal/src/helpers/safe-promise.js b/packages/marshal/src/helpers/safe-promise.js
@@ -3,14 +3,48 @@
 /// <reference types="ses"/>
 
 import { isPromise } from '@endo/promise-kit';
-import { assertChecker } from './passStyle-helpers.js';
+import { assertChecker, hasOwnPropertyOf } from './passStyle-helpers.js';
 
 /** @typedef {import('../types.js').Checker} Checker */
 
 const { details: X, quote: q } = assert;
 const { isFrozen, getPrototypeOf } = Object;
 const { ownKeys } = Reflect;
 
+/**
+ * @param {Promise} pr The value to examine
+ * @param {Checker} [check]
+ * @returns {pr is Promise} Whether it is a safe promise
+ */
+const checkPromiseOwnKeys = (pr, check = x => x) => {
+ const keys = ownKeys(pr);
+
+ const unknownKeys = keys.filter(
+ key => typeof key !== 'symbol' || !hasOwnPropertyOf(Promise.prototype, key),
+ );
+
+ return (
+ check(
+ unknownKeys.length === 0,
+ X`${pr} - Must not have any own properties: ${q(unknownKeys)}`,
+ ) &&
+ check(
+ keys.filter(key => {
+ const val = pr[key];
+ return !(
+ val === undefined ||
+ typeof val === 'number' ||
+ (typeof val === 'object' &&
+ isFrozen(val) &&
+ getPrototypeOf(val) === Object.prototype &&
+ ownKeys(val).length === 0)
+ );
+ }).length === 0,
+ X`${pr} - async_hooks own keys have unexpected values`,
+ )
+ );
+};
+
 /**
  * Under Hardened JS a promise is "safe" if its `then` method can be called
  * synchronously without giving the promise an opportunity for a
@@ -27,26 +61,14 @@ const { ownKeys } = Reflect;
  * @param {Checker} [check]
  * @returns {pr is Promise} Whether it is a safe promise
  */
-const checkSafePromise = (pr, check = x => x) => {
- let keys;
- return (
- check(isFrozen(pr), X`${pr} - Must be frozen`) &&
- check(isPromise(pr), X`${pr} - Must be a promise`) &&
- check(
- getPrototypeOf(pr) === Promise.prototype,
- X`${pr} - Must inherit from Promise.prototype: ${q(getPrototypeOf(pr))}`,
- ) &&
- check(
- // Suppressing prettier for the following line because it wants to
- // remove the "extra" parens around `pr`. However, these parens are
- // required for the TypeScript case syntax. We know this case is safe
- // because we only get here if `ifPromise(pr)` already passed.
- // eslint-disable-next-line prettier/prettier
- (keys = ownKeys(/** @type {Promise} pr */(pr))).length === 0,
- X`{pr} - Must not have any own properties: ${q(keys)}`,
- )
- );
-};
+const checkSafePromise = (pr, check = x => x) =>
+ check(isFrozen(pr), X`${pr} - Must be frozen`) &&
+ check(isPromise(pr), X`${pr} - Must be a promise`) &&
+ check(
+ getPrototypeOf(pr) === Promise.prototype,
+ X`${pr} - Must inherit from Promise.prototype: ${q(getPrototypeOf(pr))}`,
+ ) &&
+ checkPromiseOwnKeys(/** @type {Promise} */ (pr), check);
 harden(checkSafePromise);
 
 /**

diff --git a/packages/marshal/test/test-passStyleOf.js b/packages/marshal/test/test-passStyleOf.js
@@ -45,14 +45,14 @@ test('some passStyleOf rejections', t => {
  prbad2.extra = 'unexpected own property';
  harden(prbad2);
  t.throws(() => passStyleOf(prbad2), {
- message: /{pr} - Must not have any own properties: \["extra"\]/,
+ message: /\[Promise\]" - Must not have any own properties: \["extra"\]/,
  });
 
  const prbad3 = Promise.resolve();
  Object.defineProperty(prbad3, 'then', { value: () => 'bad then' });
  harden(prbad3);
  t.throws(() => passStyleOf(prbad3), {
- message: /{pr} - Must not have any own properties: \["then"\]/,
+ message: /\[Promise\]" - Must not have any own properties: \["then"\]/,
  });
 
  const thenable1 = harden({ then: () => 'thenable' });