Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 65fbe94

Browse filesBrowse files
aduh95targos
authored andcommitted
test: add escapePOSIXShell util
PR-URL: #55125 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: LiviaMedeiros <livia@cirno.name> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent c308862 commit 65fbe94
Copy full SHA for 65fbe94

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Dismiss banner
Expand file treeCollapse file tree

46 files changed

+275
-346
lines changed
Open diff view settings
Collapse file

‎test/abort/test-abort-fatal-error.js‎

Copy file name to clipboardExpand all lines: test/abort/test-abort-fatal-error.js
+6-5Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,17 @@ if (common.isWindows)
2727
const assert = require('assert');
2828
const exec = require('child_process').exec;
2929

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

34-
exec(cmdline, function(err, stdout, stderr) {
35+
exec(...cmdline, common.mustCall((err, stdout, stderr) => {
3536
if (!err) {
3637
console.log(stdout);
3738
console.log(stderr);
3839
assert(false, 'this test should fail');
3940
}
4041

4142
assert(common.nodeProcessAborted(err.code, err.signal));
42-
});
43+
}));
Collapse file

‎test/async-hooks/test-callback-error.js‎

Copy file name to clipboardExpand all lines: test/async-hooks/test-callback-error.js
+4-2Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,12 +62,14 @@ assert.ok(!arg);
6262
let program = process.execPath;
6363
let args = [
6464
'--abort-on-uncaught-exception', __filename, 'test_callback_abort' ];
65-
const options = { encoding: 'utf8' };
65+
let options = {};
6666
if (!common.isWindows) {
67-
program = `ulimit -c 0 && exec ${program} ${args.join(' ')}`;
67+
[program, options] = common.escapePOSIXShell`ulimit -c 0 && exec "${program}" ${args[0]} "${args[1]}" ${args[2]}`;
6868
args = [];
6969
options.shell = true;
7070
}
71+
72+
options.encoding = 'utf8';
7173
const child = spawnSync(program, args, options);
7274
if (common.isWindows) {
7375
assert.strictEqual(child.status, 134);
Collapse file

‎test/common/README.md‎

Copy file name to clipboardExpand all lines: test/common/README.md
+27Lines changed: 27 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,33 @@ Creates a 10 MiB file of all null characters.
112112

113113
Indicates if there is more than 1gb of total memory.
114114

115+
### ``escapePOSIXShell`shell command` ``
116+
117+
Escapes values in a string template literal to pass them as env variable. On Windows, this function
118+
does not escape anything (which is fine for most paths, as `"` is not a valid
119+
char in a path on Windows), so for tests that must pass on Windows, you should
120+
use it only to escape paths, inside double quotes.
121+
This function is meant to be used for tagged template strings.
122+
123+
```js
124+
const { escapePOSIXShell } = require('../common');
125+
const fixtures = require('../common/fixtures');
126+
const { execSync } = require('node:child_process');
127+
const origin = fixtures.path('origin');
128+
const destination = fixtures.path('destination');
129+
130+
execSync(...escapePOSIXShell`cp "${origin}" "${destination}"`);
131+
132+
// When you need to specify specific options, and/or additional env variables:
133+
const [cmd, opts] = escapePOSIXShell`cp "${origin}" "${destination}"`;
134+
console.log(typeof cmd === 'string'); // true
135+
console.log(opts === undefined || typeof opts.env === 'object'); // true
136+
execSync(cmd, { ...opts, stdio: 'ignore' });
137+
execSync(cmd, { stdio: 'ignore', env: { ...opts?.env, KEY: 'value' } });
138+
```
139+
140+
When possible, avoid using a shell; that way, there's no need to escape values.
141+
115142
### `expectsError(validator[, exact])`
116143

117144
* `validator` [\<Object>][<Object>] | [\<RegExp>][<RegExp>] |
Collapse file

‎test/common/index.js‎

Copy file name to clipboardExpand all lines: test/common/index.js
+30-5Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,15 +249,13 @@ const PIPE = (() => {
249249
// `$node --abort-on-uncaught-exception $file child`
250250
// the process aborts.
251251
function childShouldThrowAndAbort() {
252-
let testCmd = '';
252+
const escapedArgs = escapePOSIXShell`"${process.argv[0]}" --abort-on-uncaught-exception "${process.argv[1]}" child`;
253253
if (!isWindows) {
254254
// Do not create core files, as it can take a lot of disk space on
255255
// continuous testing and developers' machines
256-
testCmd += 'ulimit -c 0 && ';
256+
escapedArgs[0] = 'ulimit -c 0 && ' + escapedArgs[0];
257257
}
258-
testCmd += `"${process.argv[0]}" --abort-on-uncaught-exception `;
259-
testCmd += `"${process.argv[1]}" child`;
260-
const child = exec(testCmd);
258+
const child = exec(...escapedArgs);
261259
child.on('exit', function onExit(exitCode, signal) {
262260
const errMsg = 'Test should have aborted ' +
263261
`but instead exited with exit code ${exitCode}` +
@@ -888,6 +886,32 @@ function spawnPromisified(...args) {
888886
});
889887
}
890888

889+
/**
890+
* Escape values in a string template literal. On Windows, this function
891+
* does not escape anything (which is fine for paths, as `"` is not a valid char
892+
* in a path on Windows), so you should use it only to escape paths – or other
893+
* values on tests which are skipped on Windows.
894+
* This function is meant to be used for tagged template strings.
895+
* @returns {[string, object | undefined]} An array that can be passed as
896+
* arguments to `exec` or `execSync`.
897+
*/
898+
function escapePOSIXShell(cmdParts, ...args) {
899+
if (common.isWindows) {
900+
// On Windows, paths cannot contain `"`, so we can return the string unchanged.
901+
return [String.raw({ raw: cmdParts }, ...args)];
902+
}
903+
// On POSIX shells, we can pass values via the env, as there's a standard way for referencing a variable.
904+
const env = { ...process.env };
905+
let cmd = cmdParts[0];
906+
for (let i = 0; i < args.length; i++) {
907+
const envVarName = `ESCAPED_${i}`;
908+
env[envVarName] = args[i];
909+
cmd += '${' + envVarName + '}' + cmdParts[i + 1];
910+
}
911+
912+
return [cmd, { env }];
913+
};
914+
891915
function getPrintedStackTrace(stderr) {
892916
const lines = stderr.split('\n');
893917

@@ -951,6 +975,7 @@ const common = {
951975
childShouldThrowAndAbort,
952976
createZeroFilledFile,
953977
defaultAutoSelectFamilyAttemptTimeout,
978+
escapePOSIXShell,
954979
expectsError,
955980
expectRequiredModule,
956981
expectWarning,
Collapse file

‎test/common/index.mjs‎

Copy file name to clipboardExpand all lines: test/common/index.mjs
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const {
1111
childShouldThrowAndAbort,
1212
createZeroFilledFile,
1313
enoughTestMem,
14+
escapePOSIXShell,
1415
expectsError,
1516
expectWarning,
1617
getArrayBufferViews,
@@ -64,6 +65,7 @@ export {
6465
createRequire,
6566
createZeroFilledFile,
6667
enoughTestMem,
68+
escapePOSIXShell,
6769
expectsError,
6870
expectWarning,
6971
getArrayBufferViews,
Collapse file

‎test/parallel/test-child-process-bad-stdio.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-bad-stdio.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ ChildProcess.prototype.spawn = function() {
2727
};
2828

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

3233
return cp.exec(cmd, options, common.mustCall(callback));
3334
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-exec-encoding.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ if (process.argv[2] === 'child') {
1313
const expectedStdout = `${stdoutData}\n`;
1414
const expectedStderr = `${stderrData}\n`;
1515
function run(options, callback) {
16-
const cmd = `"${process.execPath}" "${__filename}" child`;
16+
const [cmd, opts] = common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`;
17+
options = { ...options, env: { ...opts?.env, ...options.env } };
1718

1819
cp.exec(cmd, options, common.mustSucceed((stdout, stderr) => {
1920
callback(stdout, stderr);
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-exec-maxbuf.js
+3-2Lines 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
};
Collapse file

‎test/parallel/test-child-process-exec-std-encoding.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-exec-std-encoding.js
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ if (process.argv[2] === 'child') {
1212
console.log(stdoutData);
1313
console.error(stderrData);
1414
} else {
15-
const cmd = `"${process.execPath}" "${__filename}" child`;
16-
const child = cp.exec(cmd, common.mustSucceed((stdout, stderr) => {
15+
const child = cp.exec(...common.escapePOSIXShell`"${process.execPath}" "${__filename}" child`, common.mustSucceed((stdout, stderr) => {
1716
assert.strictEqual(stdout, expectedStdout);
1817
assert.strictEqual(stderr, expectedStderr);
1918
}));
Collapse file

‎test/parallel/test-child-process-exec-timeout-expire.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-child-process-exec-timeout-expire.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ if (process.argv[2] === 'child') {
1818
return;
1919
}
2020

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

2323
cp.exec(cmd, {
24+
...opts,
2425
timeout: kExpiringParentTimer,
2526
}, common.mustCall((err, stdout, stderr) => {
2627
console.log('[stdout]', stdout.trim());

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.