From a790379a69aaa20495b3fc756d76230f593c95f3 Mon Sep 17 00:00:00 2001
From: Akos Kitta <a.kitta@arduino.cc>
Date: Thu, 11 May 2023 17:31:34 +0200
Subject: [PATCH] fix: removed unsafe `shell` when executing process
Ref: PNX-3671
Co-authored-by: per1234 <accounts@perglass.com>
Co-authored-by: Akos Kitta <a.kitta@arduino.cc>
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
---
arduino-ide-extension/package.json | 5 +-
.../src/common/protocol/executable-service.ts | 1 -
.../src/node/arduino-daemon-impl.ts | 19 +-
.../node/arduino-firmware-uploader-impl.ts | 51 ++---
.../src/node/clang-formatter.ts | 58 ++----
.../src/node/config-service-impl.ts | 13 +-
arduino-ide-extension/src/node/exec-util.ts | 58 ++----
.../src/node/executable-service-impl.ts | 26 +--
.../src/test/node/arduino-daemon-impl.test.ts | 12 +-
.../src/test/node/clang-formatter.test.ts | 162 +++++++++++++++
.../src/test/node/exec-util.test.ts | 187 +++++++++++++++---
yarn.lock | 16 +-
12 files changed, 388 insertions(+), 220 deletions(-)
create mode 100644 arduino-ide-extension/src/test/node/clang-formatter.test.ts
diff --git a/arduino-ide-extension/package.json b/arduino-ide-extension/package.json
index 2d78a1225..ee4075b66 100644
--- a/arduino-ide-extension/package.json
+++ b/arduino-ide-extension/package.json
@@ -104,17 +104,14 @@
"temp": "^0.9.1",
"temp-dir": "^2.0.0",
"tree-kill": "^1.2.1",
- "util": "^0.12.5",
- "which": "^1.3.1"
+ "util": "^0.12.5"
},
"devDependencies": {
"@octokit/rest": "^18.12.0",
"@types/chai": "^4.2.7",
- "@types/chai-string": "^1.4.2",
"@types/mocha": "^5.2.7",
"@types/react-window": "^1.8.5",
"chai": "^4.2.0",
- "chai-string": "^1.5.0",
"decompress": "^4.2.0",
"decompress-tarbz2": "^4.1.1",
"decompress-targz": "^4.1.1",
diff --git a/arduino-ide-extension/src/common/protocol/executable-service.ts b/arduino-ide-extension/src/common/protocol/executable-service.ts
index f29516fb5..81bcc50aa 100644
--- a/arduino-ide-extension/src/common/protocol/executable-service.ts
+++ b/arduino-ide-extension/src/common/protocol/executable-service.ts
@@ -5,6 +5,5 @@ export interface ExecutableService {
clangdUri: string;
cliUri: string;
lsUri: string;
- fwuploaderUri: string;
}>;
}
diff --git a/arduino-ide-extension/src/node/arduino-daemon-impl.ts b/arduino-ide-extension/src/node/arduino-daemon-impl.ts
index 55d3ec96d..0e48f3c60 100644
--- a/arduino-ide-extension/src/node/arduino-daemon-impl.ts
+++ b/arduino-ide-extension/src/node/arduino-daemon-impl.ts
@@ -44,7 +44,6 @@ export class ArduinoDaemonImpl
private _running = false;
private _port = new Deferred<string>();
- private _execPath: string | undefined;
// Backend application lifecycle.
@@ -68,7 +67,7 @@ export class ArduinoDaemonImpl
async start(): Promise<string> {
try {
this.toDispose.dispose(); // This will `kill` the previously started daemon process, if any.
- const cliPath = await this.getExecPath();
+ const cliPath = this.getExecPath();
this.onData(`Starting daemon from ${cliPath}...`);
const { daemon, port } = await this.spawnDaemonProcess();
// Watchdog process for terminating the daemon process when the backend app terminates.
@@ -132,12 +131,8 @@ export class ArduinoDaemonImpl
return this.onDaemonStoppedEmitter.event;
}
- async getExecPath(): Promise<string> {
- if (this._execPath) {
- return this._execPath;
- }
- this._execPath = await getExecPath('arduino-cli', this.onError.bind(this));
- return this._execPath;
+ getExecPath(): string {
+ return getExecPath('arduino-cli');
}
protected async getSpawnArgs(): Promise<string[]> {
@@ -151,7 +146,7 @@ export class ArduinoDaemonImpl
'--port',
'0',
'--config-file',
- `"${cliConfigPath}"`,
+ cliConfigPath,
'-v',
];
if (debug) {
@@ -173,10 +168,8 @@ export class ArduinoDaemonImpl
daemon: ChildProcess;
port: string;
}> {
- const [cliPath, args] = await Promise.all([
- this.getExecPath(),
- this.getSpawnArgs(),
- ]);
+ const args = await this.getSpawnArgs();
+ const cliPath = this.getExecPath();
const ready = new Deferred<{ daemon: ChildProcess; port: string }>();
const options = { shell: true };
const daemon = spawn(`"${cliPath}"`, args, options);
diff --git a/arduino-ide-extension/src/node/arduino-firmware-uploader-impl.ts b/arduino-ide-extension/src/node/arduino-firmware-uploader-impl.ts
index a14f406d1..c8c4b3578 100644
--- a/arduino-ide-extension/src/node/arduino-firmware-uploader-impl.ts
+++ b/arduino-ide-extension/src/node/arduino-firmware-uploader-impl.ts
@@ -1,45 +1,22 @@
+import { ILogger } from '@theia/core/lib/common/logger';
+import { inject, injectable, named } from '@theia/core/shared/inversify';
+import type { Port } from '../common/protocol';
import {
ArduinoFirmwareUploader,
FirmwareInfo,
} from '../common/protocol/arduino-firmware-uploader';
-import { injectable, inject, named } from '@theia/core/shared/inversify';
-import { ExecutableService, Port } from '../common/protocol';
import { getExecPath, spawnCommand } from './exec-util';
-import { ILogger } from '@theia/core/lib/common/logger';
import { MonitorManager } from './monitor-manager';
@injectable()
export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
- @inject(ExecutableService)
- protected executableService: ExecutableService;
-
- protected _execPath: string | undefined;
-
@inject(ILogger)
@named('fwuploader')
- protected readonly logger: ILogger;
-
+ private readonly logger: ILogger;
@inject(MonitorManager)
- protected readonly monitorManager: MonitorManager;
-
- protected onError(error: any): void {
- this.logger.error(error);
- }
-
- async getExecPath(): Promise<string> {
- if (this._execPath) {
- return this._execPath;
- }
- this._execPath = await getExecPath('arduino-fwuploader');
- return this._execPath;
- }
-
- async runCommand(args: string[]): Promise<any> {
- const execPath = await this.getExecPath();
- return await spawnCommand(`"${execPath}"`, args, this.onError.bind(this));
- }
+ private readonly monitorManager: MonitorManager;
- async uploadCertificates(command: string): Promise<any> {
+ async uploadCertificates(command: string): Promise<string> {
return await this.runCommand(['certificates', 'flash', command]);
}
@@ -70,14 +47,13 @@ export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
}
async flash(firmware: FirmwareInfo, port: Port): Promise<string> {
- let output;
const board = {
name: firmware.board_name,
fqbn: firmware.board_fqbn,
};
try {
await this.monitorManager.notifyUploadStarted(board.fqbn, port);
- output = await this.runCommand([
+ const output = await this.runCommand([
'firmware',
'flash',
'--fqbn',
@@ -87,11 +63,18 @@ export class ArduinoFirmwareUploaderImpl implements ArduinoFirmwareUploader {
'--module',
`${firmware.module}@${firmware.firmware_version}`,
]);
- } catch (e) {
- throw e;
+ return output;
} finally {
await this.monitorManager.notifyUploadFinished(board.fqbn, port);
- return output;
}
}
+
+ private onError(error: Error): void {
+ this.logger.error(error);
+ }
+
+ private async runCommand(args: string[]): Promise<string> {
+ const execPath = getExecPath('arduino-fwuploader');
+ return await spawnCommand(execPath, args, this.onError.bind(this));
+ }
}
diff --git a/arduino-ide-extension/src/node/clang-formatter.ts b/arduino-ide-extension/src/node/clang-formatter.ts
index 06f6c41d9..306ee6a63 100644
--- a/arduino-ide-extension/src/node/clang-formatter.ts
+++ b/arduino-ide-extension/src/node/clang-formatter.ts
@@ -1,4 +1,3 @@
-import * as os from 'node:os';
import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
import { MaybePromise } from '@theia/core/lib/common/types';
import { FileUri } from '@theia/core/lib/node/file-uri';
@@ -15,7 +14,7 @@ export class ClangFormatter implements Formatter {
private readonly configService: ConfigService;
@inject(EnvVariablesServer)
- private readonly envVariableServer: EnvVariablesServer;
+ private readonly envVariablesServer: EnvVariablesServer;
async format({
content,
@@ -26,26 +25,19 @@ export class ClangFormatter implements Formatter {
formatterConfigFolderUris: string[];
options?: FormatterOptions;
}): Promise<string> {
- const [execPath, style] = await Promise.all([
- this.execPath(),
- this.style(formatterConfigFolderUris, options),
- ]);
+ const execPath = this.execPath();
+ const args = await this.styleArgs(formatterConfigFolderUris, options);
const formatted = await spawnCommand(
- `"${execPath}"`,
- [style],
+ execPath,
+ args,
console.error,
content
);
return formatted;
}
- private _execPath: string | undefined;
- private async execPath(): Promise<string> {
- if (this._execPath) {
- return this._execPath;
- }
- this._execPath = await getExecPath('clang-format');
- return this._execPath;
+ private execPath(): string {
+ return getExecPath('clang-format');
}
/**
@@ -60,10 +52,10 @@ export class ClangFormatter implements Formatter {
*
* See: https://github.com/arduino/arduino-ide/issues/566
*/
- private async style(
+ private async styleArgs(
formatterConfigFolderUris: string[],
options?: FormatterOptions
- ): Promise<string> {
+ ): Promise<string[]> {
const clangFormatPaths = await Promise.all([
...formatterConfigFolderUris.map((uri) => this.clangConfigPath(uri)),
this.clangConfigPath(this.configDirPath()),
@@ -72,11 +64,11 @@ export class ClangFormatter implements Formatter {
const first = clangFormatPaths.filter(Boolean).shift();
if (first) {
console.debug(
- `Using ${ClangFormatFile} style configuration from '${first}'.`
+ `Using ${clangFormatFilename} style configuration from '${first}'.`
);
- return `-style=file:"${first}"`;
+ return ['-style', `file:${first}`];
}
- return `-style="${style(toClangOptions(options))}"`;
+ return ['-style', style(toClangOptions(options))];
}
private async dataDirPath(): Promise<string | undefined> {
@@ -88,7 +80,7 @@ export class ClangFormatter implements Formatter {
}
private async configDirPath(): Promise<string> {
- const configDirUri = await this.envVariableServer.getConfigDirUri();
+ const configDirUri = await this.envVariablesServer.getConfigDirUri();
return FileUri.fsPath(configDirUri);
}
@@ -100,7 +92,7 @@ export class ClangFormatter implements Formatter {
return undefined;
}
const folderPath = FileUri.fsPath(uri);
- const clangFormatPath = join(folderPath, ClangFormatFile);
+ const clangFormatPath = join(folderPath, clangFormatFilename);
try {
await fs.access(clangFormatPath, constants.R_OK);
return clangFormatPath;
@@ -115,7 +107,7 @@ interface ClangFormatOptions {
readonly TabWidth: number;
}
-const ClangFormatFile = '.clang-format';
+export const clangFormatFilename = '.clang-format';
function toClangOptions(
options?: FormatterOptions | undefined
@@ -129,24 +121,8 @@ function toClangOptions(
return { UseTab: 'Never', TabWidth: 2 };
}
-export function style({ TabWidth, UseTab }: ClangFormatOptions): string {
- let styleArgument = JSON.stringify(styleJson({ TabWidth, UseTab })).replace(
- /[\\"]/g,
- '\\$&'
- );
- if (os.platform() === 'win32') {
- // Windows command interpreter does not use backslash escapes. This causes the argument to have alternate quoted and
- // unquoted sections.
- // Special characters in the unquoted sections must be caret escaped.
- const styleArgumentSplit = styleArgument.split('"');
- for (let i = 1; i < styleArgumentSplit.length; i += 2) {
- styleArgumentSplit[i] = styleArgumentSplit[i].replace(/[<>^|]/g, '^$&');
- }
-
- styleArgument = styleArgumentSplit.join('"');
- }
-
- return styleArgument;
+function style({ TabWidth, UseTab }: ClangFormatOptions): string {
+ return JSON.stringify(styleJson({ TabWidth, UseTab }));
}
function styleJson({
diff --git a/arduino-ide-extension/src/node/config-service-impl.ts b/arduino-ide-extension/src/node/config-service-impl.ts
index 78b7d0cd3..cb3559688 100644
--- a/arduino-ide-extension/src/node/config-service-impl.ts
+++ b/arduino-ide-extension/src/node/config-service-impl.ts
@@ -222,8 +222,8 @@ export class ConfigServiceImpl
}
private async getFallbackCliConfig(): Promise<DefaultCliConfig> {
- const cliPath = await this.daemon.getExecPath();
- const rawJson = await spawnCommand(`"${cliPath}"`, [
+ const cliPath = this.daemon.getExecPath();
+ const rawJson = await spawnCommand(cliPath, [
'config',
'dump',
'format',
@@ -233,13 +233,8 @@ export class ConfigServiceImpl
}
private async initCliConfigTo(fsPathToDir: string): Promise<void> {
- const cliPath = await this.daemon.getExecPath();
- await spawnCommand(`"${cliPath}"`, [
- 'config',
- 'init',
- '--dest-dir',
- `"${fsPathToDir}"`,
- ]);
+ const cliPath = this.daemon.getExecPath();
+ await spawnCommand(cliPath, ['config', 'init', '--dest-dir', fsPathToDir]);
}
private async mapCliConfigToAppConfig(
diff --git a/arduino-ide-extension/src/node/exec-util.ts b/arduino-ide-extension/src/node/exec-util.ts
index 984eff038..f40f6d737 100644
--- a/arduino-ide-extension/src/node/exec-util.ts
+++ b/arduino-ide-extension/src/node/exec-util.ts
@@ -1,51 +1,17 @@
+import { spawn } from 'node:child_process';
import os from 'node:os';
-import which from 'which';
-import semver from 'semver';
import { join } from 'node:path';
-import { spawn } from 'node:child_process';
-export async function getExecPath(
- commandName: string,
- onError: (error: Error) => void = (error) => console.log(error),
- versionArg?: string | undefined,
- inBinDir?: boolean
-): Promise<string> {
- const execName = `${commandName}${os.platform() === 'win32' ? '.exe' : ''}`;
- const relativePath = ['..', '..', 'build'];
- if (inBinDir) {
- relativePath.push('bin');
- }
- const buildCommand = join(__dirname, ...relativePath, execName);
- if (!versionArg) {
- return buildCommand;
- }
- const versionRegexp = /\d+\.\d+\.\d+/;
- const buildVersion = await spawnCommand(
- `"${buildCommand}"`,
- [versionArg],
- onError
- );
- const buildShortVersion = (buildVersion.match(versionRegexp) || [])[0];
- const pathCommand = await new Promise<string | undefined>((resolve) =>
- which(execName, (error, path) => resolve(error ? undefined : path))
- );
- if (!pathCommand) {
- return buildCommand;
- }
- const pathVersion = await spawnCommand(
- `"${pathCommand}"`,
- [versionArg],
- onError
- );
- const pathShortVersion = (pathVersion.match(versionRegexp) || [])[0];
- if (
- pathShortVersion &&
- buildShortVersion &&
- semver.gt(pathShortVersion, buildShortVersion)
- ) {
- return pathCommand;
- }
- return buildCommand;
+export type ArduinoBinaryName =
+ | 'arduino-cli'
+ | 'arduino-fwuploader'
+ | 'arduino-language-server';
+export type ClangBinaryName = 'clangd' | 'clang-format';
+export type BinaryName = ArduinoBinaryName | ClangBinaryName;
+
+export function getExecPath(binaryName: BinaryName): string {
+ const filename = `${binaryName}${os.platform() === 'win32' ? '.exe' : ''}`;
+ return join(__dirname, '..', '..', 'build', filename);
}
export function spawnCommand(
@@ -55,7 +21,7 @@ export function spawnCommand(
stdIn?: string
): Promise<string> {
return new Promise<string>((resolve, reject) => {
- const cp = spawn(command, args, { windowsHide: true, shell: true });
+ const cp = spawn(command, args, { windowsHide: true });
const outBuffers: Buffer[] = [];
const errBuffers: Buffer[] = [];
cp.stdout.on('data', (b: Buffer) => outBuffers.push(b));
diff --git a/arduino-ide-extension/src/node/executable-service-impl.ts b/arduino-ide-extension/src/node/executable-service-impl.ts
index f731387d8..a22609e2b 100644
--- a/arduino-ide-extension/src/node/executable-service-impl.ts
+++ b/arduino-ide-extension/src/node/executable-service-impl.ts
@@ -1,35 +1,19 @@
-import { injectable, inject } from '@theia/core/shared/inversify';
-import { ILogger } from '@theia/core/lib/common/logger';
import { FileUri } from '@theia/core/lib/node/file-uri';
-import { getExecPath } from './exec-util';
+import { injectable } from '@theia/core/shared/inversify';
import { ExecutableService } from '../common/protocol/executable-service';
+import { getExecPath } from './exec-util';
@injectable()
export class ExecutableServiceImpl implements ExecutableService {
- @inject(ILogger)
- protected logger: ILogger;
-
async list(): Promise<{
clangdUri: string;
cliUri: string;
lsUri: string;
- fwuploaderUri: string;
}> {
- const [ls, clangd, cli, fwuploader] = await Promise.all([
- getExecPath('arduino-language-server', this.onError.bind(this)),
- getExecPath('clangd', this.onError.bind(this), undefined),
- getExecPath('arduino-cli', this.onError.bind(this)),
- getExecPath('arduino-fwuploader', this.onError.bind(this)),
- ]);
return {
- clangdUri: FileUri.create(clangd).toString(),
- cliUri: FileUri.create(cli).toString(),
- lsUri: FileUri.create(ls).toString(),
- fwuploaderUri: FileUri.create(fwuploader).toString(),
+ clangdUri: FileUri.create(getExecPath('clangd')).toString(),
+ cliUri: FileUri.create(getExecPath('arduino-cli')).toString(),
+ lsUri: FileUri.create(getExecPath('arduino-language-server')).toString(),
};
}
-
- protected onError(error: Error): void {
- this.logger.error(error);
- }
}
diff --git a/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts b/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts
index 26f12e3f3..9a5d3f875 100644
--- a/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts
+++ b/arduino-ide-extension/src/test/node/arduino-daemon-impl.test.ts
@@ -43,19 +43,13 @@ class SilentArduinoDaemonImpl extends ArduinoDaemonImpl {
}
private async initCliConfig(): Promise<string> {
- const cliPath = await this.getExecPath();
+ const cliPath = this.getExecPath();
const destDir = track.mkdirSync();
- await spawnCommand(`"${cliPath}"`, [
- 'config',
- 'init',
- '--dest-dir',
- destDir,
- ]);
+ await spawnCommand(cliPath, ['config', 'init', '--dest-dir', destDir]);
const content = fs.readFileSync(path.join(destDir, CLI_CONFIG), {
encoding: 'utf8',
});
- const cliConfig = safeLoad(content) as any;
- // cliConfig.daemon.port = String(this.port);
+ const cliConfig = safeLoad(content);
const modifiedContent = safeDump(cliConfig);
fs.writeFileSync(path.join(destDir, CLI_CONFIG), modifiedContent, {
encoding: 'utf8',
diff --git a/arduino-ide-extension/src/test/node/clang-formatter.test.ts b/arduino-ide-extension/src/test/node/clang-formatter.test.ts
new file mode 100644
index 000000000..b8814a48d
--- /dev/null
+++ b/arduino-ide-extension/src/test/node/clang-formatter.test.ts
@@ -0,0 +1,162 @@
+import {
+ Disposable,
+ DisposableCollection,
+} from '@theia/core/lib/common/disposable';
+import { FileUri } from '@theia/core/lib/node/file-uri';
+import { expect } from 'chai';
+import { promises as fs } from 'node:fs';
+import path from 'node:path';
+import temp from 'temp';
+import {
+ clangFormatFilename,
+ ClangFormatter,
+} from '../../node/clang-formatter';
+import { spawnCommand } from '../../node/exec-util';
+import { createBaseContainer, startDaemon } from './test-bindings';
+
+const unformattedContent = `void setup ( ) { pinMode(LED_BUILTIN, OUTPUT);
+}
+
+void loop() {
+ digitalWrite( LED_BUILTIN , HIGH );
+ delay( 1000 ) ;
+ digitalWrite( LED_BUILTIN , LOW);
+delay ( 1000 ) ;
+ }
+`;
+const formattedContent = `void setup() {
+ pinMode(LED_BUILTIN, OUTPUT);
+}
+
+void loop() {
+ digitalWrite(LED_BUILTIN, HIGH);
+ delay(1000);
+ digitalWrite(LED_BUILTIN, LOW);
+ delay(1000);
+}
+`;
+
+type ClangStyleValue =
+ | string
+ | number
+ | boolean
+ | ClangStyleValue[]
+ | { [key: string]: ClangStyleValue };
+type ClangConfiguration = Record<string, ClangStyleValue>;
+
+export interface ClangStyle {
+ readonly key: string;
+ readonly value: ClangStyleValue;
+}
+
+const singleClangStyles: ClangStyle[] = [
+ {
+ key: 'SpacesBeforeTrailingComments',
+ value: 0,
+ },
+ {
+ key: 'SortIncludes',
+ value: 'Never',
+ },
+ {
+ key: 'AlignTrailingComments',
+ value: true,
+ },
+ {
+ key: 'IfMacros',
+ value: ['KJ_IF_MAYBE'],
+ },
+ {
+ key: 'SpacesInLineCommentPrefix',
+ value: {
+ Minimum: 0,
+ Maximum: -1,
+ },
+ },
+];
+
+async function expectNoChanges(
+ formatter: ClangFormatter,
+ styleArg: string
+): Promise<void> {
+ const minimalContent = `
+void setup() {}
+void loop() {}
+`.trim();
+ const execPath = formatter['execPath']();
+ const actual = await spawnCommand(
+ execPath,
+ ['-style', styleArg],
+ console.error,
+ minimalContent
+ );
+ expect(actual).to.be.equal(minimalContent);
+}
+
+describe('clang-formatter', () => {
+ let tracked: typeof temp;
+ let formatter: ClangFormatter;
+ let toDispose: DisposableCollection;
+
+ before(async () => {
+ tracked = temp.track();
+ toDispose = new DisposableCollection(
+ Disposable.create(() => tracked.cleanupSync())
+ );
+ const container = await createBaseContainer({
+ additionalBindings: (bind) =>
+ bind(ClangFormatter).toSelf().inSingletonScope(),
+ });
+ await startDaemon(container, toDispose);
+ formatter = container.get<ClangFormatter>(ClangFormatter);
+ });
+
+ after(() => toDispose.dispose());
+
+ singleClangStyles
+ .map((style) => ({
+ ...style,
+ styleArg: JSON.stringify({ [style.key]: style.value }),
+ }))
+ .map(({ value, styleArg }) =>
+ it(`should execute the formatter with a single ${
+ Array.isArray(value) ? 'array' : typeof value
+ } type style configuration value: ${styleArg}`, async () => {
+ await expectNoChanges(formatter, styleArg);
+ })
+ );
+
+ it('should execute the formatter with a multiple clang formatter styles', async () => {
+ const styleArg = JSON.stringify(
+ singleClangStyles.reduce((config, curr) => {
+ config[curr.key] = curr.value;
+ return config;
+ }, {} as ClangConfiguration)
+ );
+ await expectNoChanges(formatter, styleArg);
+ });
+
+ it('should format with the default styles', async () => {
+ const actual = await formatter.format({
+ content: unformattedContent,
+ formatterConfigFolderUris: [],
+ });
+ expect(actual).to.be.equal(formattedContent);
+ });
+
+ it('should format with custom formatter configuration file', async () => {
+ const tempPath = tracked.mkdirSync();
+ await fs.writeFile(
+ path.join(tempPath, clangFormatFilename),
+ 'SpaceInEmptyParentheses: true',
+ {
+ encoding: 'utf8',
+ }
+ );
+ const actual = await formatter.format({
+ content: 'void foo() {}',
+ formatterConfigFolderUris: [FileUri.create(tempPath).toString()],
+ });
+ expect(actual).to.be.equal('void foo( ) {}');
+ });
+});
diff --git a/arduino-ide-extension/src/test/node/exec-util.test.ts b/arduino-ide-extension/src/test/node/exec-util.test.ts
index 24d947dbf..d10c56fc3 100644
--- a/arduino-ide-extension/src/test/node/exec-util.test.ts
+++ b/arduino-ide-extension/src/test/node/exec-util.test.ts
@@ -1,33 +1,164 @@
-import * as os from 'node:os';
-import { expect, use } from 'chai';
-import { getExecPath } from '../../node/exec-util';
-
-use(require('chai-string'));
-
-describe('getExecPath', () => {
- it('should resolve arduino-cli', async () => {
- const actual = await getExecPath('arduino-cli', onError, 'version');
- const expected =
- os.platform() === 'win32' ? '\\arduino-cli.exe' : '/arduino-cli';
- expect(actual).to.endsWith(expected);
- });
+import { assert, expect } from 'chai';
+import fs from 'node:fs';
+import path from 'node:path';
+import {
+ ArduinoBinaryName,
+ BinaryName,
+ ClangBinaryName,
+ getExecPath,
+ spawnCommand,
+} from '../../node/exec-util';
+import temp from 'temp';
- it('should resolve arduino-language-server', async () => {
- const actual = await getExecPath('arduino-language-server');
- const expected =
- os.platform() === 'win32'
- ? '\\arduino-language-server.exe'
- : '/arduino-language-server';
- expect(actual).to.endsWith(expected);
- });
+describe('exec-utils', () => {
+ describe('spawnCommand', () => {
+ let tracked: typeof temp;
+
+ before(() => {
+ tracked = temp.track();
+ });
+
+ after(() => {
+ if (tracked) {
+ tracked.cleanupSync();
+ }
+ });
- it('should resolve clangd', async () => {
- const actual = await getExecPath('clangd', onError, '--version');
- const expected = os.platform() === 'win32' ? '\\clangd.exe' : '/clangd';
- expect(actual).to.endsWith(expected);
+ it("should execute the command without 'shell:true' even if the path contains spaces but is not escaped", async () => {
+ const segment = 'with some spaces';
+ const cliPath = getExecPath('arduino-cli');
+ const filename = path.basename(cliPath);
+ const tempPath = tracked.mkdirSync();
+ const tempPathWitSpaces = path.join(tempPath, segment);
+ fs.mkdirSync(tempPathWitSpaces, { recursive: true });
+ const cliCopyPath = path.join(tempPathWitSpaces, filename);
+ fs.copyFileSync(cliPath, cliCopyPath);
+ expect(fs.accessSync(cliCopyPath, fs.constants.X_OK)).to.be.undefined;
+ expect(cliCopyPath.includes(segment)).to.be.true;
+ const stdout = await spawnCommand(cliCopyPath, ['version']);
+ expect(stdout.includes(filename)).to.be.true;
+ });
});
- function onError(error: Error): void {
- console.error(error);
- }
+ describe('getExecPath', () => {
+ type AssertOutput = (stdout: string) => void;
+
+ interface GetExecPathTestSuite {
+ readonly name: BinaryName;
+ readonly flags?: string[];
+ readonly assertOutput: AssertOutput;
+ /**
+ * The Arduino LS repository is not as shiny as the CLI or the firmware uploader.
+ * It does not support `version` flag either, so non-zero exit is expected.
+ */
+ readonly expectNonZeroExit?: boolean;
+ }
+
+ const binaryNameToVersionMapping: Record<BinaryName, string> = {
+ 'arduino-cli': 'cli',
+ 'arduino-language-server': 'languageServer',
+ 'arduino-fwuploader': 'fwuploader',
+ clangd: 'clangd',
+ 'clang-format': 'clangd',
+ };
+
+ function readVersionFromPackageJson(name: BinaryName): string {
+ const raw = fs.readFileSync(
+ path.join(__dirname, '..', '..', '..', 'package.json'),
+ { encoding: 'utf8' }
+ );
+ const json = JSON.parse(raw);
+ expect(json.arduino).to.be.not.undefined;
+ const mappedName = binaryNameToVersionMapping[name];
+ expect(mappedName).to.be.not.undefined;
+ const version = json.arduino[mappedName].version;
+ expect(version).to.be.not.undefined;
+ return version;
+ }
+
+ function createTaskAssert(name: ArduinoBinaryName): AssertOutput {
+ const version = readVersionFromPackageJson(name);
+ if (typeof version === 'string') {
+ return (stdout: string) => {
+ expect(stdout.includes(name)).to.be.true;
+ expect(stdout.includes(version)).to.be.true;
+ expect(stdout.includes('git-snapshot')).to.be.false;
+ };
+ }
+ return (stdout: string) => {
+ expect(stdout.includes(name)).to.be.true;
+ expect(stdout.includes('git-snapshot')).to.be.true;
+ };
+ }
+
+ function createClangdAssert(name: ClangBinaryName): AssertOutput {
+ const version = readVersionFromPackageJson(name);
+ return (stdout: string) => {
+ expect(stdout.includes(name)).to.be.true;
+ expect(stdout.includes(`version ${version}`)).to.be.true;
+ };
+ }
+
+ const suites: GetExecPathTestSuite[] = [
+ {
+ name: 'arduino-cli',
+ flags: ['version'],
+ assertOutput: createTaskAssert('arduino-cli'),
+ },
+ {
+ name: 'arduino-fwuploader',
+ flags: ['version'],
+ assertOutput: createTaskAssert('arduino-fwuploader'),
+ },
+ {
+ name: 'arduino-language-server',
+ assertOutput: (stderr: string) => {
+ expect(stderr.includes('Path to ArduinoCLI config file must be set.'))
+ .to.be.true;
+ },
+ expectNonZeroExit: true,
+ },
+ {
+ name: 'clangd',
+ flags: ['--version'],
+ assertOutput: createClangdAssert('clangd'),
+ },
+ {
+ name: 'clang-format',
+ flags: ['--version'],
+ assertOutput: createClangdAssert('clang-format'),
+ },
+ ];
+
+ // This is not a functional test but it ensures all executables provided by IDE2 are tested.
+ it('should cover all provided executables', () => {
+ expect(suites.length).to.be.equal(
+ Object.keys(binaryNameToVersionMapping).length
+ );
+ });
+
+ suites.map((suite) =>
+ it(`should resolve '${suite.name}'`, async () => {
+ const execPath = getExecPath(suite.name);
+ expect(execPath).to.be.not.undefined;
+ expect(execPath).to.be.not.empty;
+ expect(fs.accessSync(execPath, fs.constants.X_OK)).to.be.undefined;
+ if (suite.expectNonZeroExit) {
+ try {
+ await spawnCommand(execPath, suite.flags ?? []);
+ assert.fail('Expected a non-zero exit code');
+ } catch (err) {
+ expect(err).to.be.an.instanceOf(Error);
+ const stderr = (<Error>err).message;
+ expect(stderr).to.be.not.undefined;
+ expect(stderr).to.be.not.empty;
+ suite.assertOutput(stderr);
+ }
+ } else {
+ const stdout = await spawnCommand(execPath, suite.flags ?? []);
+ suite.assertOutput(stdout);
+ }
+ })
+ );
+ });
});
diff --git a/yarn.lock b/yarn.lock
index 58a12fc91..e5a69a4c7 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2741,14 +2741,7 @@
"@types/node" "*"
"@types/responselike" "^1.0.0"
-"@types/chai-string@^1.4.2":
- version "1.4.2"
- resolved "https://registry.yarnpkg.com/@types/chai-string/-/chai-string-1.4.2.tgz#0f116504a666b6c6a3c42becf86634316c9a19ac"
- integrity sha512-ld/1hV5qcPRGuwlPdvRfvM3Ka/iofOk2pH4VkasK4b1JJP1LjNmWWn0LsISf6RRzyhVOvs93rb9tM09e+UuF8Q==
- dependencies:
- "@types/chai" "*"
-
-"@types/chai@*", "@types/chai@^4.2.7":
+"@types/chai@^4.2.7":
version "4.3.4"
resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.3.4.tgz#e913e8175db8307d78b4e8fa690408ba6b65dee4"
integrity sha512-KnRanxnpfpjUTqTCXslZSEdLfXExwgNxYPdiO2WGUj8+HDjFi8R3k5RVKPeSCzLjCcshCAtVO2QBbVuAV4kTnw==
@@ -4814,11 +4807,6 @@ caw@^2.0.1:
tunnel-agent "^0.6.0"
url-to-options "^1.0.1"
-chai-string@^1.5.0:
- version "1.5.0"
- resolved "https://registry.yarnpkg.com/chai-string/-/chai-string-1.5.0.tgz#0bdb2d8a5f1dbe90bc78ec493c1c1c180dd4d3d2"
- integrity sha512-sydDC3S3pNAQMYwJrs6dQX0oBQ6KfIPuOZ78n7rocW0eJJlsHPh2t3kwW7xfwYA/1Bf6/arGtSUo16rxR2JFlw==
-
chai@^4.2.0:
version "4.3.7"
resolved "https://registry.yarnpkg.com/chai/-/chai-4.3.7.tgz#ec63f6df01829088e8bf55fca839bcd464a8ec51"
@@ -15188,7 +15176,7 @@ which-typed-array@^1.1.2, which-typed-array@^1.1.9:
has-tostringtag "^1.0.0"
is-typed-array "^1.1.10"
-which@1.3.1, which@^1.2.9, which@^1.3.1:
+which@1.3.1, which@^1.2.9:
version "1.3.1"
resolved "https://registry.yarnpkg.com/which/-/which-1.3.1.tgz#a45043d54f5805316da8d62f9f50918d3da70b0a"
integrity sha512-HxJdYWq1MTIQbJ3nw0cqssHoTNU267KlrDuGZ1WYlxDStUtKUhOaJmh112/TZmHxxUfuJqPXSOm7tDyas0OSIQ==