Skip to content

Commit 25ddcb2

Browse files
aduh95aduh95
committed
fixup! test: add escapePathsFromShellCommand util
1 parent 175dce6 commit 25ddcb2

12 files changed

+45
-89
lines changed

‎test/common/index.js‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,13 @@ const PIPE = (() => {
249249
// `$node --abort-on-uncaught-exception $file child`
250250
// the process aborts.
251251
function childShouldThrowAndAbort() {
252-
// Do not create core files, as it can take a lot of disk space on
253-
// continuous testing and developers' machines
254-
const uclimit = isWindows ?
255-
'' :
256-
'ulimit -c 0 && ';
257-
const child = exec(...escapePOSIXShell`${uclimit}"${process.argv[0]}" --abort-on-uncaught-exception "${process.argv[1]}" child`);
252+
const escapedArgs = escapePOSIXShell`"${process.argv[0]}" --abort-on-uncaught-exception "${process.argv[1]}" child`;
253+
if (!isWindows) {
254+
// Do not create core files, as it can take a lot of disk space on
255+
// continuous testing and developers' machines
256+
escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0];
257+
}
258+
const child = exec(...escapedArgs);
258259
child.on('exit', function onExit(exitCode, signal) {
259260
const errMsg = 'Test should have aborted ' +
260261
`but instead exited with exit code ${exitCode}` +

‎test/parallel/test-child-process-exec-maxbuf.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,15 @@ function runChecks(err, stdio, streamName, expected) {
1414
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
1515
// Windows, so we can simply pass the path.
1616
const execNode = (args, optionsOrCallback, callback) => {
17+
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" `;
1718
let options = optionsOrCallback;
1819
if (typeof optionsOrCallback === 'function') {
1920
options = undefined;
2021
callback = optionsOrCallback;
2122
}
2223
return cp.exec(
23-
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
24-
common.isWindows ? options : { ...options, env: { ...process.env, NODE: process.execPath } },
24+
cmd + args,
25+
{ ...opts, ...options },
2526
callback,
2627
);
2728
};

‎test/parallel/test-child-process-execsync-maxbuf.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ const { execSync } = require('child_process');
1010
const msgOut = 'this is stdout';
1111
const msgOutBuf = Buffer.from(`${msgOut}\n`);
1212

13-
const [cmd, opts] = escapePOSIXShell`"${process.execPath}" -e "console.log('${msgOut}')"`;
13+
const [cmd, opts] = escapePOSIXShell`"${process.execPath}" -e "${`console.log('${msgOut}')`}"`;
1414

1515
// Verify that an error is returned if maxBuffer is surpassed.
1616
{
@@ -51,7 +51,7 @@ const [cmd, opts] = escapePOSIXShell`"${process.execPath}" -e "console.log('${ms
5151

5252
// Default maxBuffer size is 1024 * 1024.
5353
{
54-
const ret = execSync(...escapePOSIXShell`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024))"`);
54+
const ret = execSync(...escapePOSIXShell`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024 - 1))"`);
5555

5656
assert.deepStrictEqual(
5757
ret.toString().trim(),

‎test/parallel/test-child-process-promisified.js‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,8 @@ const { promisify } = require('util');
77
const exec = promisify(child_process.exec);
88
const execFile = promisify(child_process.execFile);
99

10-
// The execPath might contain chars that should be escaped in a shell context.
11-
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
12-
// Windows, so we can simply pass the path.
13-
const execNode = (args) => exec(
14-
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
15-
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
16-
);
17-
1810
{
19-
const promise = execNode('-p 42');
11+
const promise = exec(...common.escapePOSIXShell`"${process.execPath}" -p 42`);
2012

2113
assert(promise.child instanceof child_process.ChildProcess);
2214
promise.then(common.mustCall((obj) => {
@@ -53,7 +45,7 @@ const execNode = (args) => exec(
5345
const failingCodeWithStdoutErr =
5446
'console.log(42);console.error(43);process.exit(1)';
5547
{
56-
execNode(`-e "${failingCodeWithStdoutErr}"`)
48+
exec(...common.escapePOSIXShell`"${process.execPath}" -e "${failingCodeWithStdoutErr}"`)
5749
.catch(common.mustCall((err) => {
5850
assert.strictEqual(err.code, 1);
5951
assert.strictEqual(err.stdout, '42\n');

‎test/parallel/test-cli-node-print-help.js‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,8 @@ const { exec, spawn } = require('child_process');
1111
const { once } = require('events');
1212
let stdOut;
1313

14-
// The execPath might contain chars that should be escaped in a shell context.
15-
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
16-
// Windows, so we can simply pass the path.
17-
const execNode = (args, callback) => exec(
18-
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
19-
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
20-
callback,
21-
);
22-
23-
2414
function startPrintHelpTest() {
25-
execNode('--help', common.mustSucceed((stdout, stderr) => {
15+
exec(...common.escapePOSIXShell`"${process.execPath}" --help`, common.mustSucceed((stdout, stderr) => {
2616
stdOut = stdout;
2717
validateNodePrintHelp();
2818
}));

‎test/parallel/test-cli-syntax-eval.js‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,10 @@ const common = require('../common');
44
const assert = require('assert');
55
const { exec } = require('child_process');
66

7-
// The execPath might contain chars that should be escaped in a shell context.
8-
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
9-
// Windows, so we can simply pass the path.
10-
const execNode = (args, callback) => exec(
11-
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
12-
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
13-
callback,
14-
);
15-
16-
177
// Should throw if -c and -e flags are both passed
188
['-c', '--check'].forEach(function(checkFlag) {
199
['-e', '--eval'].forEach(function(evalFlag) {
20-
const args = [checkFlag, evalFlag, 'foo'];
21-
execNode(args.join(' '), common.mustCall((err, stdout, stderr) => {
10+
exec(...common.escapePOSIXShell`"${process.execPath}" ${checkFlag} ${evalFlag} foo`, common.mustCall((err, stdout, stderr) => {
2211
assert.strictEqual(err instanceof Error, true);
2312
assert.strictEqual(err.code, 9);
2413
assert(

‎test/parallel/test-domain-abort-on-uncaught.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,15 @@ if (process.argv[2] === 'child') {
202202
} else {
203203

204204
tests.forEach(function(test, testIndex) {
205-
// Do not create core files, as it can take a lot of disk space on
206-
// continuous testing and developers' machines
207-
const ulimit = common.isWindows ? '' : 'ulimit -c 0 && ';
205+
const escapedArgs = common.escapePOSIXShell`"${process.execPath}" --abort-on-uncaught-exception "${__filename}" child ${testIndex}`;
206+
if (!common.isWindows) {
207+
// Do not create core files, as it can take a lot of disk space on
208+
// continuous testing and developers' machines
209+
escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0];
210+
}
208211

209212
try {
210-
child_process.execSync(...common.escapePOSIXShell`${ulimit}"${process.execPath}" --abort-on-uncaught-exception "${__filename}" child ${testIndex}`);
213+
child_process.execSync(...escapedArgs);
211214
} catch (e) {
212215
assert.fail(`Test index ${testIndex} failed: ${e}`);
213216
}

‎test/parallel/test-domain-throw-error-then-throw-from-uncaught-exception-handler.js‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,14 @@ function runTestWithAbortOnUncaughtException() {
8282
}
8383

8484
function createTestCmdLine(options) {
85-
// Do not create core files, as it can take a lot of disk space on
86-
// continuous testing and developers' machines
87-
const ulimit = common.isWindows ? '' : 'ulimit -c 0 && ';
88-
89-
return common.escapePOSIXShell`${ulimit}"${process.execPath}" ${
85+
const escapedArgs = common.escapePOSIXShell`"${process.execPath}" ${
9086
options?.withAbortOnUncaughtException ? '--abort-on-uncaught-exception' : ''
91-
} "${__filename} child`;
87+
} "${__filename}" child`;
88+
89+
if (!common.isWindows) {
90+
// Do not create core files, as it can take a lot of disk space on
91+
// continuous testing and developers' machines
92+
escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0];
93+
}
94+
return escapedArgs;
9295
}

‎test/parallel/test-domain-with-abort-on-uncaught-exception.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,17 @@ if (process.argv[2] === 'child') {
9191
if (options.throwInDomainErrHandler)
9292
throwInDomainErrHandlerOpt = 'throwInDomainErrHandler';
9393

94-
// Do not create core files, as it can take a lot of disk space on
95-
// continuous testing and developers' machines
96-
const ulimit = common.isWindows ? '' : 'ulimit -c 0 && ';
97-
9894
let useTryCatchOpt;
9995
if (options.useTryCatch)
10096
useTryCatchOpt = 'useTryCatch';
10197

102-
const child = exec(...common.escapePOSIXShell`${ulimit}"${process.execPath}" ${cmdLineOption || ''} "${__filename}" child ${throwInDomainErrHandlerOpt || ''} ${useTryCatchOpt || ''}`);
98+
const escapedArgs = common.escapePOSIXShell`"${process.execPath}" ${cmdLineOption || ''} "${__filename}" child ${throwInDomainErrHandlerOpt || ''} ${useTryCatchOpt || ''}`;
99+
if (!common.isWindows) {
100+
// Do not create core files, as it can take a lot of disk space on
101+
// continuous testing and developers' machines
102+
escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0];
103+
}
104+
const child = exec(...escapedArgs);
103105

104106
if (child) {
105107
child.on('exit', function onChildExited(exitCode, signal) {

‎test/parallel/test-permission-allow-child-process-cli.js‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,12 @@ if (process.argv[2] === 'child') {
1515
assert.ok(process.permission.has('child'));
1616
}
1717

18-
// The execPath might contain chars that should be escaped in a shell context.
19-
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
20-
// Windows, so we can simply pass the path.
21-
const execNode = (args) => childProcess.execSync(
22-
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
23-
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
24-
);
25-
2618
// When a permission is set by cli, the process shouldn't be able
2719
// to spawn unless --allow-child-process is sent
2820
{
2921
// doesNotThrow
3022
childProcess.spawnSync(process.execPath, ['--version']);
31-
execNode('--version');
23+
childProcess.exec(...common.escapePOSIXShell`"${process.execPath}" --version`);
3224
childProcess.fork(__filename, ['child']);
3325
childProcess.execFileSync(process.execPath, ['--version']);
3426
}

0 commit comments

Comments
 (0)