From d8d34ae6bc65167bdab925dccba89e9278017b84 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 5 Aug 2022 10:01:32 +0200 Subject: [PATCH] lib: reset `RegExp` statics before running user code Fixes: https://github.com/nodejs/node/issues/43740 Backport-PR-URL: https://github.com/nodejs/node/pull/43741 Reviewed-By: Ruben Bridgewater Reviewed-By: Geoffrey Booth Reviewed-By: Matteo Collina PR-URL: https://github.com/nodejs/node/pull/44247 Reviewed-By: Rafael Gonzaga --- lib/fs.js | 4 +- lib/internal/main/run_main_module.js | 5 ++ lib/internal/main/worker_thread.js | 4 ++ lib/internal/modules/esm/translators.js | 7 +- lib/internal/process/execution.js | 3 + lib/internal/url.js | 5 +- lib/internal/util.js | 18 +++++ .../test-startup-empty-regexp-statics.js | 68 +++++++++++++++++++ .../test-startup-empty-regexp-statics.mjs | 26 +++++++ .../test-vm-measure-memory-multi-context.js | 2 +- test/parallel/test-vm-measure-memory.js | 6 +- 11 files changed, 136 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-startup-empty-regexp-statics.js create mode 100644 test/parallel/test-startup-empty-regexp-statics.mjs diff --git a/lib/fs.js b/lib/fs.js index 874b6259431ba0..ffa216f35388e0 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -37,7 +37,6 @@ const { ObjectDefineProperty, Promise, ReflectApply, - RegExpPrototypeExec, SafeMap, SafeSet, String, @@ -90,6 +89,7 @@ const { promisify: { custom: kCustomPromisifiedSymbol, }, + SideEffectFreeRegExpPrototypeExec, } = require('internal/util'); const { constants: { @@ -2424,7 +2424,7 @@ if (isWindows) { // slash. const splitRootRe = /^(?:[a-zA-Z]:|[\\/]{2}[^\\/]+[\\/][^\\/]+)?[\\/]*/; splitRoot = function splitRoot(str) { - return RegExpPrototypeExec(splitRootRe, str)[0]; + return SideEffectFreeRegExpPrototypeExec(splitRootRe, str)[0]; }; } else { splitRoot = function splitRoot(str) { diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 1c2d421fc08996..b7347c4d457bd7 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -1,5 +1,7 @@ 'use strict'; +const { RegExpPrototypeExec } = primordials; + const { prepareMainThreadExecution, markBootstrapComplete @@ -9,6 +11,9 @@ prepareMainThreadExecution(true); markBootstrapComplete(); +// Necessary to reset RegExp statics before user code runs. +RegExpPrototypeExec(/^/, ''); + // Note: this loads the module through the ESM loader if the module is // determined to be an ES module. This hangs from the CJS module loader // because we currently allow monkey-patching of the module loaders diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js index 01f711aab25ccd..41cf85ccda8ff2 100644 --- a/lib/internal/main/worker_thread.js +++ b/lib/internal/main/worker_thread.js @@ -9,6 +9,7 @@ const { ArrayPrototypeSplice, ObjectDefineProperty, PromisePrototypeThen, + RegExpPrototypeExec, globalThis: { Atomics }, } = primordials; @@ -267,4 +268,7 @@ process._fatalException = workerOnGlobalUncaughtException; markBootstrapComplete(); +// Necessary to reset RegExp statics before user code runs. +RegExpPrototypeExec(/^/, ''); + port.start(); diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 0916e5604e80f6..6f25b2e67ab776 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -11,7 +11,7 @@ const { SafeArrayIterator, SafeMap, SafeSet, - StringPrototypeReplace, + StringPrototypeReplaceAll, StringPrototypeSlice, StringPrototypeStartsWith, SyntaxErrorPrototype, @@ -144,14 +144,13 @@ function enrichCJSError(err, content, filename) { // Strategy for loading a node-style CommonJS module const isWindows = process.platform === 'win32'; -const winSepRegEx = /\//g; translators.set('commonjs', async function commonjsStrategy(url, source, isMain) { debug(`Translating CJSModule ${url}`); let filename = internalURLModule.fileURLToPath(new URL(url)); if (isWindows) - filename = StringPrototypeReplace(filename, winSepRegEx, '\\'); + filename = StringPrototypeReplaceAll(filename, '/', '\\'); if (!cjsParse) await initCJSParse(); const { module, exportNames } = cjsPreparseModuleExports(filename); @@ -274,7 +273,7 @@ translators.set('json', async function jsonStrategy(url, source) { let module; if (pathname) { modulePath = isWindows ? - StringPrototypeReplace(pathname, winSepRegEx, '\\') : pathname; + StringPrototypeReplaceAll(pathname, '/', '\\') : pathname; module = CJSModule._cache[modulePath]; if (module && module.loaded) { const exports = module.exports; diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js index 20b66d57ca51c9..44d73164eb4234 100644 --- a/lib/internal/process/execution.js +++ b/lib/internal/process/execution.js @@ -1,6 +1,7 @@ 'use strict'; const { + RegExpPrototypeExec, globalThis, } = primordials; @@ -45,6 +46,7 @@ function evalModule(source, print) { } const { loadESM } = require('internal/process/esm_loader'); const { handleMainPromise } = require('internal/modules/run_main'); + RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. return handleMainPromise(loadESM((loader) => loader.eval(source))); } @@ -72,6 +74,7 @@ function evalScript(name, body, breakFirstLine, print) { return (main) => main(); `; globalThis.__filename = name; + RegExpPrototypeExec(/^/, ''); // Necessary to reset RegExp statics before user code runs. const result = module._compile(script, `${name}-wrapper`)(() => require('vm').runInThisContext(body, { filename: name, diff --git a/lib/internal/url.js b/lib/internal/url.js index b483b4440ebf23..2a4ffefe245070 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -23,6 +23,7 @@ const { StringPrototypeCharCodeAt, StringPrototypeIncludes, StringPrototypeReplace, + StringPrototypeReplaceAll, StringPrototypeSlice, StringPrototypeSplit, StringPrototypeStartsWith, @@ -1424,8 +1425,6 @@ function urlToHttpOptions(url) { return options; } -const forwardSlashRegEx = /\//g; - function getPathFromURLWin32(url) { const hostname = url.hostname; let pathname = url.pathname; @@ -1440,7 +1439,7 @@ function getPathFromURLWin32(url) { } } } - pathname = pathname.replace(forwardSlashRegEx, '\\'); + pathname = StringPrototypeReplaceAll(pathname, '/', '\\'); pathname = decodeURIComponent(pathname); if (hostname !== '') { // If hostname is set, then we have a UNC path diff --git a/lib/internal/util.js b/lib/internal/util.js index 15fc5ad46b7629..e250a798af7153 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -7,6 +7,7 @@ const { ArrayPrototypeSlice, ArrayPrototypeSort, Error, + FunctionPrototypeCall, ObjectCreate, ObjectDefineProperties, ObjectDefineProperty, @@ -543,6 +544,21 @@ function setOwnProperty(obj, key, value) { }); } +let internalGlobal; +function getInternalGlobal() { + if (internalGlobal == null) { + // Lazy-load to avoid a circular dependency. + const { runInNewContext } = require('vm'); + internalGlobal = runInNewContext('this', undefined, { contextName: 'internal' }); + } + return internalGlobal; +} + +function SideEffectFreeRegExpPrototypeExec(regex, string) { + const { RegExp: RegExpFromAnotherRealm } = getInternalGlobal(); + return FunctionPrototypeCall(RegExpFromAnotherRealm.prototype.exec, regex, string); +} + module.exports = { assertCrypto, cachedResult, @@ -557,6 +573,7 @@ module.exports = { filterDuplicateStrings, filterOwnProperties, getConstructorOf, + getInternalGlobal, getSystemErrorMap, getSystemErrorName, isError, @@ -567,6 +584,7 @@ module.exports = { normalizeEncoding, once, promisify, + SideEffectFreeRegExpPrototypeExec, sleep, spliceOne, toUSVString, diff --git a/test/parallel/test-startup-empty-regexp-statics.js b/test/parallel/test-startup-empty-regexp-statics.js new file mode 100644 index 00000000000000..80f6bef5a5a08e --- /dev/null +++ b/test/parallel/test-startup-empty-regexp-statics.js @@ -0,0 +1,68 @@ +'use strict'; + +const common = require('../common'); +const assert = require('node:assert'); +const { spawnSync, spawn } = require('node:child_process'); + +assert.strictEqual(RegExp.$_, ''); +assert.strictEqual(RegExp.$0, undefined); +assert.strictEqual(RegExp.$1, ''); +assert.strictEqual(RegExp.$2, ''); +assert.strictEqual(RegExp.$3, ''); +assert.strictEqual(RegExp.$4, ''); +assert.strictEqual(RegExp.$5, ''); +assert.strictEqual(RegExp.$6, ''); +assert.strictEqual(RegExp.$7, ''); +assert.strictEqual(RegExp.$8, ''); +assert.strictEqual(RegExp.$9, ''); +assert.strictEqual(RegExp.input, ''); +assert.strictEqual(RegExp.lastMatch, ''); +assert.strictEqual(RegExp.lastParen, ''); +assert.strictEqual(RegExp.leftContext, ''); +assert.strictEqual(RegExp.rightContext, ''); +assert.strictEqual(RegExp['$&'], ''); +assert.strictEqual(RegExp['$`'], ''); +assert.strictEqual(RegExp['$+'], ''); +assert.strictEqual(RegExp["$'"], ''); + +const allRegExpStatics = + 'RegExp.$_ + RegExp["$&"] + RegExp["$`"] + RegExp["$+"] + RegExp["$\'"] + ' + + 'RegExp.input + RegExp.lastMatch + RegExp.lastParen + ' + + 'RegExp.leftContext + RegExp.rightContext + ' + + Array.from({ length: 10 }, (_, i) => `RegExp.$${i}`).join(' + '); + +{ + const child = spawnSync(process.execPath, + [ '-p', allRegExpStatics ], + { stdio: ['inherit', 'pipe', 'inherit'] }); + assert.match(child.stdout.toString(), /^undefined\r?\n$/); + assert.strictEqual(child.status, 0); + assert.strictEqual(child.signal, null); +} + +{ + const child = spawnSync(process.execPath, + [ '-e', `console.log(${allRegExpStatics})`, '--input-type=module' ], + { stdio: ['inherit', 'pipe', 'inherit'] }); + assert.match(child.stdout.toString(), /^undefined\r?\n$/); + assert.strictEqual(child.status, 0); + assert.strictEqual(child.signal, null); +} + +{ + const child = spawn(process.execPath, [], { stdio: ['pipe', 'pipe', 'inherit'], encoding: 'utf8' }); + + let stdout = ''; + child.stdout.on('data', (chunk) => { + stdout += chunk; + }); + + child.on('exit', common.mustCall((status, signal) => { + assert.match(stdout, /^undefined\r?\n$/); + assert.strictEqual(status, 0); + assert.strictEqual(signal, null); + })); + child.on('error', common.mustNotCall()); + + child.stdin.end(`console.log(${allRegExpStatics});\n`); +} diff --git a/test/parallel/test-startup-empty-regexp-statics.mjs b/test/parallel/test-startup-empty-regexp-statics.mjs new file mode 100644 index 00000000000000..1f3869372b9690 --- /dev/null +++ b/test/parallel/test-startup-empty-regexp-statics.mjs @@ -0,0 +1,26 @@ +// We must load the CJS version here because the ESM wrapper call `hasIPv6` +// which compiles a RegEx. +// eslint-disable-next-line node-core/require-common-first +import '../common/index.js'; +import assert from 'node:assert'; + +assert.strictEqual(RegExp.$_, ''); +assert.strictEqual(RegExp.$0, undefined); +assert.strictEqual(RegExp.$1, ''); +assert.strictEqual(RegExp.$2, ''); +assert.strictEqual(RegExp.$3, ''); +assert.strictEqual(RegExp.$4, ''); +assert.strictEqual(RegExp.$5, ''); +assert.strictEqual(RegExp.$6, ''); +assert.strictEqual(RegExp.$7, ''); +assert.strictEqual(RegExp.$8, ''); +assert.strictEqual(RegExp.$9, ''); +assert.strictEqual(RegExp.input, ''); +assert.strictEqual(RegExp.lastMatch, ''); +assert.strictEqual(RegExp.lastParen, ''); +assert.strictEqual(RegExp.leftContext, ''); +assert.strictEqual(RegExp.rightContext, ''); +assert.strictEqual(RegExp['$&'], ''); +assert.strictEqual(RegExp['$`'], ''); +assert.strictEqual(RegExp['$+'], ''); +assert.strictEqual(RegExp["$'"], ''); diff --git a/test/parallel/test-vm-measure-memory-multi-context.js b/test/parallel/test-vm-measure-memory-multi-context.js index 3a3065a8edb631..aa9888d5f0442d 100644 --- a/test/parallel/test-vm-measure-memory-multi-context.js +++ b/test/parallel/test-vm-measure-memory-multi-context.js @@ -23,6 +23,6 @@ expectExperimentalWarning(); // We must hold on to the contexts here so that they // don't get GC'ed until the measurement is complete assert.strictEqual(arr.length, count); - assertDetailedShape(result, count); + assertDetailedShape(result, count + common.isWindows); })); } diff --git a/test/parallel/test-vm-measure-memory.js b/test/parallel/test-vm-measure-memory.js index 6b18db9be7a714..75625cb82f5f04 100644 --- a/test/parallel/test-vm-measure-memory.js +++ b/test/parallel/test-vm-measure-memory.js @@ -15,8 +15,10 @@ expectExperimentalWarning(); vm.measureMemory({ execution: 'eager' }) .then(common.mustCall(assertSummaryShape)); - vm.measureMemory({ mode: 'detailed', execution: 'eager' }) - .then(common.mustCall(assertSingleDetailedShape)); + if (!common.isWindows) { + vm.measureMemory({ mode: 'detailed', execution: 'eager' }) + .then(common.mustCall(assertSingleDetailedShape)); + } vm.measureMemory({ mode: 'summary', execution: 'eager' }) .then(common.mustCall(assertSummaryShape));