Skip to content

Commit a50dd21

Browse files
aduh95targos
aduh95
authored and
targos
committed
test: do not assume process.execPath contains no spaces
We had a bunch of tests that would fail if run from an executable that contains any char that should be escaped when run from a shell. PR-URL: #55028 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent c56e324 commit a50dd21

16 files changed

+151
-88
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ const invalidArgTypeError = {
1111
};
1212

1313
const waitCommand = common.isWindows ?
14-
`${process.execPath} -e "setInterval(()=>{}, 99)"` :
14+
// `"` is forbidden for Windows paths, no need for escaping.
15+
`"${process.execPath}" -e "setInterval(()=>{}, 99)"` :
1516
'sleep 2m';
1617

1718
{

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

+32-39
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,25 @@ function runChecks(err, stdio, streamName, expected) {
1010
assert.deepStrictEqual(stdio[streamName], expected);
1111
}
1212

13+
// The execPath might contain chars that should be escaped in a shell context.
14+
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
15+
// Windows, so we can simply pass the path.
16+
const execNode = (args, optionsOrCallback, callback) => {
17+
let options = optionsOrCallback;
18+
if (typeof optionsOrCallback === 'function') {
19+
options = undefined;
20+
callback = optionsOrCallback;
21+
}
22+
return cp.exec(
23+
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
24+
common.isWindows ? options : { ...options, env: { ...process.env, NODE: process.execPath } },
25+
callback,
26+
);
27+
};
28+
1329
// default value
1430
{
15-
const cmd =
16-
`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024))"`;
17-
18-
cp.exec(cmd, common.mustCall((err) => {
31+
execNode(`-e "console.log('a'.repeat(1024 * 1024))"`, common.mustCall((err) => {
1932
assert(err instanceof RangeError);
2033
assert.strictEqual(err.message, 'stdout maxBuffer length exceeded');
2134
assert.strictEqual(err.code, 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER');
@@ -24,20 +37,16 @@ function runChecks(err, stdio, streamName, expected) {
2437

2538
// default value
2639
{
27-
const cmd =
28-
`${process.execPath} -e "console.log('a'.repeat(1024 * 1024 - 1))"`;
29-
30-
cp.exec(cmd, common.mustSucceed((stdout, stderr) => {
40+
execNode(`-e "console.log('a'.repeat(1024 * 1024 - 1))"`, common.mustSucceed((stdout, stderr) => {
3141
assert.strictEqual(stdout.trim(), 'a'.repeat(1024 * 1024 - 1));
3242
assert.strictEqual(stderr, '');
3343
}));
3444
}
3545

3646
{
37-
const cmd = `"${process.execPath}" -e "console.log('hello world');"`;
3847
const options = { maxBuffer: Infinity };
3948

40-
cp.exec(cmd, options, common.mustSucceed((stdout, stderr) => {
49+
execNode(`-e "console.log('hello world');"`, options, common.mustSucceed((stdout, stderr) => {
4150
assert.strictEqual(stdout.trim(), 'hello world');
4251
assert.strictEqual(stderr, '');
4352
}));
@@ -57,11 +66,8 @@ function runChecks(err, stdio, streamName, expected) {
5766

5867
// default value
5968
{
60-
const cmd =
61-
`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024))"`;
62-
63-
cp.exec(
64-
cmd,
69+
execNode(
70+
`-e "console.log('a'.repeat(1024 * 1024))"`,
6571
common.mustCall((err, stdout, stderr) => {
6672
runChecks(
6773
err,
@@ -75,10 +81,7 @@ function runChecks(err, stdio, streamName, expected) {
7581

7682
// default value
7783
{
78-
const cmd =
79-
`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024 - 1))"`;
80-
81-
cp.exec(cmd, common.mustSucceed((stdout, stderr) => {
84+
execNode(`-e "console.log('a'.repeat(1024 * 1024 - 1))"`, common.mustSucceed((stdout, stderr) => {
8285
assert.strictEqual(stdout.trim(), 'a'.repeat(1024 * 1024 - 1));
8386
assert.strictEqual(stderr, '');
8487
}));
@@ -87,10 +90,8 @@ function runChecks(err, stdio, streamName, expected) {
8790
const unicode = '中文测试'; // length = 4, byte length = 12
8891

8992
{
90-
const cmd = `"${process.execPath}" -e "console.log('${unicode}');"`;
91-
92-
cp.exec(
93-
cmd,
93+
execNode(
94+
`-e "console.log('${unicode}');"`,
9495
{ maxBuffer: 10 },
9596
common.mustCall((err, stdout, stderr) => {
9697
runChecks(err, { stdout, stderr }, 'stdout', '中文测试\n');
@@ -99,10 +100,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
99100
}
100101

101102
{
102-
const cmd = `"${process.execPath}" -e "console.error('${unicode}');"`;
103-
104-
cp.exec(
105-
cmd,
103+
execNode(
104+
`-e "console.error('${unicode}');"`,
106105
{ maxBuffer: 3 },
107106
common.mustCall((err, stdout, stderr) => {
108107
runChecks(err, { stdout, stderr }, 'stderr', '中文测');
@@ -111,10 +110,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
111110
}
112111

113112
{
114-
const cmd = `"${process.execPath}" -e "console.log('${unicode}');"`;
115-
116-
const child = cp.exec(
117-
cmd,
113+
const child = execNode(
114+
`-e "console.log('${unicode}');"`,
118115
{ encoding: null, maxBuffer: 10 },
119116
common.mustCall((err, stdout, stderr) => {
120117
runChecks(err, { stdout, stderr }, 'stdout', '中文测试\n');
@@ -125,10 +122,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
125122
}
126123

127124
{
128-
const cmd = `"${process.execPath}" -e "console.error('${unicode}');"`;
129-
130-
const child = cp.exec(
131-
cmd,
125+
const child = execNode(
126+
`-e "console.error('${unicode}');"`,
132127
{ encoding: null, maxBuffer: 3 },
133128
common.mustCall((err, stdout, stderr) => {
134129
runChecks(err, { stdout, stderr }, 'stderr', '中文测');
@@ -139,10 +134,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
139134
}
140135

141136
{
142-
const cmd = `"${process.execPath}" -e "console.error('${unicode}');"`;
143-
144-
cp.exec(
145-
cmd,
137+
execNode(
138+
`-e "console.error('${unicode}');"`,
146139
{ encoding: null, maxBuffer: 5 },
147140
common.mustCall((err, stdout, stderr) => {
148141
const buf = Buffer.from(unicode).slice(0, 5);

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

+10-2
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,16 @@ 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+
1018
{
11-
const promise = exec(`${process.execPath} -p 42`);
19+
const promise = execNode('-p 42');
1220

1321
assert(promise.child instanceof child_process.ChildProcess);
1422
promise.then(common.mustCall((obj) => {
@@ -45,7 +53,7 @@ const execFile = promisify(child_process.execFile);
4553
const failingCodeWithStdoutErr =
4654
'console.log(42);console.error(43);process.exit(1)';
4755
{
48-
exec(`${process.execPath} -e "${failingCodeWithStdoutErr}"`)
56+
execNode(`-e "${failingCodeWithStdoutErr}"`)
4957
.catch(common.mustCall((err) => {
5058
assert.strictEqual(err.code, 1);
5159
assert.strictEqual(err.stdout, '42\n');

‎test/parallel/test-child-process-spawn-shell.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ command.on('close', common.mustCall((code, signal) => {
4949
}));
5050

5151
// Verify that the environment is properly inherited
52-
const env = cp.spawn(`"${process.execPath}" -pe process.env.BAZ`, {
53-
env: { ...process.env, BAZ: 'buzz' },
52+
const env = cp.spawn(`"${common.isWindows ? process.execPath : '$NODE'}" -pe process.env.BAZ`, {
53+
env: { ...process.env, BAZ: 'buzz', NODE: process.execPath },
5454
encoding: 'utf8',
5555
shell: true
5656
});

‎test/parallel/test-child-process-spawnsync-shell.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ const command = cp.spawnSync(cmd, { shell: true });
3636
assert.strictEqual(command.stdout.toString().trim(), 'bar');
3737

3838
// Verify that the environment is properly inherited
39-
const env = cp.spawnSync(`"${process.execPath}" -pe process.env.BAZ`, {
40-
env: { ...process.env, BAZ: 'buzz' },
39+
const env = cp.spawnSync(`"${common.isWindows ? process.execPath : '$NODE'}" -pe process.env.BAZ`, {
40+
env: { ...process.env, BAZ: 'buzz', NODE: process.execPath },
4141
shell: true
4242
});
4343

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

+10-1
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,18 @@ 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+
1423

1524
function startPrintHelpTest() {
16-
exec(`${process.execPath} --help`, common.mustSucceed((stdout, stderr) => {
25+
execNode('--help', common.mustSucceed((stdout, stderr) => {
1726
stdOut = stdout;
1827
validateNodePrintHelp();
1928
}));

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

+11-4
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,26 @@ const common = require('../common');
44
const assert = require('assert');
55
const { exec } = require('child_process');
66

7-
const node = process.execPath;
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+
816

917
// Should throw if -c and -e flags are both passed
1018
['-c', '--check'].forEach(function(checkFlag) {
1119
['-e', '--eval'].forEach(function(evalFlag) {
1220
const args = [checkFlag, evalFlag, 'foo'];
13-
const cmd = [node, ...args].join(' ');
14-
exec(cmd, common.mustCall((err, stdout, stderr) => {
21+
execNode(args.join(' '), common.mustCall((err, stdout, stderr) => {
1522
assert.strictEqual(err instanceof Error, true);
1623
assert.strictEqual(err.code, 9);
1724
assert(
1825
stderr.startsWith(
19-
`${node}: either --check or --eval can be used, not both`
26+
`${process.execPath}: either --check or --eval can be used, not both`
2027
)
2128
);
2229
}));

‎test/parallel/test-http-chunk-problem.js

+11-4
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,21 @@ const filename = tmpdir.resolve('big');
4343
let server;
4444

4545
function executeRequest(cb) {
46-
cp.exec([`"${process.execPath}"`,
47-
`"${__filename}"`,
46+
// The execPath might contain chars that should be escaped in a shell context.
47+
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
48+
// Windows, so we can simply pass the path.
49+
const node = `"${common.isWindows ? process.execPath : '$NODE'}"`;
50+
const file = `"${common.isWindows ? __filename : '$FILE'}"`;
51+
const env = common.isWindows ? process.env : { ...process.env, NODE: process.execPath, FILE: __filename };
52+
cp.exec([node,
53+
file,
4854
'request',
4955
server.address().port,
5056
'|',
51-
`"${process.execPath}"`,
52-
`"${__filename}"`,
57+
node,
58+
file,
5359
'shasum' ].join(' '),
60+
{ env },
5461
(err, stdout, stderr) => {
5562
if (stderr.trim() !== '') {
5663
console.log(stderr);

‎test/parallel/test-npm-install.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,15 @@ fs.writeFileSync(pkgPath, pkgContent);
4040

4141
const env = { ...process.env,
4242
PATH: path.dirname(process.execPath),
43+
NODE: process.execPath,
44+
NPM: npmPath,
4345
NPM_CONFIG_PREFIX: path.join(npmSandbox, 'npm-prefix'),
4446
NPM_CONFIG_TMP: path.join(npmSandbox, 'npm-tmp'),
4547
NPM_CONFIG_AUDIT: false,
4648
NPM_CONFIG_UPDATE_NOTIFIER: false,
4749
HOME: homeDir };
4850

49-
exec(`${process.execPath} ${npmPath} install`, {
51+
exec(`"${common.isWindows ? process.execPath : '$NODE'}" "${common.isWindows ? npmPath : '$NPM'}" install`, {
5052
cwd: installDir,
5153
env: env
5254
}, common.mustCall(handleExit));

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,20 @@ 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+
1826
// When a permission is set by cli, the process shouldn't be able
1927
// to spawn unless --allow-child-process is sent
2028
{
2129
// doesNotThrow
2230
childProcess.spawnSync(process.execPath, ['--version']);
23-
childProcess.execSync(process.execPath, ['--version']);
31+
execNode('--version');
2432
childProcess.fork(__filename, ['child']);
2533
childProcess.execFileSync(process.execPath, ['--version']);
2634
}

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

+12-4
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ 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) => [
22+
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
23+
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
24+
];
25+
1826
// When a permission is set by cli, the process shouldn't be able
1927
// to spawn
2028
{
@@ -31,13 +39,13 @@ if (process.argv[2] === 'child') {
3139
permission: 'ChildProcess',
3240
}));
3341
assert.throws(() => {
34-
childProcess.exec(process.execPath, ['--version']);
42+
childProcess.exec(...execNode('--version'));
3543
}, common.expectsError({
3644
code: 'ERR_ACCESS_DENIED',
3745
permission: 'ChildProcess',
3846
}));
3947
assert.throws(() => {
40-
childProcess.execSync(process.execPath, ['--version']);
48+
childProcess.execSync(...execNode('--version'));
4149
}, common.expectsError({
4250
code: 'ERR_ACCESS_DENIED',
4351
permission: 'ChildProcess',
@@ -49,13 +57,13 @@ if (process.argv[2] === 'child') {
4957
permission: 'ChildProcess',
5058
}));
5159
assert.throws(() => {
52-
childProcess.execFile(process.execPath, ['--version']);
60+
childProcess.execFile(...execNode('--version'));
5361
}, common.expectsError({
5462
code: 'ERR_ACCESS_DENIED',
5563
permission: 'ChildProcess',
5664
}));
5765
assert.throws(() => {
58-
childProcess.execFileSync(process.execPath, ['--version']);
66+
childProcess.execFileSync(...execNode('--version'));
5967
}, common.expectsError({
6068
code: 'ERR_ACCESS_DENIED',
6169
permission: 'ChildProcess',

‎test/parallel/test-pipe-head.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ const assert = require('assert');
55

66
const exec = require('child_process').exec;
77

8-
const nodePath = process.argv[0];
98
const script = fixtures.path('print-10-lines.js');
109

11-
const cmd = `"${nodePath}" "${script}" | head -2`;
10+
const cmd = `"${common.isWindows ? process.execPath : '$NODE'}" "${common.isWindows ? script : '$FILE'}" | head -2`;
1211

13-
exec(cmd, common.mustSucceed((stdout, stderr) => {
12+
exec(cmd, {
13+
env: common.isWindows ? process.env : { ...process.env, NODE: process.execPath, FILE: script },
14+
}, common.mustSucceed((stdout, stderr) => {
1415
const lines = stdout.split('\n');
1516
assert.strictEqual(lines.length, 3);
1617
}));

‎test/parallel/test-stdin-from-file-spawn.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,13 @@ setTimeout(() => {
3939
}, 100);
4040
`);
4141

42-
execSync(`${process.argv[0]} ${tmpJsFile} < ${tmpCmdFile}`);
42+
// The execPath might contain chars that should be escaped in a shell context.
43+
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
44+
// Windows, so we can simply pass the path.
45+
execSync(
46+
`"${common.isWindows ? process.execPath : '$NODE'}" "${
47+
common.isWindows ? tmpJsFile : '$FILE'}" < "${common.isWindows ? tmpCmdFile : '$CMD_FILE'}"`,
48+
common.isWindows ? undefined : {
49+
env: { ...process.env, NODE: process.execPath, FILE: tmpJsFile, CMD_FILE: tmpCmdFile },
50+
},
51+
);

0 commit comments

Comments
 (0)