From 65c52e30a3d48d283c2bac418326a2c116937592 Mon Sep 17 00:00:00 2001 From: Mathieu Hofman Date: Wed, 16 Mar 2022 02:38:43 +0000 Subject: [PATCH] Do not register async hook symbols as well-known --- packages/init/node-async_hooks-symbols.d.ts | 11 ---- packages/init/node-async_hooks.js | 58 ++++++++++++--------- packages/init/test/test-async_hooks.js | 8 +-- packages/ses/src/whitelist.js | 9 ++-- 4 files changed, 40 insertions(+), 46 deletions(-) delete mode 100644 packages/init/node-async_hooks-symbols.d.ts diff --git a/packages/init/node-async_hooks-symbols.d.ts b/packages/init/node-async_hooks-symbols.d.ts deleted file mode 100644 index 09fd7027de..0000000000 --- a/packages/init/node-async_hooks-symbols.d.ts +++ /dev/null @@ -1,11 +0,0 @@ -// @ts-check - -export {}; - -declare global { - interface SymbolConstructor { - readonly nodeAsyncHooksAsyncId: unique symbol; - readonly nodeAsyncHooksTriggerAsyncId: unique symbol; - readonly nodeAsyncHooksDestroyed: unique symbol; - } -} diff --git a/packages/init/node-async_hooks.js b/packages/init/node-async_hooks.js index 225788ed07..5949c8945f 100644 --- a/packages/init/node-async_hooks.js +++ b/packages/init/node-async_hooks.js @@ -1,27 +1,30 @@ import { createHook, AsyncResource } from 'async_hooks'; -/// - -const asyncHooksWellKnownNameFromDescription = { - async_id_symbol: 'nodeAsyncHooksAsyncId', - trigger_async_id_symbol: 'nodeAsyncHooksTriggerAsyncId', - destroyed: 'nodeAsyncHooksDestroyed', +const asyncHooksSymbols = { + async_id_symbol: undefined, + trigger_async_id_symbol: undefined, + destroyed: undefined, }; const promiseAsyncHookFallbackStates = new WeakMap(); const setAsyncSymbol = (description, symbol) => { - const wellKnownName = asyncHooksWellKnownNameFromDescription[description]; - if (!wellKnownName) { + if (!(description in asyncHooksSymbols)) { throw new Error('Unknown symbol'); - } else if (!Symbol[wellKnownName]) { - Symbol[wellKnownName] = symbol; + } else if (!asyncHooksSymbols[description]) { + if (symbol.description !== description) { + // Throw an error since the whitelist mechanism relies on the description + throw new Error( + `Mismatched symbol found for ${description}: ${String(symbol)}`, + ); + } + asyncHooksSymbols[description] = symbol; return true; - } else if (Symbol[wellKnownName] !== symbol) { + } else if (asyncHooksSymbols[description] !== symbol) { // process._rawDebug( // `Found duplicate ${description}:`, // symbol, - // Symbol[wellKnownName], + // asyncHooksSymbols[description], // ); return false; } else { @@ -37,7 +40,7 @@ const findAsyncSymbolsFromAsyncResource = () => { let found = 0; Object.getOwnPropertySymbols(new AsyncResource('Bootstrap')).forEach(sym => { const { description } = sym; - if (description in asyncHooksWellKnownNameFromDescription) { + if (description in asyncHooksSymbols) { if (setAsyncSymbol(description, sym)) { found += 1; } @@ -136,11 +139,11 @@ const getAsyncHookFallbackState = (promise, { create = false } = {}) => { let state = promiseAsyncHookFallbackStates.get(promise); if (!state && create) { state = { - [Symbol.nodeAsyncHooksAsyncId]: undefined, - [Symbol.nodeAsyncHooksTriggerAsyncId]: undefined, + [asyncHooksSymbols.async_id_symbol]: undefined, + [asyncHooksSymbols.trigger_async_id_symbol]: undefined, }; - if (Symbol.nodeAsyncHooksDestroyed) { - state[Symbol.nodeAsyncHooksDestroyed] = undefined; + if (asyncHooksSymbols.destroyed) { + state[asyncHooksSymbols.destroyed] = undefined; } promiseAsyncHookFallbackStates.set(promise, state); } @@ -196,7 +199,10 @@ export const setup = ({ withDestroy = true } = {}) => { findAsyncSymbolsFromAsyncResource(); } - if (!Symbol.nodeAsyncHooksAsyncId || !Symbol.nodeAsyncHooksTriggerAsyncId) { + if ( + !asyncHooksSymbols.async_id_symbol || + !asyncHooksSymbols.trigger_async_id_symbol + ) { // process._rawDebug(`Async symbols not found, moving on`); return; } @@ -204,20 +210,22 @@ export const setup = ({ withDestroy = true } = {}) => { const PromiseProto = Promise.prototype; Object.defineProperty( PromiseProto, - Symbol.nodeAsyncHooksAsyncId, - getAsyncHookSymbolPromiseProtoDesc(Symbol.nodeAsyncHooksAsyncId), + asyncHooksSymbols.async_id_symbol, + getAsyncHookSymbolPromiseProtoDesc(asyncHooksSymbols.async_id_symbol), ); Object.defineProperty( PromiseProto, - Symbol.nodeAsyncHooksTriggerAsyncId, - getAsyncHookSymbolPromiseProtoDesc(Symbol.nodeAsyncHooksTriggerAsyncId), + asyncHooksSymbols.trigger_async_id_symbol, + getAsyncHookSymbolPromiseProtoDesc( + asyncHooksSymbols.trigger_async_id_symbol, + ), ); - if (Symbol.nodeAsyncHooksDestroyed) { + if (asyncHooksSymbols.destroyed) { Object.defineProperty( PromiseProto, - Symbol.nodeAsyncHooksDestroyed, - getAsyncHookSymbolPromiseProtoDesc(Symbol.nodeAsyncHooksDestroyed, { + asyncHooksSymbols.destroyed, + getAsyncHookSymbolPromiseProtoDesc(asyncHooksSymbols.destroyed, { disallowGet: true, }), ); diff --git a/packages/init/test/test-async_hooks.js b/packages/init/test/test-async_hooks.js index fa76f403ed..7941da6e33 100644 --- a/packages/init/test/test-async_hooks.js +++ b/packages/init/test/test-async_hooks.js @@ -16,8 +16,8 @@ const gcP = (async () => { })(); test('async_hooks Promise patch', async t => { - const hasSymbols = - Symbol.nodeAsyncHooksAsyncId && Symbol.nodeAsyncHooksTriggerAsyncId; + const hasAsyncSymbols = + Object.getOwnPropertySymbols(Promise.prototype).length > 1; let resolve; const q = (() => { const p1 = new Promise(r => (resolve = r)); @@ -48,7 +48,7 @@ test('async_hooks Promise patch', async t => { // Create a promise with symbols attached const p3 = Promise.resolve(); - t.is(Reflect.ownKeys(p3).length > 0, hasSymbols); + t.is(Reflect.ownKeys(p3).length > 0, hasAsyncSymbols); return Promise.resolve().then(() => { resolve(8); @@ -60,7 +60,7 @@ test('async_hooks Promise patch', async t => { // node versions will fail and generate a new one because of an own check p1.then(() => {}); - t.is(Reflect.ownKeys(ret).length > 0, hasSymbols); + t.is(Reflect.ownKeys(ret).length > 0, hasAsyncSymbols); // testHooks.disable(); diff --git a/packages/ses/src/whitelist.js b/packages/ses/src/whitelist.js index 11f713aa06..71377e4080 100644 --- a/packages/ses/src/whitelist.js +++ b/packages/ses/src/whitelist.js @@ -504,9 +504,6 @@ export const whitelist = { keyFor: fn, match: 'symbol', matchAll: 'symbol', - nodeAsyncHooksAsyncId: 'symbol', - nodeAsyncHooksTriggerAsyncId: 'symbol', - nodeAsyncHooksDestroyed: 'symbol', prototype: '%SymbolPrototype%', replace: 'symbol', search: 'symbol', @@ -1296,9 +1293,9 @@ export const whitelist = { then: fn, '@@toStringTag': 'string', // Non-standard, used in node to prevent async_hooks from breaking - '@@nodeAsyncHooksAsyncId': accessor, - '@@nodeAsyncHooksTriggerAsyncId': accessor, - '@@nodeAsyncHooksDestroyed': accessor, + 'UniqueSymbol(async_id_symbol)': accessor, + 'UniqueSymbol(trigger_async_id_symbol)': accessor, + 'UniqueSymbol(destroyed)': accessor, }, '%InertAsyncFunction%': {