test: add escapePOSIXShell util

PR-URL: https://github.com/nodejs/node/pull/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>

test/common/README.md

@@ -112,6 +112,33 @@ Creates a 10 MiB file of all null characters.
 
 Indicates if there is more than 1gb of total memory.
 
+### ``escapePOSIXShell`shell command` ``
+
+Escapes values in a string template literal to pass them as env variable. On Windows, this function
+does not escape anything (which is fine for most paths, as `"` is not a valid
+char in a path on Windows), so for tests that must pass on Windows, you should
+use it only to escape paths, inside double quotes.
+This function is meant to be used for tagged template strings.
+
+```js
+const { escapePOSIXShell } = require('../common');
+const fixtures = require('../common/fixtures');
+const { execSync } = require('node:child_process');
+const origin = fixtures.path('origin');
+const destination = fixtures.path('destination');
+
+execSync(...escapePOSIXShell`cp "${origin}" "${destination}"`);
+
+// When you need to specify specific options, and/or additional env variables:
+const [cmd, opts] = escapePOSIXShell`cp "${origin}" "${destination}"`;
+console.log(typeof cmd === 'string'); // true
+console.log(opts === undefined || typeof opts.env === 'object'); // true
+execSync(cmd, { ...opts, stdio: 'ignore' });
+execSync(cmd, { stdio: 'ignore', env: { ...opts?.env, KEY: 'value' } });
+```
+
+When possible, avoid using a shell; that way, there's no need to escape values.
+
 ### `expectsError(validator[, exact])`
 
 * `validator` [\<Object>][<Object>] | [\<RegExp>][<RegExp>] |

test/common/index.js

@@ -249,15 +249,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}` +
@@ -888,6 +886,32 @@ function spawnPromisified(...args) {
   });
 }
 
+/**
+ * Escape values in a string template literal. On Windows, this function
+ * 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.
+ * This function is meant to be used for tagged template strings.
+ * @returns {[string, object | undefined]} An array that can be passed as
+ *                                         arguments to `exec` or `execSync`.
+ */
+function escapePOSIXShell(cmdParts, ...args) {
+  if (common.isWindows) {
+    // On Windows, paths cannot contain `"`, so we can return the string unchanged.
+    return [String.raw({ raw: cmdParts }, ...args)];
+  }
+  // On POSIX shells, we can pass values via the env, as there's a standard way for referencing a variable.
+  const env = { ...process.env };
+  let cmd = cmdParts[0];
+  for (let i = 0; i < args.length; i++) {
+    const envVarName = `ESCAPED_${i}`;
+    env[envVarName] = args[i];
+    cmd += '${' + envVarName + '}' + cmdParts[i + 1];
+  }
+
+  return [cmd, { env }];
+};
+
 function getPrintedStackTrace(stderr) {
   const lines = stderr.split('\n');
 
@@ -951,6 +975,7 @@ const common = {
   childShouldThrowAndAbort,
   createZeroFilledFile,
   defaultAutoSelectFamilyAttemptTimeout,
+  escapePOSIXShell,
   expectsError,
   expectRequiredModule,
   expectWarning,

test/common/index.mjs

@@ -11,6 +11,7 @@ const {
   childShouldThrowAndAbort,
   createZeroFilledFile,
   enoughTestMem,
+  escapePOSIXShell,
   expectsError,
   expectWarning,
   getArrayBufferViews,
@@ -64,6 +65,7 @@ export {
   createRequire,
   createZeroFilledFile,
   enoughTestMem,
+  escapePOSIXShell,
   expectsError,
   expectWarning,
   getArrayBufferViews,