Skip to content

Commit fb10de1

Browse files
Akos Kittakittaakos
Akos Kitta
authored and
kittaakos
committed
fix: jsonc parsing in the IDE2 backend
Occurred when `settings.json` contained comments or a trailing comma. Closes #1945 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent 24dc0bb commit fb10de1

7 files changed

+118
-57
lines changed

‎arduino-ide-extension/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@
7878
"google-protobuf": "^3.20.1",
7979
"hash.js": "^1.1.7",
8080
"js-yaml": "^3.13.1",
81+
"jsonc-parser": "^2.2.0",
8182
"just-diff": "^5.1.1",
8283
"jwt-decode": "^3.1.2",
8384
"keytar": "7.2.0",

‎arduino-ide-extension/src/node/arduino-daemon-impl.ts

+9-29
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { join } from 'path';
2-
import { promises as fs } from 'fs';
32
import { inject, injectable, named } from '@theia/core/shared/inversify';
43
import { spawn, ChildProcess } from 'child_process';
54
import { FileUri } from '@theia/core/lib/node/file-uri';
@@ -16,7 +15,7 @@ import { BackendApplicationContribution } from '@theia/core/lib/node/backend-app
1615
import { ArduinoDaemon, NotificationServiceServer } from '../common/protocol';
1716
import { CLI_CONFIG } from './cli-config';
1817
import { getExecPath } from './exec-util';
19-
import { ErrnoException } from './utils/errors';
18+
import { SettingsReader } from './settings-reader';
2019

2120
@injectable()
2221
export class ArduinoDaemonImpl
@@ -32,6 +31,9 @@ export class ArduinoDaemonImpl
3231
@inject(NotificationServiceServer)
3332
private readonly notificationService: NotificationServiceServer;
3433

34+
@inject(SettingsReader)
35+
private readonly settingsReader: SettingsReader;
36+
3537
private readonly toDispose = new DisposableCollection();
3638
private readonly onDaemonStartedEmitter = new Emitter<string>();
3739
private readonly onDaemonStoppedEmitter = new Emitter<void>();
@@ -149,34 +151,12 @@ export class ArduinoDaemonImpl
149151
}
150152

151153
private async debugDaemon(): Promise<boolean> {
152-
// Poor man's preferences on the backend. (https://github.com/arduino/arduino-ide/issues/1056#issuecomment-1153975064)
153-
const configDirUri = await this.envVariablesServer.getConfigDirUri();
154-
const configDirPath = FileUri.fsPath(configDirUri);
155-
try {
156-
const raw = await fs.readFile(join(configDirPath, 'settings.json'), {
157-
encoding: 'utf8',
158-
});
159-
const json = this.tryParse(raw);
160-
if (json) {
161-
const value = json['arduino.cli.daemon.debug'];
162-
return typeof value === 'boolean' && !!value;
163-
}
164-
return false;
165-
} catch (error) {
166-
if (ErrnoException.isENOENT(error)) {
167-
return false;
168-
}
169-
throw error;
170-
}
171-
}
172-
173-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
174-
private tryParse(raw: string): any | undefined {
175-
try {
176-
return JSON.parse(raw);
177-
} catch {
178-
return undefined;
154+
const settings = await this.settingsReader.read();
155+
if (settings) {
156+
const value = settings['arduino.cli.daemon.debug'];
157+
return value === true;
179158
}
159+
return false;
180160
}
181161

182162
protected async spawnDaemonProcess(): Promise<{

‎arduino-ide-extension/src/node/arduino-ide-backend-module.ts

+3
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ import {
118118
LocalDirectoryPluginDeployerResolverWithFallback,
119119
PluginDeployer_GH_12064,
120120
} from './theia/plugin-ext/plugin-deployer';
121+
import { SettingsReader } from './settings-reader';
121122

122123
export default new ContainerModule((bind, unbind, isBound, rebind) => {
123124
bind(BackendApplication).toSelf().inSingletonScope();
@@ -403,6 +404,8 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
403404
.toSelf()
404405
.inSingletonScope();
405406
rebind(PluginDeployer).to(PluginDeployer_GH_12064).inSingletonScope();
407+
408+
bind(SettingsReader).toSelf().inSingletonScope();
406409
});
407410

408411
function bindChildLogger(bind: interfaces.Bind, name: string): void {

‎arduino-ide-extension/src/node/settings-reader.ts

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { EnvVariablesServer } from '@theia/core/lib/common/env-variables';
2+
import { FileUri } from '@theia/core/lib/node/file-uri';
3+
import { inject, injectable } from '@theia/core/shared/inversify';
4+
import { promises as fs } from 'fs';
5+
import {
6+
parse as parseJsonc,
7+
ParseError,
8+
printParseErrorCode,
9+
} from 'jsonc-parser';
10+
import { join } from 'path';
11+
import { ErrnoException } from './utils/errors';
12+
13+
// Poor man's preferences on the backend. (https://github.com/arduino/arduino-ide/issues/1056#issuecomment-1153975064)
14+
@injectable()
15+
export class SettingsReader {
16+
@inject(EnvVariablesServer)
17+
private readonly envVariableServer: EnvVariablesServer;
18+
19+
async read(): Promise<Record<string, unknown> | undefined> {
20+
const configDirUri = await this.envVariableServer.getConfigDirUri();
21+
const configDirPath = FileUri.fsPath(configDirUri);
22+
const settingsPath = join(configDirPath, 'settings.json');
23+
try {
24+
const raw = await fs.readFile(settingsPath, { encoding: 'utf8' });
25+
return parse(raw);
26+
} catch (err) {
27+
if (ErrnoException.isENOENT(err)) {
28+
return undefined;
29+
}
30+
}
31+
}
32+
}
33+
34+
export function parse(raw: string): Record<string, unknown> | undefined {
35+
const errors: ParseError[] = [];
36+
const settings =
37+
parseJsonc(raw, errors, {
38+
allowEmptyContent: true,
39+
allowTrailingComma: true,
40+
disallowComments: false,
41+
}) ?? {};
42+
if (errors.length) {
43+
console.error('Detected JSONC parser errors:');
44+
console.error('----- CONTENT START -----');
45+
console.error(raw);
46+
console.error('----- CONTENT END -----');
47+
errors.forEach(({ error, offset }) =>
48+
console.error(` - ${printParseErrorCode(error)} at ${offset}`)
49+
);
50+
return undefined;
51+
}
52+
return typeof settings === 'object' ? settings : undefined;
53+
}

‎arduino-ide-extension/src/node/sketches-service-impl.ts

+5-28
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
firstToUpperCase,
4444
startsWithUpperCase,
4545
} from '../common/utils';
46+
import { SettingsReader } from './settings-reader';
4647

4748
const RecentSketches = 'recent-sketches.json';
4849
const DefaultIno = `void setup() {
@@ -86,6 +87,9 @@ export class SketchesServiceImpl
8687
@inject(IsTempSketch)
8788
private readonly isTempSketch: IsTempSketch;
8889

90+
@inject(SettingsReader)
91+
private readonly settingsReader: SettingsReader;
92+
8993
async getSketches({ uri }: { uri?: string }): Promise<SketchContainer> {
9094
const root = await this.root(uri);
9195
if (!root) {
@@ -631,38 +635,11 @@ export class SketchesServiceImpl
631635
return crypto.createHash('md5').update(path).digest('hex').toUpperCase();
632636
}
633637

634-
// Returns the default.ino from the settings or from default folder.
635-
private async readSettings(): Promise<Record<string, unknown> | undefined> {
636-
const configDirUri = await this.envVariableServer.getConfigDirUri();
637-
const configDirPath = FileUri.fsPath(configDirUri);
638-
639-
try {
640-
const raw = await fs.readFile(join(configDirPath, 'settings.json'), {
641-
encoding: 'utf8',
642-
});
643-
644-
return this.tryParse(raw);
645-
} catch (err) {
646-
if (ErrnoException.isENOENT(err)) {
647-
return undefined;
648-
}
649-
throw err;
650-
}
651-
}
652-
653-
private tryParse(raw: string): Record<string, unknown> | undefined {
654-
try {
655-
return JSON.parse(raw);
656-
} catch {
657-
return undefined;
658-
}
659-
}
660-
661638
// Returns the default.ino from the settings or from default folder.
662639
private async loadInoContent(): Promise<string> {
663640
if (!this.inoContent) {
664641
this.inoContent = new Deferred<string>();
665-
const settings = await this.readSettings();
642+
const settings = await this.settingsReader.read();
666643
if (settings) {
667644
const inoBlueprintPath = settings['arduino.sketch.inoBlueprint'];
668645
if (inoBlueprintPath && typeof inoBlueprintPath === 'string') {

‎arduino-ide-extension/src/test/node/settings.reader.test.ts

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { expect } from 'chai';
2+
import { parse } from '../../node/settings-reader';
3+
4+
describe('settings-reader', () => {
5+
describe('parse', () => {
6+
it('should handle comments', () => {
7+
const actual = parse(`
8+
{
9+
"alma": "korte",
10+
// comment
11+
"szilva": false
12+
}`);
13+
expect(actual).to.be.deep.equal({
14+
alma: 'korte',
15+
szilva: false,
16+
});
17+
});
18+
19+
it('should handle trailing comma', () => {
20+
const actual = parse(`
21+
{
22+
"alma": "korte",
23+
"szilva": 123,
24+
}`);
25+
expect(actual).to.be.deep.equal({
26+
alma: 'korte',
27+
szilva: 123,
28+
});
29+
});
30+
31+
it('should parse empty', () => {
32+
const actual = parse('');
33+
expect(actual).to.be.deep.equal({});
34+
});
35+
36+
it('should parse to undefined when parse has failed', () => {
37+
const actual = parse(`
38+
{
39+
alma:: 'korte'
40+
trash
41+
}`);
42+
expect(actual).to.be.undefined;
43+
});
44+
});
45+
});

‎arduino-ide-extension/src/test/node/test-bindings.ts

+2
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
MonitorServiceFactory,
5252
MonitorServiceFactoryOptions,
5353
} from '../../node/monitor-service-factory';
54+
import { SettingsReader } from '../../node/settings-reader';
5455
import { SketchesServiceImpl } from '../../node/sketches-service-impl';
5556
import { EnvVariablesServer } from '../../node/theia/env-variables/env-variables-server';
5657

@@ -277,6 +278,7 @@ export function createBaseContainer(
277278
bind(IsTempSketch).toSelf().inSingletonScope();
278279
bind(SketchesServiceImpl).toSelf().inSingletonScope();
279280
bind(SketchesService).toService(SketchesServiceImpl);
281+
bind(SettingsReader).toSelf().inSingletonScope();
280282
if (containerCustomizations) {
281283
containerCustomizations(bind, rebind);
282284
}

0 commit comments

Comments
 (0)