Merge pull request Ericsson#57 from Discookie/ericsson/fix-serialize-…

…path

Refactor executor to pass shell arguments as array

README.md

@@ -83,7 +83,7 @@ Since CodeChecker-related paths vary greatly between systems, the following sett
 | CodeChecker > Backend > Compilation database path <br> (default: *(empty)*) | Path to a custom compilation database, in case of a custom build system. The database setup dialog sets the path for the current workspace only. |
 | CodeChecker > Editor > Show database dialog <br> (default: `on`) | Controls the dialog when opening a workspace without a compilation database. |
 | CodeChecker > Editor > Enable CodeLens <br> (default: `on`) | Enable CodeLens for displaying the reproduction path in the editor. |
-| CodeChecker > Executor > Executable path <br> (default: `CodeChecker`) |  Path to the CodeChecker executable (can be an executable in the `PATH` environment variable). |
+| CodeChecker > Executor > Executable path <br> (default: `CodeChecker`) | Path to the CodeChecker executable. Can be an executable in the `PATH` environment variable, or an absolute path to one. |
 | CodeChecker > Executor > Thread count <br> (default: *(empty)*) | CodeChecker's thread count - leave empty to use all threads. |
 | CodeChecker > Executor > Arguments <br> (default: *(empty)*) | Additional arguments to CodeChecker. For example, if you want to use a config file for CodeChecker pass '--config <config.json>'. For supported arguments, run `CodeChecker analyze --help`. <br> *Note:* The resulting command-line can be previewed with the command `CodeChecker: Show full command line`. |
 | CodeChecker > Executor > Run on save <br> (default: `on`) | Controls auto-run of CodeChecker on saving a file. |

package.json

@@ -84,7 +84,7 @@
       "properties": {
         "codechecker.executor.executablePath": {
           "type": "string",
-          "description": "Path to CodeChecker's executable",
+          "description": "Path to CodeChecker's executable. Can be an executable in the `PATH` environment variable, or an absolute path to one.",
           "default": "CodeChecker",
           "order": 1
         },
@@ -95,10 +95,7 @@
           "order": 2
         },
         "codechecker.backend.compilationDatabasePath": {
-          "type": [
-            "string",
-            "null"
-          ],
+          "type": "string",
           "description": "Path to a custom compilation database, in case of a custom build system",
           "default": null,
           "order": 3
@@ -237,6 +234,7 @@
     "@types/glob": "^7.1.3",
     "@types/mocha": "^8.0.4",
     "@types/node": "^12.11.7",
+    "@types/shell-quote": "^1.7.1",
     "eslint": "^7.19.0",
     "@typescript-eslint/eslint-plugin": "^4.14.1",
     "@typescript-eslint/parser": "^4.14.1",
@@ -248,5 +246,8 @@
     "vsce": "^1.100.1",
     "webpack": "^5.19.0",
     "webpack-cli": "^4.4.0"
+  },
+  "dependencies": {
+    "shell-quote": "^1.7.3"
   }
 }

src/backend/executor/bridge.ts

@@ -12,8 +12,9 @@ import {
 import * as fs from 'fs';
 import * as path from 'path';
 import { ExtensionApi } from '../api';
-import { getConfigAndReplaceVariables } from '../../utils/config';
+import { getConfigAndReplaceVariables, replaceVariables } from '../../utils/config';
 import { ProcessStatus, ProcessType, ScheduledProcess } from '.';
+import { parse } from 'shell-quote';
 
 // Structure:
 //   CodeChecker analyzer version: \n {"base_package_version": "M.m.p", ...}
@@ -121,18 +122,35 @@ export class ExecutorBridge implements Disposable {
         return undefined;
     }
 
-    public getAnalyzeCmdLine(...files: Uri[]): string | undefined {
+    public getAnalyzeCmdArgs(...files: Uri[]): string[] | undefined {
         if (!workspace.workspaceFolders?.length) {
             return undefined;
         }
 
+        const workspaceFolder = workspace.workspaceFolders[0].uri.fsPath;
+
         // TODO: Refactor for less code repetition across functions
-        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath')
-            ?? 'CodeChecker';
         const reportsFolder = this.getReportsFolder();
-        const ccArguments = getConfigAndReplaceVariables('codechecker.executor', 'arguments') ?? '';
-        const ccThreads = workspace.getConfiguration('codechecker.executor').get<string>('threadCount');
 
+        const ccArgumentsSetting = workspace.getConfiguration('codechecker.executor').get<string>('arguments');
+
+        // TODO: Merge this collection with replaceVariables
+        const env: { [key: string]: string } = {
+            workspaceFolder,
+            workspaceRoot: workspaceFolder,
+            cwd: process.cwd()
+        };
+        for (const [key, val] of Object.entries(process.env)) {
+            if (val !== undefined) {
+                env[`env:${key}`] = val;
+            }
+        }
+
+        const ccArguments = parse(ccArgumentsSetting ?? '', env)
+            .filter((entry) => typeof entry === 'string' && entry.length > 0)
+            .map((entry) => replaceVariables(entry as string)!);
+
+        const ccThreads = workspace.getConfiguration('codechecker.executor').get<string>('threadCount');
         const ccCompileCmd = this.getCompileCommandsPath();
 
         if (ccCompileCmd === undefined) {
@@ -141,69 +159,66 @@ export class ExecutorBridge implements Disposable {
         }
 
         const filePaths = files.length
-            ? `--file ${files.map((uri) => `"${uri.fsPath}"`).join(' ')}`
-            : '';
+            ? ['--file', ...files.map((uri) => uri.fsPath)]
+            : [];
 
-        return [
-            `${ccPath} analyze`,
-            `"${ccCompileCmd}"`,
-            `--output "${reportsFolder}"`,
-            `${ccThreads ? '-j ' + ccThreads : ''}`,
-            `${ccArguments}`,
-            `${filePaths}`,
-        ].join(' ');
+        const args = [
+            'analyze', ccCompileCmd,
+            '--output', reportsFolder,
+        ];
+
+        if (ccThreads) {
+            args.push('-j', ccThreads);
+        }
+
+        args.push(...ccArguments, ...filePaths);
+
+        return args;
     }
 
-    public getLogCmdLine(): string | undefined {
+    public getLogCmdArgs(): string[] | undefined {
         if (!workspace.workspaceFolders?.length) {
             return undefined;
         }
 
         const workspaceFolder = workspace.workspaceFolders[0].uri.fsPath;
-
-        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath')
-            ?? 'CodeChecker';
         const ccFolder = getConfigAndReplaceVariables('codechecker.backend', 'outputFolder')
             ?? path.join(workspaceFolder, '.codechecker');
 
         // Use a predefined path here
         const ccCompileCmd = path.join(ccFolder, 'compile_commands.json');
 
         return [
-            `${ccPath} log`,
-            `--output "${ccCompileCmd}"`,
-            '--build "make"'
-        ].join(' ');
+            'log',
+            '--output', ccCompileCmd,
+            '--build', 'make'
+        ];
     }
 
-    public getParseCmdLine(...files: Uri[]): string | undefined {
+    public getParseCmdArgs(...files: Uri[]): string[] | undefined {
         if (!workspace.workspaceFolders?.length) {
             return undefined;
         }
 
-        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath') ?? 'CodeChecker';
         const reportsFolder = this.getReportsFolder();
 
         const filePaths = files.length
-            ? `--file ${files.map((uri) => `"${uri.fsPath}"`).join(' ')}`
-            : '';
+            ? ['--file', ...files.map((uri) => uri.fsPath)]
+            : [];
 
         return [
-            `${ccPath} parse`,
-            `${reportsFolder}`,
-            '-e json',
-            filePaths
-        ].join(' ');
+            'parse',
+            reportsFolder,
+            '-e', 'json',
+            ...filePaths
+        ];
     }
 
-    public getVersionCmdLine(): string | undefined {
-        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath')
-            ?? 'CodeChecker';
-
+    public getVersionCmdArgs(): string[] | undefined {
         return [
-            `${ccPath} analyzer-version`,
-            '--output "json"',
-        ].join(' ');
+            'analyzer-version',
+            '--output', 'json',
+        ];
     }
 
     private analyzeOnSave() {
@@ -249,13 +264,14 @@ export class ExecutorBridge implements Disposable {
             return;
         }
 
-        const commandLine = this.getAnalyzeCmdLine(file);
+        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath') || 'CodeChecker';
+        const commandArgs = this.getAnalyzeCmdArgs(file);
 
-        if (commandLine === undefined) {
+        if (commandArgs === undefined) {
             return;
         }
 
-        const process = new ScheduledProcess(commandLine, { processType: ProcessType.analyze });
+        const process = new ScheduledProcess(ccPath, commandArgs, { processType: ProcessType.analyze });
 
         ExtensionApi.executorManager.addToQueue(process, 'prepend');
     }
@@ -268,13 +284,14 @@ export class ExecutorBridge implements Disposable {
         // Kill the process, since the entire project is getting analyzed anyways
         this.stopAnalysis();
 
-        const commandLine = this.getAnalyzeCmdLine();
+        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath') || 'CodeChecker';
+        const commandArgs = this.getAnalyzeCmdArgs();
 
-        if (commandLine === undefined) {
+        if (commandArgs === undefined) {
             return;
         }
 
-        const process = new ScheduledProcess(commandLine, { processType: ProcessType.analyze });
+        const process = new ScheduledProcess(ccPath, commandArgs, { processType: ProcessType.analyze });
 
         ExtensionApi.executorManager.addToQueue(process, 'replace');
     }
@@ -292,13 +309,14 @@ export class ExecutorBridge implements Disposable {
             return;
         }
 
-        const commandLine = this.getParseCmdLine(...files);
+        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath') || 'CodeChecker';
+        const commandArgs = this.getParseCmdArgs(...files);
 
-        if (commandLine === undefined) {
+        if (commandArgs === undefined) {
             return;
         }
 
-        const process = new ScheduledProcess(commandLine, { processType: ProcessType.parse });
+        const process = new ScheduledProcess(ccPath, commandArgs, { processType: ProcessType.parse });
 
         // TODO: Find a better way to collect full logger output
         let processOutput = '';
@@ -321,16 +339,17 @@ export class ExecutorBridge implements Disposable {
                 return;
             }
 
-            const commandLine = this.getVersionCmdLine();
+            const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath') || 'CodeChecker';
+            const commandArgs = this.getVersionCmdArgs();
 
-            if (commandLine === undefined) {
+            if (commandArgs === undefined) {
                 this._bridgeMessages.fire('>>> Unable to determine CodeChecker version commandline\n');
 
                 this.versionChecked = false;
                 return;
             }
 
-            const process = new ScheduledProcess(commandLine, { processType: ProcessType.version });
+            const process = new ScheduledProcess(ccPath, commandArgs, { processType: ProcessType.version });
 
             let processOutput = '';
 

src/backend/executor/process.ts

@@ -1,4 +1,5 @@
 import * as child_process from 'child_process';
+import { quote } from 'shell-quote';
 import { Disposable, Event, EventEmitter, ExtensionContext, workspace } from 'vscode';
 
 export enum ProcessStatus {
@@ -30,8 +31,18 @@ export interface ProcessParameters {
 }
 
 export class ScheduledProcess implements Disposable {
-    /** Full command line of the scheduled process. */
-    public readonly commandLine: string;
+    /** Command line of the executed process.
+     * Note: In the executed command line, each argument is passed separately, so no need to escape individual args.
+     */
+    public get commandLine() {
+        return quote([
+            this.executable,
+            ...this.commandArgs
+        ]);
+    };
+
+    public readonly executable: string;
+    public readonly commandArgs: string[];
 
     private activeProcess?: child_process.ChildProcess;
 
@@ -72,14 +83,16 @@ export class ScheduledProcess implements Disposable {
         return this._processStatusChange.event;
     }
 
-    constructor(commandLine: string, parameters: ProcessParameters) {
-        this.commandLine = commandLine;
-        this.processParameters = parameters;
+    constructor(executable: string, commandArgs?: string[], parameters?: ProcessParameters) {
+        this.executable = executable;
+        this.commandArgs = commandArgs ?? [];
+        this.processParameters = parameters ?? {};
 
+        const processType = parameters?.processType ?? '';
         const forwardDefaults: string[] = [ ProcessType.parse ];
 
         if (this.processParameters.forwardStdoutToLogs === undefined) {
-            this.processParameters.forwardStdoutToLogs = !forwardDefaults.includes(parameters.processType ?? '');
+            this.processParameters.forwardStdoutToLogs = !forwardDefaults.includes(processType);
         }
     }
 
@@ -110,7 +123,7 @@ export class ScheduledProcess implements Disposable {
 
         this._processStderr.fire(`>>> Starting process '${commonName}'\n`);
         this._processStderr.fire(`> ${this.commandLine}\n`);
-        this.activeProcess = child_process.spawn(this.commandLine, { shell: true });
+        this.activeProcess = child_process.spawn(this.executable, this.commandArgs);
 
         this.activeProcess.stdout!.on('data', (stdout: Buffer) => {
             const decoded = stdout.toString();

src/editor/executor.ts

@@ -1,3 +1,4 @@
+import { quote } from 'shell-quote';
 import {
     ExtensionContext,
     StatusBarAlignment,
@@ -8,6 +9,7 @@ import {
 import { Editor } from '.';
 import { ExtensionApi } from '../backend';
 import { ProcessStatus } from '../backend/executor/process';
+import { getConfigAndReplaceVariables } from '../utils/config';
 
 export class ExecutorAlerts {
     private statusBarItem: StatusBarItem;
@@ -31,7 +33,12 @@ export class ExecutorAlerts {
     }
 
     printCmdLine() {
-        const commandLine = ExtensionApi.executorBridge.getAnalyzeCmdLine();
+        const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath') || 'CodeChecker';
+        const commandLine = quote([
+            ccPath,
+            ...ExtensionApi.executorBridge.getAnalyzeCmdArgs() ?? []
+        ]);
+
         Editor.loggerPanel.window.appendLine('>>> Full command line:');
         Editor.loggerPanel.window.appendLine(`>>> ${commandLine}`);
 

src/editor/initialize.ts

@@ -1,4 +1,5 @@
 import { ExtensionContext, Uri, commands, window, workspace } from 'vscode';
+import { quote } from 'shell-quote';
 import { ExtensionApi } from '../backend';
 
 export class FolderInitializer {
@@ -54,13 +55,19 @@ export class FolderInitializer {
         case 'Run CodeChecker log':
             await workspace.fs.createDirectory(codeCheckerFolder);
 
+            const ccPath = getConfigAndReplaceVariables('codechecker.executor', 'executablePath') || 'CodeChecker';
+            const commandLine = quote([
+                ccPath,
+                ...ExtensionApi.executorBridge.getLogCmdArgs()!
+            ]);
+
             const terminal = window.createTerminal('CodeChecker');
             terminal.show(false);
 
             // Wait some time until the terminal is initialized properly. For now there is no elegant solution to solve
             // this problem than using setTimeout.
             setTimeout(() => {
-                terminal.sendText(ExtensionApi.executorBridge.getLogCmdLine()!, false);
+                terminal.sendText(commandLine, false);
             }, 1000);
 
             return;
@@ -82,7 +89,7 @@ export class FolderInitializer {
 
         // If initialization failed, show notification again
         if (
-            ExtensionApi.executorBridge.getAnalyzeCmdLine() === undefined &&
+            ExtensionApi.executorBridge.getAnalyzeCmdArgs() === undefined &&
             workspace.getConfiguration('codechecker.editor').get('showDatabaseDialog') !== false
         ) {
             await this.showDialog();