Skip to content

Commit

Permalink
test: add escapePOSIXShell util
Browse files Browse the repository at this point in the history
  • Loading branch information
aduh95 committed Sep 25, 2024
1 parent 0f7bdcc commit b931733
Show file tree
Hide file tree
Showing 47 changed files with 237 additions and 334 deletions.
11 changes: 6 additions & 5 deletions test/abort/test-abort-fatal-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,17 @@ if (common.isWindows)
const assert = require('assert');
const exec = require('child_process').exec;

let cmdline = `ulimit -c 0; ${process.execPath}`;
cmdline += ' --max-old-space-size=16 --max-semi-space-size=4';
cmdline += ' -e "a = []; for (i = 0; i < 1e9; i++) { a.push({}) }"';
const cmdline =
common.escapePOSIXShell`ulimit -c 0; "${
process.execPath
}" --max-old-space-size=16 --max-semi-space-size=4 -e "a = []; for (i = 0; i < 1e9; i++) { a.push({}) }"`;

exec(cmdline, function(err, stdout, stderr) {
exec(...cmdline, common.mustCall((err, stdout, stderr) => {
if (!err) {
console.log(stdout);
console.log(stderr);
assert(false, 'this test should fail');
}

assert(common.nodeProcessAborted(err.code, err.signal));
});
}));
5 changes: 3 additions & 2 deletions test/async-hooks/test-callback-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ assert.ok(!arg);
let program = process.execPath;
let args = [
'--abort-on-uncaught-exception', __filename, 'test_callback_abort' ];
const options = { encoding: 'utf8' };
let options = { encoding: 'utf8' };
if (!common.isWindows) {
program = `ulimit -c 0 && exec ${program} ${args.join(' ')}`;
[program, options] = common.escapePOSIXShell`ulimit -c 0 && exec "${program}" ${args[0]} "${args[1]}" ${args[2]}`;
args = [];
options.shell = true;
options.encoding = 'utf8';
}
const child = spawnSync(program, args, options);
if (common.isWindows) {
Expand Down
26 changes: 26 additions & 0 deletions test/common/escapePOSIXShell.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';

/**

Check failure on line 3 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected multiple line comments instead of a block comment
* Escape quoted values in a string template literal. On Windows, this function

Check warning on line 4 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected JSDoc block to be aligned
* does not escape anything (which is fine for paths, as `"` is not a valid char
* in a path on Windows), so you should use it only to escape paths – or other
* values on tests which are skipped on Windows.
* @returns {[string, object | undefined]} An array that can be passed as
* arguments to `exec` or `execSync`.
*/
module.exports = function escapePOSIXShell(cmdParts, ...args) {
if (common.isWindows) {

Check failure on line 12 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 2 spaces but found 1

Check failure on line 12 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

'common' is not defined
// On Windows, paths cannot contain `"`, so we can return the string unchanged.

Check failure on line 13 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 4 spaces but found 3
return [String.raw({ raw: cmdParts }, ...args)];

Check failure on line 14 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 4 spaces but found 3
}

Check failure on line 15 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 2 spaces but found 1
// On POSIX shells, we can pass values via the env, as there's a standard way for referencing a variable.

Check failure on line 16 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 2 spaces but found 1
const env = { ...process.env };

Check failure on line 17 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 2 spaces but found 1
let cmd = cmdParts[0];

Check failure on line 18 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 2 spaces but found 1
for (let i = 0; i < args.length; i++) {

Check failure on line 19 in test/common/escapePOSIXShell.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Expected indentation of 2 spaces but found 1
const envVarName = `ESCAPED_${i}`;
env[envVarName] = args[i];
cmd += '${' + envVarName + '}' + cmdParts[i + 1];
}

return [cmd, { env }];
};
10 changes: 5 additions & 5 deletions test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const { inspect, getCallSite } = require('util');
const { isMainThread } = require('worker_threads');
const { isModuleNamespaceObject } = require('util/types');

const escapePOSIXShell = require('./escapePOSIXShell');
const tmpdir = require('./tmpdir');
const bits = ['arm64', 'loong64', 'mips', 'mipsel', 'ppc64', 'riscv64', 's390x', 'x64']
.includes(process.arch) ? 64 : 32;
Expand Down Expand Up @@ -249,15 +250,13 @@ const PIPE = (() => {
// `$node --abort-on-uncaught-exception $file child`
// the process aborts.
function childShouldThrowAndAbort() {
let testCmd = '';
const escapedArgs = escapePOSIXShell`"${process.argv[0]}" --abort-on-uncaught-exception "${process.argv[1]}" child`;
if (!isWindows) {
// Do not create core files, as it can take a lot of disk space on
// continuous testing and developers' machines
testCmd += 'ulimit -c 0 && ';
escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0];
}
testCmd += `"${process.argv[0]}" --abort-on-uncaught-exception `;
testCmd += `"${process.argv[1]}" child`;
const child = exec(testCmd);
const child = exec(...escapedArgs);
child.on('exit', function onExit(exitCode, signal) {
const errMsg = 'Test should have aborted ' +
`but instead exited with exit code ${exitCode}` +
Expand Down Expand Up @@ -951,6 +950,7 @@ const common = {
childShouldThrowAndAbort,
createZeroFilledFile,
defaultAutoSelectFamilyAttemptTimeout,
escapePOSIXShell,
expectsError,
expectRequiredModule,
expectWarning,
Expand Down
2 changes: 2 additions & 0 deletions test/common/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const {
childShouldThrowAndAbort,
createZeroFilledFile,
enoughTestMem,
escapePOSIXShell,
expectsError,
expectWarning,
getArrayBufferViews,
Expand Down Expand Up @@ -64,6 +65,7 @@ export {
createRequire,
createZeroFilledFile,
enoughTestMem,
escapePOSIXShell,
expectsError,
expectWarning,
getArrayBufferViews,
Expand Down
4 changes: 3 additions & 1 deletion test/fixtures/snapshot/child-process-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ const {
setDeserializeMainFunction,
isBuildingSnapshot
} = require('v8').startupSnapshot;
const escapePOSIXShell = require('../../common/escapePOSIXShell');

function spawn() {
const { spawnSync, execFileSync, execSync } = require('child_process');
spawnSync(process.execPath, [ __filename, 'spawnSync' ], { stdio: 'inherit' });
execSync(`"${process.execPath}" "${__filename}" "execSync"`, { stdio: 'inherit' });
const [cmd, opts] = escapePOSIXShell`"${process.execPath}" "${__filename}" "execSync"`;
execSync(cmd, { ...opts, stdio: 'inherit'});
execFileSync(process.execPath, [ __filename, 'execFileSync' ], { stdio: 'inherit' });
}

Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-bad-stdio.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ ChildProcess.prototype.spawn = function() {
};

function createChild(options, callback) {
const cmd = `"${process.execPath}" "${__filename}" child`;
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;
options = { ...options, env: { ...opts.env, ...options.env } };

return cp.exec(cmd, options, common.mustCall(callback));
}
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-exec-encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ if (process.argv[2] === 'child') {
const expectedStdout = `${stdoutData}\n`;
const expectedStderr = `${stderrData}\n`;
function run(options, callback) {
const cmd = `"${process.execPath}" "${__filename}" child`;
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;
options = { ...options, env: { ...opts.env, ...options.env } };

cp.exec(cmd, options, common.mustSucceed((stdout, stderr) => {
callback(stdout, stderr);
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-child-process-exec-maxbuf.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ function runChecks(err, stdio, streamName, expected) {
// 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) => {
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" `;
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 } },
cmd + args,
{ ...opts, ...options },
callback,
);
};
Expand Down
3 changes: 1 addition & 2 deletions test/parallel/test-child-process-exec-std-encoding.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ if (process.argv[2] === 'child') {
console.log(stdoutData);
console.error(stderrData);
} else {
const cmd = `"${process.execPath}" "${__filename}" child`;
const child = cp.exec(cmd, common.mustSucceed((stdout, stderr) => {
const child = cp.exec(...common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`, common.mustSucceed((stdout, stderr) => {
assert.strictEqual(stdout, expectedStdout);
assert.strictEqual(stderr, expectedStderr);
}));
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-exec-timeout-expire.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ if (process.argv[2] === 'child') {
return;
}

const cmd = `"${process.execPath}" "${__filename}" child`;
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;

cp.exec(cmd, {
...opts,
timeout: kExpiringParentTimer,
}, common.mustCall((err, stdout, stderr) => {
console.log('[stdout]', stdout.trim());
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-child-process-exec-timeout-kill.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ if (process.argv[2] === 'child') {
return;
}

const cmd = `"${process.execPath}" "${__filename}" child`;
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;

// Test with a different kill signal.
cp.exec(cmd, {
...opts,
timeout: kExpiringParentTimer,
killSignal: 'SIGKILL'
}, common.mustCall((err, stdout, stderr) => {
Expand Down
5 changes: 3 additions & 2 deletions test/parallel/test-child-process-exec-timeout-not-expired.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ if (process.argv[2] === 'child') {
return;
}

const cmd = `"${process.execPath}" "${__filename}" child`;
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;

cp.exec(cmd, {
timeout: kTimeoutNotSupposedToExpire
...opts,
timeout: kTimeoutNotSupposedToExpire,
}, common.mustSucceed((stdout, stderr) => {
assert.strictEqual(stdout.trim(), 'child stdout');
assert.strictEqual(stderr.trim(), 'child stderr');
Expand Down
21 changes: 7 additions & 14 deletions test/parallel/test-child-process-execsync-maxbuf.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
require('../common');
const { escapePOSIXShell } = require('../common');

// This test checks that the maxBuffer option for child_process.spawnSync()
// works as expected.
Expand All @@ -10,15 +10,12 @@ const { execSync } = require('child_process');
const msgOut = 'this is stdout';
const msgOutBuf = Buffer.from(`${msgOut}\n`);

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

// Verify that an error is returned if maxBuffer is surpassed.
{
assert.throws(() => {
execSync(`"${process.execPath}" ${args.join(' ')}`, { maxBuffer: 1 });
execSync(cmd, { ...opts, maxBuffer: 1 });
}, (e) => {
assert.ok(e, 'maxBuffer should error');
assert.strictEqual(e.code, 'ENOBUFS');
Expand All @@ -33,8 +30,8 @@ const args = [
// Verify that a maxBuffer size of Infinity works.
{
const ret = execSync(
`"${process.execPath}" ${args.join(' ')}`,
{ maxBuffer: Infinity }
cmd,
{ ...opts, maxBuffer: Infinity },
);

assert.deepStrictEqual(ret, msgOutBuf);
Expand All @@ -43,9 +40,7 @@ const args = [
// Default maxBuffer size is 1024 * 1024.
{
assert.throws(() => {
execSync(
`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024))"`
);
execSync(...escapePOSIXShell`"${process.execPath}" -e "console.log('a'.repeat(1024 * 1024))"`);
}, (e) => {
assert.ok(e, 'maxBuffer should error');
assert.strictEqual(e.code, 'ENOBUFS');
Expand All @@ -56,9 +51,7 @@ const args = [

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

assert.deepStrictEqual(
ret.toString().trim(),
Expand Down
12 changes: 2 additions & 10 deletions test/parallel/test-child-process-promisified.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,8 @@ 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 = execNode('-p 42');
const promise = exec(...common.escapePOSIXShell`"${process.execPath}" -p 42`);

assert(promise.child instanceof child_process.ChildProcess);
promise.then(common.mustCall((obj) => {
Expand Down Expand Up @@ -53,7 +45,7 @@ const execNode = (args) => exec(
const failingCodeWithStdoutErr =
'console.log(42);console.error(43);process.exit(1)';
{
execNode(`-e "${failingCodeWithStdoutErr}"`)
exec(...common.escapePOSIXShell`"${process.execPath}" -e "${failingCodeWithStdoutErr}"`)
.catch(common.mustCall((err) => {
assert.strictEqual(err.code, 1);
assert.strictEqual(err.stdout, '42\n');
Expand Down
Loading

0 comments on commit b931733

Please sign in to comment.