Skip to content

Commit

Permalink
lib: reset RegExp statics before running user code
Browse files Browse the repository at this point in the history
Fixes: nodejs#43740

PR-URL: nodejs#43741
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
aduh95 committed Aug 23, 2022
1 parent ab73cc8 commit aa76869
Show file tree
Hide file tree
Showing 11 changed files with 136 additions and 12 deletions.
4 changes: 2 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ const {
ObjectDefineProperty,
Promise,
ReflectApply,
RegExpPrototypeExec,
SafeMap,
SafeSet,
String,
Expand Down Expand Up @@ -90,6 +89,7 @@ const {
promisify: {
custom: kCustomPromisifiedSymbol,
},
SideEffectFreeRegExpPrototypeExec,
} = require('internal/util');
const {
constants: {
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/main/run_main_module.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const { RegExpPrototypeExec } = primordials;

const {
prepareMainThreadExecution,
markBootstrapComplete
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions lib/internal/main/worker_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const {
ArrayPrototypeSplice,
ObjectDefineProperty,
PromisePrototypeThen,
RegExpPrototypeExec,
globalThis: { Atomics },
} = primordials;

Expand Down Expand Up @@ -267,4 +268,7 @@ process._fatalException = workerOnGlobalUncaughtException;

markBootstrapComplete();

// Necessary to reset RegExp statics before user code runs.
RegExpPrototypeExec(/^/, '');

port.start();
7 changes: 3 additions & 4 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const {
SafeArrayIterator,
SafeMap,
SafeSet,
StringPrototypeReplace,
StringPrototypeReplaceAll,
StringPrototypeSlice,
StringPrototypeStartsWith,
SyntaxErrorPrototype,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/process/execution.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const {
RegExpPrototypeExec,
globalThis,
} = primordials;

Expand Down Expand Up @@ -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)));
}

Expand Down Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions lib/internal/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const {
StringPrototypeCharCodeAt,
StringPrototypeIncludes,
StringPrototypeReplace,
StringPrototypeReplaceAll,
StringPrototypeSlice,
StringPrototypeSplit,
StringPrototypeStartsWith,
Expand Down Expand Up @@ -1424,8 +1425,6 @@ function urlToHttpOptions(url) {
return options;
}

const forwardSlashRegEx = /\//g;

function getPathFromURLWin32(url) {
const hostname = url.hostname;
let pathname = url.pathname;
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions lib/internal/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
ArrayPrototypeSlice,
ArrayPrototypeSort,
Error,
FunctionPrototypeCall,
ObjectCreate,
ObjectDefineProperties,
ObjectDefineProperty,
Expand Down Expand Up @@ -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,
Expand All @@ -557,6 +573,7 @@ module.exports = {
filterDuplicateStrings,
filterOwnProperties,
getConstructorOf,
getInternalGlobal,
getSystemErrorMap,
getSystemErrorName,
isError,
Expand All @@ -567,6 +584,7 @@ module.exports = {
normalizeEncoding,
once,
promisify,
SideEffectFreeRegExpPrototypeExec,
sleep,
spliceOne,
toUSVString,
Expand Down
68 changes: 68 additions & 0 deletions test/parallel/test-startup-empty-regexp-statics.js
Original file line number Diff line number Diff line change
@@ -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`);
}
26 changes: 26 additions & 0 deletions test/parallel/test-startup-empty-regexp-statics.mjs
Original file line number Diff line number Diff line change
@@ -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["$'"], '');
2 changes: 1 addition & 1 deletion test/parallel/test-vm-measure-memory-multi-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
}
6 changes: 4 additions & 2 deletions test/parallel/test-vm-measure-memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit aa76869

Please sign in to comment.