Skip to content

Commit

Permalink
test: do not assume process.execPath contains no spaces
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
aduh95 authored and targos committed Oct 4, 2024
1 parent c56e324 commit a50dd21
Show file tree
Hide file tree
Showing 16 changed files with 151 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const invalidArgTypeError = {
};

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

{
Expand Down
71 changes: 32 additions & 39 deletions test/parallel/test-child-process-exec-maxbuf.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,25 @@ function runChecks(err, stdio, streamName, expected) {
assert.deepStrictEqual(stdio[streamName], expected);
}

// The execPath might contain chars that should be escaped in a shell context.
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
// Windows, so we can simply pass the path.
const execNode = (args, optionsOrCallback, callback) => {
let options = optionsOrCallback;
if (typeof optionsOrCallback === 'function') {
options = undefined;
callback = optionsOrCallback;
}
return cp.exec(
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
common.isWindows ? options : { ...options, env: { ...process.env, NODE: process.execPath } },
callback,
);
};

// default value
{
const cmd =
`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024))"`;

cp.exec(cmd, common.mustCall((err) => {
execNode(`-e "console.log('a'.repeat(1024 * 1024))"`, common.mustCall((err) => {
assert(err instanceof RangeError);
assert.strictEqual(err.message, 'stdout maxBuffer length exceeded');
assert.strictEqual(err.code, 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER');
Expand All @@ -24,20 +37,16 @@ function runChecks(err, stdio, streamName, expected) {

// default value
{
const cmd =
`${process.execPath} -e "console.log('a'.repeat(1024 * 1024 - 1))"`;

cp.exec(cmd, common.mustSucceed((stdout, stderr) => {
execNode(`-e "console.log('a'.repeat(1024 * 1024 - 1))"`, common.mustSucceed((stdout, stderr) => {
assert.strictEqual(stdout.trim(), 'a'.repeat(1024 * 1024 - 1));
assert.strictEqual(stderr, '');
}));
}

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

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

// default value
{
const cmd =
`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024))"`;

cp.exec(
cmd,
execNode(
`-e "console.log('a'.repeat(1024 * 1024))"`,
common.mustCall((err, stdout, stderr) => {
runChecks(
err,
Expand All @@ -75,10 +81,7 @@ function runChecks(err, stdio, streamName, expected) {

// default value
{
const cmd =
`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024 - 1))"`;

cp.exec(cmd, common.mustSucceed((stdout, stderr) => {
execNode(`-e "console.log('a'.repeat(1024 * 1024 - 1))"`, common.mustSucceed((stdout, stderr) => {
assert.strictEqual(stdout.trim(), 'a'.repeat(1024 * 1024 - 1));
assert.strictEqual(stderr, '');
}));
Expand All @@ -87,10 +90,8 @@ function runChecks(err, stdio, streamName, expected) {
const unicode = '中文测试'; // length = 4, byte length = 12

{
const cmd = `"${process.execPath}" -e "console.log('${unicode}');"`;

cp.exec(
cmd,
execNode(
`-e "console.log('${unicode}');"`,
{ maxBuffer: 10 },
common.mustCall((err, stdout, stderr) => {
runChecks(err, { stdout, stderr }, 'stdout', '中文测试\n');
Expand All @@ -99,10 +100,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
}

{
const cmd = `"${process.execPath}" -e "console.error('${unicode}');"`;

cp.exec(
cmd,
execNode(
`-e "console.error('${unicode}');"`,
{ maxBuffer: 3 },
common.mustCall((err, stdout, stderr) => {
runChecks(err, { stdout, stderr }, 'stderr', '中文测');
Expand All @@ -111,10 +110,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
}

{
const cmd = `"${process.execPath}" -e "console.log('${unicode}');"`;

const child = cp.exec(
cmd,
const child = execNode(
`-e "console.log('${unicode}');"`,
{ encoding: null, maxBuffer: 10 },
common.mustCall((err, stdout, stderr) => {
runChecks(err, { stdout, stderr }, 'stdout', '中文测试\n');
Expand All @@ -125,10 +122,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
}

{
const cmd = `"${process.execPath}" -e "console.error('${unicode}');"`;

const child = cp.exec(
cmd,
const child = execNode(
`-e "console.error('${unicode}');"`,
{ encoding: null, maxBuffer: 3 },
common.mustCall((err, stdout, stderr) => {
runChecks(err, { stdout, stderr }, 'stderr', '中文测');
Expand All @@ -139,10 +134,8 @@ const unicode = '中文测试'; // length = 4, byte length = 12
}

{
const cmd = `"${process.execPath}" -e "console.error('${unicode}');"`;

cp.exec(
cmd,
execNode(
`-e "console.error('${unicode}');"`,
{ encoding: null, maxBuffer: 5 },
common.mustCall((err, stdout, stderr) => {
const buf = Buffer.from(unicode).slice(0, 5);
Expand Down
12 changes: 10 additions & 2 deletions test/parallel/test-child-process-promisified.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,16 @@ const { promisify } = require('util');
const exec = promisify(child_process.exec);
const execFile = promisify(child_process.execFile);

// The execPath might contain chars that should be escaped in a shell context.
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
// Windows, so we can simply pass the path.
const execNode = (args) => exec(
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
);

{
const promise = exec(`${process.execPath} -p 42`);
const promise = execNode('-p 42');

assert(promise.child instanceof child_process.ChildProcess);
promise.then(common.mustCall((obj) => {
Expand Down Expand Up @@ -45,7 +53,7 @@ const execFile = promisify(child_process.execFile);
const failingCodeWithStdoutErr =
'console.log(42);console.error(43);process.exit(1)';
{
exec(`${process.execPath} -e "${failingCodeWithStdoutErr}"`)
execNode(`-e "${failingCodeWithStdoutErr}"`)
.catch(common.mustCall((err) => {
assert.strictEqual(err.code, 1);
assert.strictEqual(err.stdout, '42\n');
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-child-process-spawn-shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ command.on('close', common.mustCall((code, signal) => {
}));

// Verify that the environment is properly inherited
const env = cp.spawn(`"${process.execPath}" -pe process.env.BAZ`, {
env: { ...process.env, BAZ: 'buzz' },
const env = cp.spawn(`"${common.isWindows ? process.execPath : '$NODE'}" -pe process.env.BAZ`, {
env: { ...process.env, BAZ: 'buzz', NODE: process.execPath },
encoding: 'utf8',
shell: true
});
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-child-process-spawnsync-shell.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ const command = cp.spawnSync(cmd, { shell: true });
assert.strictEqual(command.stdout.toString().trim(), 'bar');

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

Expand Down
11 changes: 10 additions & 1 deletion test/parallel/test-cli-node-print-help.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,18 @@ const { exec, spawn } = require('child_process');
const { once } = require('events');
let stdOut;

// The execPath might contain chars that should be escaped in a shell context.
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
// Windows, so we can simply pass the path.
const execNode = (args, callback) => exec(
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
callback,
);


function startPrintHelpTest() {
exec(`${process.execPath} --help`, common.mustSucceed((stdout, stderr) => {
execNode('--help', common.mustSucceed((stdout, stderr) => {
stdOut = stdout;
validateNodePrintHelp();
}));
Expand Down
15 changes: 11 additions & 4 deletions test/parallel/test-cli-syntax-eval.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,26 @@ const common = require('../common');
const assert = require('assert');
const { exec } = require('child_process');

const node = process.execPath;
// The execPath might contain chars that should be escaped in a shell context.
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
// Windows, so we can simply pass the path.
const execNode = (args, callback) => exec(
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
callback,
);


// Should throw if -c and -e flags are both passed
['-c', '--check'].forEach(function(checkFlag) {
['-e', '--eval'].forEach(function(evalFlag) {
const args = [checkFlag, evalFlag, 'foo'];
const cmd = [node, ...args].join(' ');
exec(cmd, common.mustCall((err, stdout, stderr) => {
execNode(args.join(' '), common.mustCall((err, stdout, stderr) => {
assert.strictEqual(err instanceof Error, true);
assert.strictEqual(err.code, 9);
assert(
stderr.startsWith(
`${node}: either --check or --eval can be used, not both`
`${process.execPath}: either --check or --eval can be used, not both`
)
);
}));
Expand Down
15 changes: 11 additions & 4 deletions test/parallel/test-http-chunk-problem.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,21 @@ const filename = tmpdir.resolve('big');
let server;

function executeRequest(cb) {
cp.exec([`"${process.execPath}"`,
`"${__filename}"`,
// The execPath might contain chars that should be escaped in a shell context.
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
// Windows, so we can simply pass the path.
const node = `"${common.isWindows ? process.execPath : '$NODE'}"`;
const file = `"${common.isWindows ? __filename : '$FILE'}"`;
const env = common.isWindows ? process.env : { ...process.env, NODE: process.execPath, FILE: __filename };
cp.exec([node,
file,
'request',
server.address().port,
'|',
`"${process.execPath}"`,
`"${__filename}"`,
node,
file,
'shasum' ].join(' '),
{ env },
(err, stdout, stderr) => {
if (stderr.trim() !== '') {
console.log(stderr);
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-npm-install.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ fs.writeFileSync(pkgPath, pkgContent);

const env = { ...process.env,
PATH: path.dirname(process.execPath),
NODE: process.execPath,
NPM: npmPath,
NPM_CONFIG_PREFIX: path.join(npmSandbox, 'npm-prefix'),
NPM_CONFIG_TMP: path.join(npmSandbox, 'npm-tmp'),
NPM_CONFIG_AUDIT: false,
NPM_CONFIG_UPDATE_NOTIFIER: false,
HOME: homeDir };

exec(`${process.execPath} ${npmPath} install`, {
exec(`"${common.isWindows ? process.execPath : '$NODE'}" "${common.isWindows ? npmPath : '$NPM'}" install`, {
cwd: installDir,
env: env
}, common.mustCall(handleExit));
Expand Down
10 changes: 9 additions & 1 deletion test/parallel/test-permission-allow-child-process-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,20 @@ if (process.argv[2] === 'child') {
assert.ok(process.permission.has('child'));
}

// The execPath might contain chars that should be escaped in a shell context.
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
// Windows, so we can simply pass the path.
const execNode = (args) => childProcess.execSync(
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
);

// When a permission is set by cli, the process shouldn't be able
// to spawn unless --allow-child-process is sent
{
// doesNotThrow
childProcess.spawnSync(process.execPath, ['--version']);
childProcess.execSync(process.execPath, ['--version']);
execNode('--version');
childProcess.fork(__filename, ['child']);
childProcess.execFileSync(process.execPath, ['--version']);
}
16 changes: 12 additions & 4 deletions test/parallel/test-permission-child-process-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ if (process.argv[2] === 'child') {
assert.ok(!process.permission.has('child'));
}

// The execPath might contain chars that should be escaped in a shell context.
// On non-Windows, we can pass the path via the env; `"` is not a valid char on
// Windows, so we can simply pass the path.
const execNode = (args) => [
`"${common.isWindows ? process.execPath : '$NODE'}" ${args}`,
common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath } },
];

// When a permission is set by cli, the process shouldn't be able
// to spawn
{
Expand All @@ -31,13 +39,13 @@ if (process.argv[2] === 'child') {
permission: 'ChildProcess',
}));
assert.throws(() => {
childProcess.exec(process.execPath, ['--version']);
childProcess.exec(...execNode('--version'));
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'ChildProcess',
}));
assert.throws(() => {
childProcess.execSync(process.execPath, ['--version']);
childProcess.execSync(...execNode('--version'));
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'ChildProcess',
Expand All @@ -49,13 +57,13 @@ if (process.argv[2] === 'child') {
permission: 'ChildProcess',
}));
assert.throws(() => {
childProcess.execFile(process.execPath, ['--version']);
childProcess.execFile(...execNode('--version'));
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'ChildProcess',
}));
assert.throws(() => {
childProcess.execFileSync(process.execPath, ['--version']);
childProcess.execFileSync(...execNode('--version'));
}, common.expectsError({
code: 'ERR_ACCESS_DENIED',
permission: 'ChildProcess',
Expand Down
7 changes: 4 additions & 3 deletions test/parallel/test-pipe-head.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ const assert = require('assert');

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

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

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

exec(cmd, common.mustSucceed((stdout, stderr) => {
exec(cmd, {
env: common.isWindows ? process.env : { ...process.env, NODE: process.execPath, FILE: script },
}, common.mustSucceed((stdout, stderr) => {
const lines = stdout.split('\n');
assert.strictEqual(lines.length, 3);
}));
11 changes: 10 additions & 1 deletion test/parallel/test-stdin-from-file-spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,13 @@ setTimeout(() => {
}, 100);
`);

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

0 comments on commit a50dd21

Please sign in to comment.