diff --git a/src/bin.ts b/src/bin.ts index 8cc4be60..c7421859 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -11,6 +11,7 @@ import YAML from "yaml"; import config from "./config"; import { info } from "./message"; import { version } from "./metadata"; +import NodeOptions from "./util/NodeOptions"; import forwardSignals from "./util/forwardSignals"; import { readPkgUp } from "./util/readPkgUp"; @@ -22,11 +23,12 @@ const loaderPath = pathToFileURL(resolve(__dirname, "../dist/loader.js")).href; export function main() { const [cmd, ...args] = process.argv.slice(2); + const nodeOptions = new NodeOptions(process.env.NODE_OPTIONS); if (!cmd) return usage(); info("Running with appmap-node version %s", version); - addNodeOptions("--require", registerPath); + nodeOptions.push("--require", registerPath); if (config.default) { info("Writing default config to %s", config.configPath); writeFileSync(config.configPath, YAML.stringify(config)); @@ -35,9 +37,11 @@ export function main() { // FIXME: Probably there should be a way to remove this altogether // by changing our custom loader implementation - if (isTsEsmLoaderNeeded(cmd, args)) addNodeOptions("--loader", "ts-node/esm"); + if (isTsEsmLoaderNeeded(cmd, args)) nodeOptions.push("--loader", "ts-node/esm"); - addNodeOptions("--loader", loaderPath, "--no-warnings"); + nodeOptions.push("--loader", loaderPath, "--no-warnings"); + + process.env.NODE_OPTIONS = nodeOptions.toString(); let child: ChildProcess | undefined; if (isScript(cmd)) { @@ -100,12 +104,6 @@ function isTsEsmLoaderNeeded(cmd: string, args: string[]) { return [cmd, ...args].includes("ts-node") || tsConfigTsNodeEsmIsTrue(); } -function addNodeOptions(...options: string[]) { - const envar = (process.env.NODE_OPTIONS ?? "").split(" "); - envar.push(...options); - process.env.NODE_OPTIONS = envar.join(" "); -} - /* Heuristic to check if argument is a node script. Currently just checks if it's an existing file with a known extension; the assumption being if it's not, then it's probably a command. */ function isScript(arg: string) { diff --git a/src/util/NodeOptions.ts b/src/util/NodeOptions.ts new file mode 100644 index 00000000..5680809a --- /dev/null +++ b/src/util/NodeOptions.ts @@ -0,0 +1,49 @@ +export default class NodeOptions extends Array { + constructor(value?: string) { + super(); + if (!value) return; + if (typeof value !== "string") throw new Error("NODE_OPTIONS must be a string"); + + this.push(...parseNodeOptionsEnvVar(value)); + } + + toString(): string { + return this.map(maybeQuote).join(" "); + } + + static get [Symbol.species]() { + return Array; + } +} + +// based on ParseNodeOptionsEnvVar() +// https://github.com/nodejs/node/blob/39f1b899cd536de4d4c9bbf56f24927d8d06999a/src/node_options.cc#L1397 +function parseNodeOptionsEnvVar(nodeOptions: string): string[] { + const result: string[] = []; + let buffer = ""; + let inString = false; + for (let i = 0; i < nodeOptions.length; i++) { + let c = nodeOptions[i]; + if (c === "\\" && inString) { + if (i + 1 === nodeOptions.length) break; + c = nodeOptions[++i]; + } else if (c === " " && !inString) { + result.push(buffer); + buffer = ""; + continue; + } else if (c === '"') { + inString = !inString; + continue; + } + + buffer += c; + } + + result.push(buffer); + return result; +} + +function maybeQuote(value: string): string { + if (value.includes(" ")) return `"${value.replaceAll("\\", "\\\\").replaceAll('"', '\\"')}"`; + return value; +} diff --git a/src/util/__tests__/NodeOptions.test.ts b/src/util/__tests__/NodeOptions.test.ts new file mode 100644 index 00000000..d88b6547 --- /dev/null +++ b/src/util/__tests__/NodeOptions.test.ts @@ -0,0 +1,15 @@ +import NodeOptions from "../NodeOptions"; + +describe(NodeOptions, () => { + it("splits the options correctly", () => { + const options = new NodeOptions('--option "spaced argument"'); + expect(Array.from(options)).toEqual(["--option", "spaced argument"]); + }); + + it("stringifies spaced options properly", () => { + const opts = new NodeOptions(""); + opts.push("--option", 'spaced \\argu"me\\nt'); + expect(opts.toString()).toBe('--option "spaced \\\\argu\\"me\\\\nt"'); + expect(Array.from(new NodeOptions(opts.toString()))).toEqual(Array.from(opts)); + }); +}); diff --git a/test/smoketest.mjs b/test/smoketest.mjs index 28d225e8..55e98bac 100755 --- a/test/smoketest.mjs +++ b/test/smoketest.mjs @@ -16,17 +16,17 @@ import tmp from "tmp"; * it doesn't crash and produces an AppMap. */ // Create a temporary directory to work in. -const tmpDir = tmp.dirSync({ unsafeCleanup: true }).name; +// To test NODE_OPTIONS handling, use a space in the directory name and use mjs. +const tmpDir = tmp.dirSync({ unsafeCleanup: true, template: "appmap node test XXXXXX" }).name; -// Package up the appmap-node package. -runCommand("yarn", "pack", "-o", join(tmpDir, "appmap-node.tgz")); +runCommand("yarn", "pack", "-o", JSON.stringify(join(tmpDir, "appmap-node.tgz"))); chdir(tmpDir); runCommand("npm", "init", "-y"); -runCommand("npm", "install", join(tmpDir, "appmap-node.tgz")); +runCommand("npm", "install", "appmap-node.tgz"); writeFileSync( - "index.js", + "index.mjs", ` function main() { console.log("Hello world!"); @@ -35,7 +35,7 @@ main(); `, ); -runCommand("npm", "exec", "appmap-node", "index.js"); +runCommand("npm", "exec", "appmap-node", "index.mjs"); // verify that appmap has been created const files = glob.globSync("tmp/**/*.appmap.json"); @@ -44,7 +44,7 @@ assert(files.length === 1); function runCommand(command, ...args) { const { status } = spawnSync(command, args, { stdio: "inherit", - shell: process.platform == "win32", + shell: true, }); assert(status === 0); }