diff --git a/packages/create/create.js b/packages/create/create.js index eda9364e..037c31c8 100755 --- a/packages/create/create.js +++ b/packages/create/create.js @@ -16,6 +16,7 @@ const { copyPluginTemplates } = require("./template"); let dev = false; const scriptExt = process.platform === "win32" ? ".cmd" : ""; const finished = util.promisify(stream.finished); +const { escapeArg } = require("./escape_arg"); // I hate this, there is a bug with rl.close on windows that was meant to have been fixed in Node14.4 // See nodejs/node#21771 and nodejs/node#30701 for the apparent fix that was applied @@ -106,15 +107,25 @@ async function safeOutputFile(file, data, options={}) { } async function execFile(cmd, args) { - logger.verbose(`executing ${cmd} ${args.join(" ")}`); + const escaped = args.map(escapeArg); + logger.verbose(`executing ${cmd} ${escaped.join(" ")}`); return new Promise((resolve, reject) => { - let child = child_process.execFile(cmd, args, { shell: true }, (err, stdout, stderr) => { - if (err) { - reject(err); - } else { - resolve({ stdout, stderr }); + let child = child_process.execFile( + cmd, + escaped, + { + shell: true, + // eslint-disable-next-line node/no-process-env + env: process.platform === "win32" ? { ...process.env, pct: "%" } : undefined, + }, + (err, stdout, stderr) => { + if (err) { + reject(err); + } else { + resolve({ stdout, stderr }); + } } - }); + ); let stdout = new LineSplitter({ readableObjectMode: true }); stdout.on("data", line => { logger.verbose(line.toString()); }); child.stdout.pipe(stdout); diff --git a/packages/create/escape_arg.js b/packages/create/escape_arg.js new file mode 100644 index 00000000..143257d2 --- /dev/null +++ b/packages/create/escape_arg.js @@ -0,0 +1,62 @@ +"use strict"; + +function escapeArg(arg) { + // When shell: true is used execFile does not escaping of arguments, they are + // simply joined by space and then passed to the shell. + + // For Windows the command is cmd.exe /d /s /c "command arg arg ..." + // The parsing and escaping on windows is cursed. The following references are not for mortals: + // https://daviddeley.com/autohotkey/parameters/parameters.htm#WIN + // https://stackoverflow.com/a/4095133 + // https://ss64.com/nt/syntax-esc.html + if (process.platform === "win32") { + if (!/[\t "%&<>\\^|]/.test(arg)) { + return arg; + } + + const escaped = []; + let pos = 0; + while (pos < arg.length) { + if (arg[pos] === "\\") { + // If a backslash is followed by a quote or the end of the string, then + // double the number of backslashes because why not! + let peek = pos + 1; + while (peek !== arg.length && arg[peek] === "\\") { + peek += 1; + } + if (peek === arg.length || arg[peek] === '"') { + const count = peek - pos; + escaped.push("\\".repeat(count * 2)); + pos = peek; + continue; + } else { + escaped.push("\\"); + } + } else if (arg[pos] === '"') { + escaped.push('""'); + } else if (arg[pos] === "%") { + // Expect a pct environment variable to contain %, because cmd does variable substitutions + // before parsing string quotes, and there's no mechanism for escaping % symbols, and this + // is a half sane workaround. The less sane workaround is to replace % with %%cd:~,%. + escaped.push("%pct%"); + } else { + escaped.push(arg[pos]); + } + pos += 1; + } + return `"${escaped.join("")}"`; + } + + // For all other platforms the command is /bin/sh -c + // The command and its arguments are passed as a single argument to the shell. + // See https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02 + // eslint-disable-next-line no-lonely-if + if (!/[|&;<>()$`\\"' \t*?\[#~=%]/.test(arg)) { + return arg; + } + return `"${arg.replace(/[$`"\\]/g, "\\$&")}"`; +} + +module.exports = { + escapeArg, +}; diff --git a/test/create/echo_args.js b/test/create/echo_args.js new file mode 100644 index 00000000..1319b1cf --- /dev/null +++ b/test/create/echo_args.js @@ -0,0 +1,4 @@ +"use strict"; +if (module === require.main) { + process.stdout.write(JSON.stringify(process.argv.slice(2))); +} diff --git a/test/create/escape_arg.js b/test/create/escape_arg.js new file mode 100644 index 00000000..808036c5 --- /dev/null +++ b/test/create/escape_arg.js @@ -0,0 +1,71 @@ +"use strict"; +const child_process = require("child_process"); +const util = require("util"); +const assert = require("assert").strict; +const path = require("path"); + +const { escapeArg } = require("../../packages/create/escape_arg"); +const execFile = util.promisify(child_process.execFile); + +async function exec(file, args) { + const { stdout, stderr } = await execFile( + file, + args.map(escapeArg), + // eslint-disable-next-line node/no-process-env + { shell: true, cwd: __dirname, env: { ...process.env, pct: "%"} } + ); + return { stdout: JSON.parse(stdout), stderr }; +} + +const strings = [ + '"a"', + "a sentence of text.", + " irregular \tspaces ", + '"C:\\path\\"', + "C:\\path\\", + '\\ "\\" \\\\ "\\\\" \\\\\\ "\\\\\\" \\\\\\\\ "\\\\\\\\" \\\\\\\\\\ "\\\\\\\\\\" \\\\\\\\\\\ "\\\\\\\\\\\\"', + '"C:\\path"', + "C:\\path", + "%OS%", + "!OS!", + "😮", +]; +for (let i=1; i < 128; i++) { + const char = String.fromCodePoint(i); + // Skip newline and carriage return because those are difficult to escape + // Skip most alphanumeric characters as they don't behave differently to each other. + if (/[\n\r1-9B-Zb-z]/.test(char)) { + continue; + } + strings.push(char, `${char}${char}`, `${char}"`); +} + +// Test 100 strings at a time. +const tests = [[]]; +for (const string of strings) { + if (tests[tests.length - 1].length >= 100) { + tests.push([]); + } + tests[tests.length - 1].push(string); +} + + +describe("create/escape_arg", function() { + it("should correctly escape strings when args are forwarded", async function() { + const ext = process.platform === "win32" ? ".cmd" : ".sh"; + for (const args of tests) { + assert.deepEqual( + await exec(`.${path.sep}forward_args${ext}`, ["echo_args.js", ...args]), + { stdout: args, stderr: "" } + ); + } + }); + it("should correctly escape strings when invoking node directly", async function() { + for (const args of tests) { + assert.deepEqual( + await exec("node", ["echo_args.js", ...args]), + { stdout: args, stderr: "" } + ); + } + }); +}); diff --git a/test/create/forward_args.cmd b/test/create/forward_args.cmd new file mode 100644 index 00000000..fc77d29c --- /dev/null +++ b/test/create/forward_args.cmd @@ -0,0 +1,3 @@ +@ECHO off +REM this is how npm and package.json bin entries forwards arguments on Windows +node.exe %* diff --git a/test/create/forward_args.sh b/test/create/forward_args.sh new file mode 100755 index 00000000..f7044d7f --- /dev/null +++ b/test/create/forward_args.sh @@ -0,0 +1,3 @@ +#!/bin/sh +# this is how package.json bin entries forwards arguments on Linux +exec node "$@"