fix: Correct spawning of Windows binaries by batch/cmd with spaces (#1057)

Signed-off-by: Chad Wilson <chadw@thoughtworks.com>

Author: chadlwilson

package-lock.json

@@ -8,7 +8,7 @@
   "description": "Gauge support for VScode.",
   "author": "ThoughtWorks",
   "license": "MIT",
-  "version": "0.2.1",
+  "version": "0.2.2",
   "publisher": "getgauge",
   "engines": {
     "vscode": "^1.82.0"

package.json

@@ -1,6 +1,6 @@
 'use strict';
 
-import { CommonSpawnOptions, spawn, spawnSync } from 'child_process';
+import { ChildProcess, CommonSpawnOptions, spawn, spawnSync, SpawnSyncReturns } from 'child_process';
 import { platform } from 'os';
 import { window } from 'vscode';
 import { GaugeCommands, GRADLE_COMMAND, MAVEN_COMMAND } from './constants';
@@ -10,11 +10,11 @@ export class CLI {
     private readonly _gaugeVersion: string;
     private readonly _gaugeCommitHash: string;
     private readonly _gaugePlugins: Array<any>;
-    private readonly _gaugeCommand: string;
-    private readonly _mvnCommand: string;
-    private readonly _gradleCommand: string;
+    private readonly _gaugeCommand: Command;
+    private readonly _mvnCommand: Command;
+    private readonly _gradleCommand: Command;
 
-    public constructor(cmd: string, manifest: any, mvnCommand: string, gradleCommand: string) {
+    public constructor(cmd: Command, manifest: any, mvnCommand: Command, gradleCommand: Command) {
         this._gaugeCommand = cmd;
         this._mvnCommand = mvnCommand;
         this._gradleCommand = gradleCommand;
@@ -23,18 +23,12 @@ export class CLI {
         this._gaugePlugins = manifest.plugins;
     }
 
-    public static getDefaultSpawnOptions(): CommonSpawnOptions {
-        // should only deal with platform specific options
-        return platform() === "win32" ? { shell: true } : {};
-    }
-
     public static instance(): CLI {
         const gaugeCommand = this.getCommand(GaugeCommands.Gauge);
         const mvnCommand = this.getCommand(MAVEN_COMMAND);
-        let gradleCommand = this.getGradleCommand();
-        if (!gaugeCommand || gaugeCommand === '') return new CLI(gaugeCommand, {}, mvnCommand, gradleCommand);
-        let options = this.getDefaultSpawnOptions();
-        let gv = spawnSync(gaugeCommand, [GaugeCommands.Version, GaugeCommands.MachineReadable], options);
+        const gradleCommand = this.getGradleCommand();
+        if (!gaugeCommand) return new CLI(undefined, {}, mvnCommand, gradleCommand);
+        let gv = gaugeCommand.spawnSync([GaugeCommands.Version, GaugeCommands.MachineReadable]);
         let gaugeVersionInfo;
         try {
             gaugeVersionInfo = JSON.parse(gv.stdout.toString());
@@ -49,12 +43,12 @@ export class CLI {
         return this._gaugePlugins.some((p: any) => p.name === pluginName);
     }
 
-    public gaugeCommand(): string {
+    public gaugeCommand(): Command {
         return this._gaugeCommand;
     }
 
     public isGaugeInstalled(): boolean {
-        return !!this._gaugeCommand && this._gaugeCommand !== '';
+        return !!this._gaugeCommand;
     }
 
     public isGaugeVersionGreaterOrEqual(version: string): boolean {
@@ -65,31 +59,21 @@ export class CLI {
         return this._gaugePlugins.find((p) => p.name === language).version;
     }
 
-    public getGaugeVersion(): string {
-        return this._gaugeVersion;
-    }
-
     public async installGaugeRunner(language: string): Promise<any> {
         let oc = window.createOutputChannel("Gauge Install");
         let chan = new OutputChannel(oc, `Installing gauge ${language} plugin ...\n`, "");
         return new Promise((resolve, reject) => {
-            let options = CLI.getDefaultSpawnOptions();
-            let childProcess = spawn(this._gaugeCommand, [GaugeCommands.Install, language], options);
+            let childProcess = this._gaugeCommand.spawn([GaugeCommands.Install, language]);
             childProcess.stdout.on('data', (chunk) => chan.appendOutBuf(chunk.toString()));
             childProcess.stderr.on('data', (chunk) => chan.appendErrBuf(chunk.toString()));
             childProcess.on('exit', (code) => {
-                let postFailureMessage = '\nRefer https://docs.gauge.org/plugin.html' +
-                    ' to install manually';
+                let postFailureMessage = '\nRefer to https://docs.gauge.org/plugin.html to install manually';
                 chan.onFinish(resolve, code, "", postFailureMessage, false);
             });
         });
     }
 
-    public isMavenInstalled(): boolean {
-        return !!this._mvnCommand && this._mvnCommand !== '';
-    }
-
-    public mavenCommand(): string {
+    public mavenCommand(): Command {
         return this._mvnCommand;
     }
 
@@ -107,23 +91,55 @@ export class CLI {
         return `${v}\n${cm}\n\n${plugins}`;
     }
 
-    public static getCommandCandidates(command: string): string[] {
-        return (platform() === 'win32' ? [".exe", ".bat", ".cmd"] : [""])
-            .map((ext) => `${command}${ext}`);
+    public static getCommandCandidates(command: string): Command[] {
+        return platform() === 'win32' ? [
+            new Command(command, ".exe"),
+            new Command(command, ".bat", true),
+            new Command(command, ".cmd", true),
+        ] : [
+            new Command(command)
+        ]
     }
 
-    public static checkSpawnable(command: string): boolean {
-        const result = spawnSync(command, [], CLI.getDefaultSpawnOptions());
+    public static isSpawnable(command: Command): boolean {
+        const result = command.spawnSync();
         return result.status === 0 && !result.error;
     }
 
-    private static getCommand(command: string): string {
+    private static getCommand(command: string): Command | undefined {
         for (const candidate of this.getCommandCandidates(command)) {
-            if (this.checkSpawnable(candidate)) return candidate;
+            if (this.isSpawnable(candidate)) return candidate;
         }
     }
 
     private static getGradleCommand() {
-        return platform() === 'win32' ? `${GRADLE_COMMAND}.bat` : `./${GRADLE_COMMAND}`;
+        return platform() === 'win32' ? new Command(GRADLE_COMMAND, ".bat", true) : new Command(`./${GRADLE_COMMAND}`);
+    }
+}
+
+export type PlatformDependentSpawnOptions = {
+    shell?: boolean
+}
+
+export class Command {
+    public readonly command: string
+    public readonly defaultSpawnOptions: PlatformDependentSpawnOptions
+
+    constructor(public readonly cmdPrefix: string, public readonly cmdSuffix: string = "", public readonly shellMode: boolean = false) {
+        this.command = this.cmdPrefix + this.cmdSuffix;
+        this.defaultSpawnOptions = this.shellMode ? { shell: true } : {};
+    }
+
+    spawn(args: string[] = [], options: CommonSpawnOptions = {}): ChildProcess {
+        return spawn(this.command, this.argsForSpawnType(args), { ...options, ...this.defaultSpawnOptions });
+    }
+
+    spawnSync(args: string[] = [], options: CommonSpawnOptions = {}): SpawnSyncReturns<Buffer> {
+        return spawnSync(this.command, this.argsForSpawnType(args), { ...options, ...this.defaultSpawnOptions });
+    }
+
+    // See https://github.com/nodejs/node/issues/38490
+    argsForSpawnType(args: string[]): string[] {
+        return this.shellMode ? args.map(arg => arg.indexOf(" ") !== -1 ? `"${arg}"` : arg) : args;
     }
 }

src/cli.ts

@@ -37,7 +37,7 @@ export class GaugeExecutor extends Disposable {
     private postExecute: Function[] = [];
     private _disposables: Disposable[] = [];
     private gaugeDebugger: GaugeDebugger;
-    private processors: Array<LineTextProcessor> = new Array();
+    private processors: Array<LineTextProcessor> = [];
 
     constructor(private gaugeWorkspace: GaugeWorkspace, private cli: CLI) {
         super(() => this.dispose());
@@ -62,18 +62,17 @@ export class GaugeExecutor extends Disposable {
                 this.gaugeDebugger.addDebugEnv().then((env) => {
                     let cmd = config.getProject().getExecutionCommand(this.cli);
                     let args = this.getArgs(spec, config);
-                    let initialText = ['Running tool:', cmd, args.join(' ')].join(' ');
+                    let initialText = ['Running tool:', cmd.command, cmd.argsForSpawnType(args).join(' ')].join(' ');
                     let chan = new OutputChannel(this.outputChannel, initialText, config.getProject().root());
                     const relPath = relative(config.getProject().root(), config.getStatus());
                     this.preExecute.forEach((f) => { f.call(null, env, relPath); });
                     this.aborted = false;
-                    let options: SpawnOptions = { cwd: config.getProject().root(), env: env , detached: false };
-                    if (platform() !== 'win32') {
-                        options.detached = true;
-                    } else {
-                        options.shell = true;
-                    }
-                    this.childProcess = spawn(cmd, args, options);
+                    let options: SpawnOptions = {
+                        cwd: config.getProject().root(),
+                        env: env,
+                        detached: platform() !== 'win32'
+                    };
+                    this.childProcess = cmd.spawn(args, options);
                     this.childProcess.stdout.on('data', this.filterStdoutDataDumpsToTextLines((lineText: string) => {
                         chan.appendOutBuf(lineText);
                         lineText.split("\n").forEach((lineText) => {

src/execution/gaugeExecutor.ts

@@ -146,16 +146,16 @@ export class GaugeWorkspace extends Disposable {
         let project = ProjectFactory.get(folder);
         if (this._clientsMap.has(project.root())) return;
         process.env.GAUGE_IGNORE_RUNNER_BUILD_FAILURES = "true";
+        let cmd = this.cli.gaugeCommand();
         let serverOptions: ServerOptions = {
-          command: this.cli.gaugeCommand(),
-          args: ["daemon", "--lsp", "--dir=" + project.root()],
-          options: { env: { ...process.env, ...project.envs(this.cli) } },
+          command: cmd.command,
+          args: cmd.argsForSpawnType(["daemon", "--lsp", "--dir", project.root()]),
+          options: {
+              env: { ...process.env, ...project.envs(this.cli) },
+              ...cmd.defaultSpawnOptions,
+          },
         };
 
-        if (platform() === "win32") {
-            serverOptions.options.shell = true;
-        }
-
         this._launchConfig = workspace.getConfiguration(GAUGE_LAUNCH_CONFIG);
         if (this._launchConfig.get(DEBUG_LOG_LEVEL_CONFIG)) {
             serverOptions.args.push("-l");

src/gaugeWorkspace.ts

@@ -1,6 +1,5 @@
 'use strict';
 
-import { CommonSpawnOptions, spawn, spawnSync } from 'child_process';
 import * as fs from 'fs-extra';
 import * as path from 'path';
 import { commands, Disposable, Progress, Uri, window, workspace } from 'vscode';
@@ -68,9 +67,9 @@ export class ProjectInitializer extends Disposable {
 
     private async createFromCommandLine(template: FileListItem, projectFolder: Uri, p: ProgressHandler) {
         let args = [GaugeCommands.Init, template.label];
-        let options: CommonSpawnOptions = { cwd: projectFolder.fsPath, env: process.env, ...CLI.getDefaultSpawnOptions() };
+        const cmd = this.cli.gaugeCommand();
         p.report("Initializing project...");
-        let proc = spawn(this.cli.gaugeCommand(), args, options);
+        let proc = cmd.spawn(args, { cwd: projectFolder.fsPath, env: process.env });
         proc.addListener('error', async (err) => {
             this.handleError(p, "Failed to create template. " + err.message, projectFolder.fsPath);
         });
@@ -83,8 +82,7 @@ export class ProjectInitializer extends Disposable {
 
     private async getTemplatesList(): Promise<Array<FileListItem>> {
         let args = ["template", "--list", "--machine-readable"];
-        let options: CommonSpawnOptions = { env: process.env, ...CLI.getDefaultSpawnOptions() };
-        let cp = spawnSync(this.cli.gaugeCommand(), args, options);
+        let cp = this.cli.gaugeCommand().spawnSync(args, { env: process.env });
         try {
             let _templates = JSON.parse(cp.stdout.toString());
             return _templates.map((tmpl) => new FileListItem(tmpl.key, tmpl.Description, tmpl.value));

src/init/projectInit.ts

@@ -1,7 +1,7 @@
 'use strict';
 
 import { isAbsolute, relative } from 'path';
-import { CLI } from '../cli';
+import { CLI, Command } from '../cli';
 
 export class GaugeProject {
     private readonly _projectRoot: string;
@@ -18,7 +18,7 @@ export class GaugeProject {
         }
     }
 
-    public getExecutionCommand(cli: CLI): string {
+    public getExecutionCommand(cli: CLI): Command {
         return cli.gaugeCommand();
     }