From fb10de1446c7d3ed93a7bb30a0404ffbdc01f4a3 Mon Sep 17 00:00:00 2001 From: Akos Kitta Date: Fri, 10 Mar 2023 13:55:34 +0100 Subject: [PATCH] fix: jsonc parsing in the IDE2 backend Occurred when `settings.json` contained comments or a trailing comma. Closes #1945 Signed-off-by: Akos Kitta --- arduino-ide-extension/package.json | 1 + .../src/node/arduino-daemon-impl.ts | 38 ++++--------- .../src/node/arduino-ide-backend-module.ts | 3 ++ .../src/node/settings-reader.ts | 53 +++++++++++++++++++ .../src/node/sketches-service-impl.ts | 33 ++---------- .../src/test/node/settings.reader.test.ts | 45 ++++++++++++++++ .../src/test/node/test-bindings.ts | 2 + 7 files changed, 118 insertions(+), 57 deletions(-) create mode 100644 arduino-ide-extension/src/node/settings-reader.ts create mode 100644 arduino-ide-extension/src/test/node/settings.reader.test.ts diff --git a/arduino-ide-extension/package.json b/arduino-ide-extension/package.json index ad52691c0..11a8a6a3f 100644 --- a/arduino-ide-extension/package.json +++ b/arduino-ide-extension/package.json @@ -78,6 +78,7 @@ "google-protobuf": "^3.20.1", "hash.js": "^1.1.7", "js-yaml": "^3.13.1", + "jsonc-parser": "^2.2.0", "just-diff": "^5.1.1", "jwt-decode": "^3.1.2", "keytar": "7.2.0", diff --git a/arduino-ide-extension/src/node/arduino-daemon-impl.ts b/arduino-ide-extension/src/node/arduino-daemon-impl.ts index 3f2480d07..ec0e0cc31 100644 --- a/arduino-ide-extension/src/node/arduino-daemon-impl.ts +++ b/arduino-ide-extension/src/node/arduino-daemon-impl.ts @@ -1,5 +1,4 @@ import { join } from 'path'; -import { promises as fs } from 'fs'; import { inject, injectable, named } from '@theia/core/shared/inversify'; import { spawn, ChildProcess } from 'child_process'; import { FileUri } from '@theia/core/lib/node/file-uri'; @@ -16,7 +15,7 @@ import { BackendApplicationContribution } from '@theia/core/lib/node/backend-app import { ArduinoDaemon, NotificationServiceServer } from '../common/protocol'; import { CLI_CONFIG } from './cli-config'; import { getExecPath } from './exec-util'; -import { ErrnoException } from './utils/errors'; +import { SettingsReader } from './settings-reader'; @injectable() export class ArduinoDaemonImpl @@ -32,6 +31,9 @@ export class ArduinoDaemonImpl @inject(NotificationServiceServer) private readonly notificationService: NotificationServiceServer; + @inject(SettingsReader) + private readonly settingsReader: SettingsReader; + private readonly toDispose = new DisposableCollection(); private readonly onDaemonStartedEmitter = new Emitter(); private readonly onDaemonStoppedEmitter = new Emitter(); @@ -149,34 +151,12 @@ export class ArduinoDaemonImpl } private async debugDaemon(): Promise { - // Poor man's preferences on the backend. (https://github.com/arduino/arduino-ide/issues/1056#issuecomment-1153975064) - const configDirUri = await this.envVariablesServer.getConfigDirUri(); - const configDirPath = FileUri.fsPath(configDirUri); - try { - const raw = await fs.readFile(join(configDirPath, 'settings.json'), { - encoding: 'utf8', - }); - const json = this.tryParse(raw); - if (json) { - const value = json['arduino.cli.daemon.debug']; - return typeof value === 'boolean' && !!value; - } - return false; - } catch (error) { - if (ErrnoException.isENOENT(error)) { - return false; - } - throw error; - } - } - - // eslint-disable-next-line @typescript-eslint/no-explicit-any - private tryParse(raw: string): any | undefined { - try { - return JSON.parse(raw); - } catch { - return undefined; + const settings = await this.settingsReader.read(); + if (settings) { + const value = settings['arduino.cli.daemon.debug']; + return value === true; } + return false; } protected async spawnDaemonProcess(): Promise<{ diff --git a/arduino-ide-extension/src/node/arduino-ide-backend-module.ts b/arduino-ide-extension/src/node/arduino-ide-backend-module.ts index 812761f77..5cd81f7c3 100644 --- a/arduino-ide-extension/src/node/arduino-ide-backend-module.ts +++ b/arduino-ide-extension/src/node/arduino-ide-backend-module.ts @@ -118,6 +118,7 @@ import { LocalDirectoryPluginDeployerResolverWithFallback, PluginDeployer_GH_12064, } from './theia/plugin-ext/plugin-deployer'; +import { SettingsReader } from './settings-reader'; export default new ContainerModule((bind, unbind, isBound, rebind) => { bind(BackendApplication).toSelf().inSingletonScope(); @@ -403,6 +404,8 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => { .toSelf() .inSingletonScope(); rebind(PluginDeployer).to(PluginDeployer_GH_12064).inSingletonScope(); + + bind(SettingsReader).toSelf().inSingletonScope(); }); function bindChildLogger(bind: interfaces.Bind, name: string): void { diff --git a/arduino-ide-extension/src/node/settings-reader.ts b/arduino-ide-extension/src/node/settings-reader.ts new file mode 100644 index 000000000..d6ab15811 --- /dev/null +++ b/arduino-ide-extension/src/node/settings-reader.ts @@ -0,0 +1,53 @@ +import { EnvVariablesServer } from '@theia/core/lib/common/env-variables'; +import { FileUri } from '@theia/core/lib/node/file-uri'; +import { inject, injectable } from '@theia/core/shared/inversify'; +import { promises as fs } from 'fs'; +import { + parse as parseJsonc, + ParseError, + printParseErrorCode, +} from 'jsonc-parser'; +import { join } from 'path'; +import { ErrnoException } from './utils/errors'; + +// Poor man's preferences on the backend. (https://github.com/arduino/arduino-ide/issues/1056#issuecomment-1153975064) +@injectable() +export class SettingsReader { + @inject(EnvVariablesServer) + private readonly envVariableServer: EnvVariablesServer; + + async read(): Promise | undefined> { + const configDirUri = await this.envVariableServer.getConfigDirUri(); + const configDirPath = FileUri.fsPath(configDirUri); + const settingsPath = join(configDirPath, 'settings.json'); + try { + const raw = await fs.readFile(settingsPath, { encoding: 'utf8' }); + return parse(raw); + } catch (err) { + if (ErrnoException.isENOENT(err)) { + return undefined; + } + } + } +} + +export function parse(raw: string): Record | undefined { + const errors: ParseError[] = []; + const settings = + parseJsonc(raw, errors, { + allowEmptyContent: true, + allowTrailingComma: true, + disallowComments: false, + }) ?? {}; + if (errors.length) { + console.error('Detected JSONC parser errors:'); + console.error('----- CONTENT START -----'); + console.error(raw); + console.error('----- CONTENT END -----'); + errors.forEach(({ error, offset }) => + console.error(` - ${printParseErrorCode(error)} at ${offset}`) + ); + return undefined; + } + return typeof settings === 'object' ? settings : undefined; +} diff --git a/arduino-ide-extension/src/node/sketches-service-impl.ts b/arduino-ide-extension/src/node/sketches-service-impl.ts index 42ca3e9a3..f4ff29db2 100644 --- a/arduino-ide-extension/src/node/sketches-service-impl.ts +++ b/arduino-ide-extension/src/node/sketches-service-impl.ts @@ -43,6 +43,7 @@ import { firstToUpperCase, startsWithUpperCase, } from '../common/utils'; +import { SettingsReader } from './settings-reader'; const RecentSketches = 'recent-sketches.json'; const DefaultIno = `void setup() { @@ -86,6 +87,9 @@ export class SketchesServiceImpl @inject(IsTempSketch) private readonly isTempSketch: IsTempSketch; + @inject(SettingsReader) + private readonly settingsReader: SettingsReader; + async getSketches({ uri }: { uri?: string }): Promise { const root = await this.root(uri); if (!root) { @@ -631,38 +635,11 @@ export class SketchesServiceImpl return crypto.createHash('md5').update(path).digest('hex').toUpperCase(); } - // Returns the default.ino from the settings or from default folder. - private async readSettings(): Promise | undefined> { - const configDirUri = await this.envVariableServer.getConfigDirUri(); - const configDirPath = FileUri.fsPath(configDirUri); - - try { - const raw = await fs.readFile(join(configDirPath, 'settings.json'), { - encoding: 'utf8', - }); - - return this.tryParse(raw); - } catch (err) { - if (ErrnoException.isENOENT(err)) { - return undefined; - } - throw err; - } - } - - private tryParse(raw: string): Record | undefined { - try { - return JSON.parse(raw); - } catch { - return undefined; - } - } - // Returns the default.ino from the settings or from default folder. private async loadInoContent(): Promise { if (!this.inoContent) { this.inoContent = new Deferred(); - const settings = await this.readSettings(); + const settings = await this.settingsReader.read(); if (settings) { const inoBlueprintPath = settings['arduino.sketch.inoBlueprint']; if (inoBlueprintPath && typeof inoBlueprintPath === 'string') { diff --git a/arduino-ide-extension/src/test/node/settings.reader.test.ts b/arduino-ide-extension/src/test/node/settings.reader.test.ts new file mode 100644 index 000000000..65e6b7a7e --- /dev/null +++ b/arduino-ide-extension/src/test/node/settings.reader.test.ts @@ -0,0 +1,45 @@ +import { expect } from 'chai'; +import { parse } from '../../node/settings-reader'; + +describe('settings-reader', () => { + describe('parse', () => { + it('should handle comments', () => { + const actual = parse(` +{ + "alma": "korte", + // comment + "szilva": false +}`); + expect(actual).to.be.deep.equal({ + alma: 'korte', + szilva: false, + }); + }); + + it('should handle trailing comma', () => { + const actual = parse(` +{ + "alma": "korte", + "szilva": 123, +}`); + expect(actual).to.be.deep.equal({ + alma: 'korte', + szilva: 123, + }); + }); + + it('should parse empty', () => { + const actual = parse(''); + expect(actual).to.be.deep.equal({}); + }); + + it('should parse to undefined when parse has failed', () => { + const actual = parse(` +{ + alma:: 'korte' + trash +}`); + expect(actual).to.be.undefined; + }); + }); +}); diff --git a/arduino-ide-extension/src/test/node/test-bindings.ts b/arduino-ide-extension/src/test/node/test-bindings.ts index 1aef2ff72..19b99f3a3 100644 --- a/arduino-ide-extension/src/test/node/test-bindings.ts +++ b/arduino-ide-extension/src/test/node/test-bindings.ts @@ -51,6 +51,7 @@ import { MonitorServiceFactory, MonitorServiceFactoryOptions, } from '../../node/monitor-service-factory'; +import { SettingsReader } from '../../node/settings-reader'; import { SketchesServiceImpl } from '../../node/sketches-service-impl'; import { EnvVariablesServer } from '../../node/theia/env-variables/env-variables-server'; @@ -277,6 +278,7 @@ export function createBaseContainer( bind(IsTempSketch).toSelf().inSingletonScope(); bind(SketchesServiceImpl).toSelf().inSingletonScope(); bind(SketchesService).toService(SketchesServiceImpl); + bind(SettingsReader).toSelf().inSingletonScope(); if (containerCustomizations) { containerCustomizations(bind, rebind); }