From 6a3f64354a34f87755138555e0c7433c725e9b6d Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 20 Sep 2024 14:59:09 +0200 Subject: [PATCH 1/2] 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. --- ...rocess-exec-abortcontroller-promisified.js | 3 +- .../test-child-process-exec-maxbuf.js | 71 +++++++++---------- .../test-child-process-promisified.js | 12 +++- .../test-child-process-spawn-shell.js | 4 +- .../test-child-process-spawnsync-shell.js | 4 +- test/parallel/test-cli-node-print-help.js | 11 ++- test/parallel/test-cli-syntax-eval.js | 15 ++-- test/parallel/test-http-chunk-problem.js | 15 ++-- test/parallel/test-npm-install.js | 4 +- ...test-permission-allow-child-process-cli.js | 10 ++- .../test-permission-child-process-cli.js | 16 +++-- test/parallel/test-pipe-head.js | 7 +- test/parallel/test-stdin-from-file-spawn.js | 11 ++- .../sequential/test-child-process-execsync.js | 17 ++--- .../test-cli-syntax-file-not-found.js | 18 +++-- test/sequential/test-cli-syntax-good.js | 19 ++--- 16 files changed, 149 insertions(+), 88 deletions(-) diff --git a/test/parallel/test-child-process-exec-abortcontroller-promisified.js b/test/parallel/test-child-process-exec-abortcontroller-promisified.js index d87b502ab8e2d7..04e7b70671b94c 100644 --- a/test/parallel/test-child-process-exec-abortcontroller-promisified.js +++ b/test/parallel/test-child-process-exec-abortcontroller-promisified.js @@ -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'; { diff --git a/test/parallel/test-child-process-exec-maxbuf.js b/test/parallel/test-child-process-exec-maxbuf.js index c434c8531d7ebf..f2e29b8a7d0745 100644 --- a/test/parallel/test-child-process-exec-maxbuf.js +++ b/test/parallel/test-child-process-exec-maxbuf.js @@ -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'); @@ -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, ''); })); @@ -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, @@ -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, ''); })); @@ -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'); @@ -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', '中文测'); @@ -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'); @@ -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', '中文测'); @@ -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); diff --git a/test/parallel/test-child-process-promisified.js b/test/parallel/test-child-process-promisified.js index bc623130994a2e..de707ce703f4e2 100644 --- a/test/parallel/test-child-process-promisified.js +++ b/test/parallel/test-child-process-promisified.js @@ -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) => { @@ -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'); diff --git a/test/parallel/test-child-process-spawn-shell.js b/test/parallel/test-child-process-spawn-shell.js index 9b8de0507130d6..a63e92f29d0adb 100644 --- a/test/parallel/test-child-process-spawn-shell.js +++ b/test/parallel/test-child-process-spawn-shell.js @@ -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 }); diff --git a/test/parallel/test-child-process-spawnsync-shell.js b/test/parallel/test-child-process-spawnsync-shell.js index 0dd50cedd123b2..9db8a53366fc47 100644 --- a/test/parallel/test-child-process-spawnsync-shell.js +++ b/test/parallel/test-child-process-spawnsync-shell.js @@ -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 }); diff --git a/test/parallel/test-cli-node-print-help.js b/test/parallel/test-cli-node-print-help.js index 8daf00278f8135..eea05dc1f00885 100644 --- a/test/parallel/test-cli-node-print-help.js +++ b/test/parallel/test-cli-node-print-help.js @@ -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(); })); diff --git a/test/parallel/test-cli-syntax-eval.js b/test/parallel/test-cli-syntax-eval.js index 31fe2d3449d824..b5b8db4547a864 100644 --- a/test/parallel/test-cli-syntax-eval.js +++ b/test/parallel/test-cli-syntax-eval.js @@ -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` ) ); })); diff --git a/test/parallel/test-http-chunk-problem.js b/test/parallel/test-http-chunk-problem.js index a3c354aecde468..3629b7576600e8 100644 --- a/test/parallel/test-http-chunk-problem.js +++ b/test/parallel/test-http-chunk-problem.js @@ -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); diff --git a/test/parallel/test-npm-install.js b/test/parallel/test-npm-install.js index 5adca8001e3133..ec848339b74004 100644 --- a/test/parallel/test-npm-install.js +++ b/test/parallel/test-npm-install.js @@ -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)); diff --git a/test/parallel/test-permission-allow-child-process-cli.js b/test/parallel/test-permission-allow-child-process-cli.js index 6cffc19719350b..1216e29886a443 100644 --- a/test/parallel/test-permission-allow-child-process-cli.js +++ b/test/parallel/test-permission-allow-child-process-cli.js @@ -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']); } diff --git a/test/parallel/test-permission-child-process-cli.js b/test/parallel/test-permission-child-process-cli.js index 3ce473ab498e0e..5a9c6345aaf946 100644 --- a/test/parallel/test-permission-child-process-cli.js +++ b/test/parallel/test-permission-child-process-cli.js @@ -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 { @@ -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', @@ -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', diff --git a/test/parallel/test-pipe-head.js b/test/parallel/test-pipe-head.js index 1e79249c290500..f0b66a9d435808 100644 --- a/test/parallel/test-pipe-head.js +++ b/test/parallel/test-pipe-head.js @@ -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); })); diff --git a/test/parallel/test-stdin-from-file-spawn.js b/test/parallel/test-stdin-from-file-spawn.js index 3830ac124a1d8a..b8e19fdf0689b9 100644 --- a/test/parallel/test-stdin-from-file-spawn.js +++ b/test/parallel/test-stdin-from-file-spawn.js @@ -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 }, + }, +); diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index 75acbc34a902bd..53ee42b21490dd 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -38,8 +38,6 @@ if (common.isWindows) { SLEEP = 10000; } -const execOpts = { encoding: 'utf8', shell: true }; - // Verify that stderr is not accessed when a bad shell is used assert.throws( function() { execSync('exit -1', { shell: 'bad_shell' }); }, @@ -54,8 +52,8 @@ let caught = false; let ret, err; const start = Date.now(); try { - const cmd = `"${process.execPath}" -e "setTimeout(function(){}, ${SLEEP});"`; - ret = execSync(cmd, { timeout: TIMER }); + const cmd = `"${common.isWindows ? process.execPath : '$NODE'}" -e "setTimeout(function(){}, ${SLEEP});"`; + ret = execSync(cmd, { env: { ...process.env, NODE: process.execPath }, timeout: TIMER }); } catch (e) { caught = true; assert.strictEqual(getSystemErrorName(e.errno), 'ETIMEDOUT'); @@ -78,16 +76,17 @@ const msgBuf = Buffer.from(`${msg}\n`); // console.log ends every line with just '\n', even on Windows. -const cmd = `"${process.execPath}" -e "console.log('${msg}');"`; +const cmd = `"${common.isWindows ? process.execPath : '$NODE'}" -e "console.log('${msg}');"`; +const env = common.isWindows ? process.env : { ...process.env, NODE: process.execPath }; { - const ret = execSync(cmd); + const ret = execSync(cmd, common.isWindows ? undefined : { env }); assert.strictEqual(ret.length, msgBuf.length); assert.deepStrictEqual(ret, msgBuf); } { - const ret = execSync(cmd, { encoding: 'utf8' }); + const ret = execSync(cmd, { encoding: 'utf8', env }); assert.strictEqual(ret, `${msg}\n`); } @@ -156,4 +155,6 @@ const args = [ } // Verify the shell option works properly -execFileSync(process.execPath, [], execOpts); +execFileSync(`"${common.isWindows ? process.execPath : '$NODE'}"`, [], { + encoding: 'utf8', shell: true, env +}); diff --git a/test/sequential/test-cli-syntax-file-not-found.js b/test/sequential/test-cli-syntax-file-not-found.js index 61f84d8e71624d..74ab521f1c1186 100644 --- a/test/sequential/test-cli-syntax-file-not-found.js +++ b/test/sequential/test-cli-syntax-file-not-found.js @@ -5,12 +5,18 @@ const assert = require('assert'); const { exec } = require('child_process'); const fixtures = require('../common/fixtures'); -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 = (flag, file) => exec( + `"${common.isWindows ? process.execPath : '$NODE'}" ${flag} "${common.isWindows ? file : '$FILE'}"`, + common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath, FILE: file } }, +); // Test both sets of arguments that check syntax const syntaxArgs = [ - ['-c'], - ['--check'], + '-c', + '--check', ]; const notFoundRE = /^Error: Cannot find module/m; @@ -23,10 +29,8 @@ const notFoundRE = /^Error: Cannot find module/m; file = fixtures.path(file); // Loop each possible option, `-c` or `--check` - syntaxArgs.forEach(function(args) { - const _args = args.concat(file); - const cmd = [node, ..._args].join(' '); - exec(cmd, common.mustCall((err, stdout, stderr) => { + syntaxArgs.forEach(function(flag) { + execNode(flag, file, common.mustCall((err, stdout, stderr) => { // No stdout should be produced assert.strictEqual(stdout, ''); diff --git a/test/sequential/test-cli-syntax-good.js b/test/sequential/test-cli-syntax-good.js index 44051c7a4a3617..3b705c82993ff0 100644 --- a/test/sequential/test-cli-syntax-good.js +++ b/test/sequential/test-cli-syntax-good.js @@ -5,12 +5,18 @@ const assert = require('assert'); const { exec } = require('child_process'); const fixtures = require('../common/fixtures'); -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 = (flag, file) => exec( + `"${common.isWindows ? process.execPath : '$NODE'}" ${flag} "${common.isWindows ? file : '$FILE'}"`, + common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath, FILE: file } }, +); // Test both sets of arguments that check syntax const syntaxArgs = [ - ['-c'], - ['--check'], + '-c', + '--check', ]; // Test good syntax with and without shebang @@ -25,11 +31,8 @@ const syntaxArgs = [ file = fixtures.path(file); // Loop each possible option, `-c` or `--check` - syntaxArgs.forEach(function(args) { - const _args = args.concat(file); - - const cmd = [node, ..._args].join(' '); - exec(cmd, common.mustCall((err, stdout, stderr) => { + syntaxArgs.forEach(function(flag) { + execNode(flag, file, common.mustCall((err, stdout, stderr) => { if (err) { console.log('-- stdout --'); console.log(stdout); From 2ea4fa072621b7ecc795070f812b80bd3bd331ee Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Fri, 20 Sep 2024 16:02:03 +0200 Subject: [PATCH 2/2] fixup! test: do not assume `process.execPath` contains no spaces --- test/sequential/test-cli-syntax-file-not-found.js | 3 ++- test/sequential/test-cli-syntax-good.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/test/sequential/test-cli-syntax-file-not-found.js b/test/sequential/test-cli-syntax-file-not-found.js index 74ab521f1c1186..189d6b8d58f349 100644 --- a/test/sequential/test-cli-syntax-file-not-found.js +++ b/test/sequential/test-cli-syntax-file-not-found.js @@ -8,9 +8,10 @@ const fixtures = require('../common/fixtures'); // 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 = (flag, file) => exec( +const execNode = (flag, file, callback) => exec( `"${common.isWindows ? process.execPath : '$NODE'}" ${flag} "${common.isWindows ? file : '$FILE'}"`, common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath, FILE: file } }, + callback, ); // Test both sets of arguments that check syntax diff --git a/test/sequential/test-cli-syntax-good.js b/test/sequential/test-cli-syntax-good.js index 3b705c82993ff0..24c68781876556 100644 --- a/test/sequential/test-cli-syntax-good.js +++ b/test/sequential/test-cli-syntax-good.js @@ -8,9 +8,10 @@ const fixtures = require('../common/fixtures'); // 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 = (flag, file) => exec( +const execNode = (flag, file, callback) => exec( `"${common.isWindows ? process.execPath : '$NODE'}" ${flag} "${common.isWindows ? file : '$FILE'}"`, common.isWindows ? undefined : { env: { ...process.env, NODE: process.execPath, FILE: file } }, + callback, ); // Test both sets of arguments that check syntax