adopt json.ParseOptions.allowEmptyContent and getNodeType

aeschli · aeschli · commit ce9f8e1c94fe · 2019-10-25T10:16:53.000+02:00
diff --git a/src/vs/platform/diagnostics/node/diagnosticsService.ts b/src/vs/platform/diagnostics/node/diagnosticsService.ts
@@ -7,7 +7,7 @@ import { virtualMachineHint } from 'vs/base/node/id';
 import { IMachineInfo, WorkspaceStats, WorkspaceStatItem, PerformanceInfo, SystemInfo, IRemoteDiagnosticInfo, IRemoteDiagnosticError, isRemoteDiagnosticError, IWorkspaceInformation } from 'vs/platform/diagnostics/common/diagnostics';
 import { readdir, stat, exists, readFile } from 'fs';
 import { join, basename } from 'vs/base/common/path';
-import { parse, ParseError } from 'vs/base/common/json';
+import { parse, ParseError, getNodeType } from 'vs/base/common/json';
 import { listProcesses } from 'vs/base/node/ps';
 import product from 'vs/platform/product/common/product';
 import { repeat, pad } from 'vs/base/common/strings';
@@ -223,7 +223,7 @@ export function collectLaunchConfigs(folder: string): Promise<WorkspaceStatItem[
 						return resolve([]);
 					}
 
-					if (json['configurations']) {
+					if (getNodeType(json) === 'object' && json['configurations']) {
 						for (const each of json['configurations']) {
 							const type = each['type'];
 							if (type) {
diff --git a/src/vs/platform/userDataSync/common/settingsSync.ts b/src/vs/platform/userDataSync/common/settingsSync.ts
@@ -188,7 +188,7 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser {
 
 	private hasErrors(content: string): boolean {
 		const parseErrors: ParseError[] = [];
-		parse(content, parseErrors);
+		parse(content, parseErrors, { allowEmptyContent: true, allowTrailingComma: true });
 		return parseErrors.length > 0;
 	}
 
@@ -248,13 +248,23 @@ export class SettingsSynchroniser extends Disposable implements ISynchroniser {
 	}
 
 	private getIgnoredSettings(settingsContent?: string): string[] {
-		const value: string[] = (settingsContent ? parse(settingsContent)['configurationSync.settingsToIgnore'] : this.configurationService.getValue<string[]>('configurationSync.settingsToIgnore')) || [];
+		let value: string[] = [];
+		if (settingsContent) {
+			const setting = parse(settingsContent);
+			if (setting) {
+				value = setting['configurationSync.settingsToIgnore'];
+			}
+		} else {
+			value = this.configurationService.getValue<string[]>('configurationSync.settingsToIgnore');
+		}
 		const added: string[] = [], removed: string[] = [];
-		for (const key of value) {
-			if (startsWith(key, '-')) {
-				removed.push(key.substring(1));
-			} else {
-				added.push(key);
+		if (Array.isArray(value)) {
+			for (const key of value) {
+				if (startsWith(key, '-')) {
+					removed.push(key.substring(1));
+				} else {
+					added.push(key);
+				}
 			}
 		}
 		return [...DEFAULT_IGNORED_SETTINGS, ...added].filter(setting => removed.indexOf(setting) === -1);
diff --git a/src/vs/platform/workspaces/common/workspaces.ts b/src/vs/platform/workspaces/common/workspaces.ts
@@ -285,12 +285,9 @@ function doParseStoredWorkspace(path: URI, contents: string): IStoredWorkspace {
 	let storedWorkspace: IStoredWorkspace = json.parse(contents); // use fault tolerant parser
 
 	// Filter out folders which do not have a path or uri set
-	if (Array.isArray(storedWorkspace.folders)) {
+	if (storedWorkspace && Array.isArray(storedWorkspace.folders)) {
 		storedWorkspace.folders = storedWorkspace.folders.filter(folder => isStoredWorkspaceFolder(folder));
-	}
-
-	// Validate
-	if (!Array.isArray(storedWorkspace.folders)) {
+	} else {
 		throw new Error(`${path} looks like an invalid workspace file.`);
 	}
 
diff --git a/src/vs/platform/workspaces/electron-main/workspacesMainService.ts b/src/vs/platform/workspaces/electron-main/workspacesMainService.ts
@@ -130,12 +130,9 @@ export class WorkspacesMainService extends Disposable implements IWorkspacesMain
 		let storedWorkspace: IStoredWorkspace = json.parse(contents); // use fault tolerant parser
 
 		// Filter out folders which do not have a path or uri set
-		if (Array.isArray(storedWorkspace.folders)) {
+		if (storedWorkspace && Array.isArray(storedWorkspace.folders)) {
 			storedWorkspace.folders = storedWorkspace.folders.filter(folder => isStoredWorkspaceFolder(folder));
-		}
-
-		// Validate
-		if (!Array.isArray(storedWorkspace.folders)) {
+		} else {
 			throw new Error(`${path.toString()} looks like an invalid workspace file.`);
 		}
 
diff --git a/src/vs/workbench/contrib/codeEditor/browser/languageConfigurationExtensionPoint.ts b/src/vs/workbench/contrib/codeEditor/browser/languageConfigurationExtensionPoint.ts
@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as nls from 'vs/nls';
-import { ParseError, parse } from 'vs/base/common/json';
+import { ParseError, parse, getNodeType } from 'vs/base/common/json';
 import { IJSONSchema } from 'vs/base/common/jsonSchema';
 import * as types from 'vs/base/common/types';
 import { URI } from 'vs/base/common/uri';
@@ -91,10 +91,14 @@ export class LanguageConfigurationFileHandler {
 	private _handleConfigFile(languageIdentifier: LanguageIdentifier, configFileLocation: URI): void {
 		this._fileService.readFile(configFileLocation).then((contents) => {
 			const errors: ParseError[] = [];
-			const configuration = <ILanguageConfiguration>parse(contents.value.toString(), errors);
+			let configuration = <ILanguageConfiguration>parse(contents.value.toString(), errors);
 			if (errors.length) {
 				console.error(nls.localize('parseErrors', "Errors parsing {0}: {1}", configFileLocation.toString(), errors.map(e => (`[${e.offset}, ${e.length}] ${getParseErrorMessage(e.error)}`)).join('\n')));
 			}
+			if (getNodeType(configuration) !== 'object') {
+				console.error(nls.localize('formatError', "{0}: Invalid format, JSON object expected.", configFileLocation.toString()));
+				configuration = {};
+			}
 			this._handleConfig(languageIdentifier, configuration);
 		}, (err) => {
 			console.error(err);
diff --git a/src/vs/workbench/contrib/extensions/browser/extensionsActions.ts b/src/vs/workbench/contrib/extensions/browser/extensionsActions.ts
@@ -2101,7 +2101,7 @@ export abstract class AbstractConfigureRecommendedExtensionsAction extends Actio
 	protected getWorkspaceFolderExtensionsConfigContent(extensionsFileResource: URI): Promise<IExtensionsConfigContent> {
 		return Promise.resolve(this.fileService.readFile(extensionsFileResource))
 			.then(content => {
-				return (<IExtensionsConfigContent>json.parse(content.value.toString()));
+				return (<IExtensionsConfigContent>json.parse(content.value.toString()) || {});
 			}, err => ({ recommendations: [], unwantedRecommendations: [] }));
 	}
 
diff --git a/src/vs/workbench/contrib/snippets/browser/snippetsFile.ts b/src/vs/workbench/contrib/snippets/browser/snippetsFile.ts
@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { parse as jsonParse } from 'vs/base/common/json';
+import { parse as jsonParse, getNodeType } from 'vs/base/common/json';
 import { forEach } from 'vs/base/common/collections';
 import { localize } from 'vs/nls';
 import { extname, basename } from 'vs/base/common/path';
@@ -203,7 +203,7 @@ export class SnippetFile {
 		if (!this._loadPromise) {
 			this._loadPromise = Promise.resolve(this._fileService.readFile(this.location)).then(content => {
 				const data = <JsonSerializedSnippets>jsonParse(content.value.toString());
-				if (typeof data === 'object') {
+				if (getNodeType(data) === 'object') {
 					forEach(data, entry => {
 						const { key: name, value: scopeOrTemplate } = entry;
 						if (isJsonSerializedSnippet(scopeOrTemplate)) {
diff --git a/src/vs/workbench/contrib/url/common/trustedDomainsFileSystemProvider.ts b/src/vs/workbench/contrib/url/common/trustedDomainsFileSystemProvider.ts
@@ -121,7 +121,7 @@ export class TrustedDomainsFileSystemProvider implements IFileSystemProvider, IW
 			this.storageService.store('http.linkProtectionTrustedDomainsContent', trustedDomainsContent, StorageScope.GLOBAL);
 			this.storageService.store(
 				'http.linkProtectionTrustedDomains',
-				JSON.stringify(trustedDomains),
+				JSON.stringify(trustedDomains) || '',
 				StorageScope.GLOBAL
 			);
 		} catch (err) { }
diff --git a/src/vs/workbench/services/configuration/common/configurationEditingService.ts b/src/vs/workbench/services/configuration/common/configurationEditingService.ts
@@ -409,7 +409,7 @@ export class ConfigurationEditingService {
 			return false;
 		}
 		const parseErrors: json.ParseError[] = [];
-		json.parse(model.getValue(), parseErrors);
+		json.parse(model.getValue(), parseErrors, { allowTrailingComma: true, allowEmptyContent: true });
 		return parseErrors.length > 0;
 	}
 
diff --git a/src/vs/workbench/services/configuration/common/jsonEditingService.ts b/src/vs/workbench/services/configuration/common/jsonEditingService.ts
@@ -98,7 +98,7 @@ export class JSONEditingService implements IJSONEditingService {
 
 	private hasParseErrors(model: ITextModel): boolean {
 		const parseErrors: json.ParseError[] = [];
-		json.parse(model.getValue(), parseErrors);
+		json.parse(model.getValue(), parseErrors, { allowTrailingComma: true, allowEmptyContent: true });
 		return parseErrors.length > 0;
 	}
 
diff --git a/src/vs/workbench/services/extensions/node/extensionPoints.ts b/src/vs/workbench/services/extensions/node/extensionPoints.ts
@@ -51,7 +51,7 @@ class ExtensionManifestParser extends ExtensionManifestHandler {
 		return pfs.readFile(this._absoluteManifestPath).then((manifestContents) => {
 			const errors: json.ParseError[] = [];
 			const manifest = json.parse(manifestContents.toString(), errors);
-			if (!!manifest && errors.length === 0) {
+			if (errors.length === 0 && json.getNodeType(manifest) === 'object') {
 				if (manifest.__metadata) {
 					manifest.uuid = manifest.__metadata.id;
 				}
@@ -104,6 +104,9 @@ class ExtensionManifestNLSReplacer extends ExtensionManifestHandler {
 				this._log.error(this._absoluteFolderPath, nls.localize('jsonsParseReportErrors', "Failed to parse {0}: {1}.", localized, getParseErrorMessage(error.error)));
 			});
 		};
+		const reportInvalidFormat = (localized: string | null): void => {
+			this._log.error(this._absoluteFolderPath, nls.localize('jsonInvalidFormat', "Invalid format {0}: JSON object expected.", localized));
+		};
 
 		let extension = path.extname(this._absoluteManifestPath);
 		let basename = this._absoluteManifestPath.substr(0, this._absoluteManifestPath.length - extension.length);
@@ -118,6 +121,9 @@ class ExtensionManifestNLSReplacer extends ExtensionManifestHandler {
 				if (errors.length > 0) {
 					reportErrors(translationPath, errors);
 					return { values: undefined, default: `${basename}.nls.json` };
+				} else if (json.getNodeType(translationBundle) !== 'object') {
+					reportInvalidFormat(translationPath);
+					return { values: undefined, default: `${basename}.nls.json` };
 				} else {
 					let values = translationBundle.contents ? translationBundle.contents.package : undefined;
 					return { values: values, default: `${basename}.nls.json` };
@@ -140,6 +146,9 @@ class ExtensionManifestNLSReplacer extends ExtensionManifestHandler {
 						if (errors.length > 0) {
 							reportErrors(messageBundle.localized, errors);
 							return { values: undefined, default: messageBundle.original };
+						} else if (json.getNodeType(messages) !== 'object') {
+							reportInvalidFormat(messageBundle.localized);
+							return { values: undefined, default: messageBundle.original };
 						}
 						return { values: messages, default: messageBundle.original };
 					}, (err) => {
@@ -161,6 +170,9 @@ class ExtensionManifestNLSReplacer extends ExtensionManifestHandler {
 				if (errors.length > 0) {
 					reportErrors(localizedMessages.default, errors);
 					return extensionDescription;
+				} else if (json.getNodeType(localizedMessages) !== 'object') {
+					reportInvalidFormat(localizedMessages.default);
+					return extensionDescription;
 				}
 				const localized = localizedMessages.values || Object.create(null);
 				ExtensionManifestNLSReplacer._replaceNLStrings(this._nlsConfig, extensionDescription, localized, defaults, this._log, this._absoluteFolderPath);
diff --git a/src/vs/workbench/services/keybinding/browser/keymapService.ts b/src/vs/workbench/services/keybinding/browser/keymapService.ts
@@ -19,7 +19,7 @@ import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { URI } from 'vs/base/common/uri';
 import { IFileService } from 'vs/platform/files/common/files';
 import { RunOnceScheduler } from 'vs/base/common/async';
-import { parse } from 'vs/base/common/json';
+import { parse, getNodeType } from 'vs/base/common/json';
 import * as objects from 'vs/base/common/objects';
 import { IEnvironmentService } from 'vs/platform/environment/common/environment';
 import { Registry } from 'vs/platform/registry/common/platform';
@@ -482,9 +482,13 @@ class UserKeyboardLayout extends Disposable {
 		try {
 			const content = await this.fileService.readFile(this.keyboardLayoutResource);
 			const value = parse(content.value.toString());
-			const layoutInfo = value.layout;
-			const mappings = value.rawMapping;
-			this._keyboardLayout = KeymapInfo.createKeyboardLayoutFromDebugInfo(layoutInfo, mappings, true);
+			if (getNodeType(value) === 'object') {
+				const layoutInfo = value.layout;
+				const mappings = value.rawMapping;
+				this._keyboardLayout = KeymapInfo.createKeyboardLayoutFromDebugInfo(layoutInfo, mappings, true);
+			} else {
+				this._keyboardLayout = null;
+			}
 		} catch (e) {
 			this._keyboardLayout = null;
 		}
diff --git a/src/vs/workbench/services/themes/browser/fileIconThemeData.ts b/src/vs/workbench/services/themes/browser/fileIconThemeData.ts
@@ -189,8 +189,10 @@ function _loadIconThemeDocument(fileService: IFileService, location: URI): Promi
 	return fileService.readFile(location).then((content) => {
 		let errors: Json.ParseError[] = [];
 		let contentValue = Json.parse(content.value.toString(), errors);
-		if (errors.length > 0 || !contentValue) {
+		if (errors.length > 0) {
 			return Promise.reject(new Error(nls.localize('error.cannotparseicontheme', "Problems parsing file icons file: {0}", errors.map(e => getParseErrorMessage(e.error)).join(', '))));
+		} else if (Json.getNodeType(contentValue) !== 'object') {
+			return Promise.reject(new Error(nls.localize('error.invalidformat', "Invalid format for file icons theme file: Object expected.")));
 		}
 		return Promise.resolve(contentValue);
 	});
diff --git a/src/vs/workbench/services/themes/common/colorThemeData.ts b/src/vs/workbench/services/themes/common/colorThemeData.ts
@@ -302,6 +302,8 @@ function _loadColorTheme(fileService: IFileService, themeLocation: URI, resultRu
 			let contentValue = Json.parse(content.value.toString(), errors);
 			if (errors.length > 0) {
 				return Promise.reject(new Error(nls.localize('error.cannotparsejson', "Problems parsing JSON theme file: {0}", errors.map(e => getParseErrorMessage(e.error)).join(', '))));
+			} else if (Json.getNodeType(contentValue) !== 'object') {
+				return Promise.reject(new Error(nls.localize('error.invalidformat', "Invalid format for JSON theme file: Object expected.")));
 			}
 			let includeCompletes: Promise<any> = Promise.resolve(null);
 			if (contentValue.include) {

Original file line number	Diff line number	Diff line change
`@@ -2101,7 +2101,7 @@ export abstract class AbstractConfigureRecommendedExtensionsAction extends Actio`
`2101`	`2101`	`protected getWorkspaceFolderExtensionsConfigContent(extensionsFileResource: URI): Promise<IExtensionsConfigContent> {`
`2102`	`2102`	`return Promise.resolve(this.fileService.readFile(extensionsFileResource))`
`2103`	`2103`	`.then(content => {`
`2104`		`- return (<IExtensionsConfigContent>json.parse(content.value.toString()));`
	`2104`	`+ return (<IExtensionsConfigContent>json.parse(content.value.toString()) \|\| {});`
`2105`	`2105`	`}, err => ({ recommendations: [], unwantedRecommendations: [] }));`
`2106`	`2106`	`}`
`2107`	`2107`
Original file line number	Diff line number	Diff line change
`@@ -409,7 +409,7 @@ export class ConfigurationEditingService {`
`409`	`409`	`return false;`
`410`	`410`	`}`
`411`	`411`	`const parseErrors: json.ParseError[] = [];`
`412`		`- json.parse(model.getValue(), parseErrors);`
	`412`	`+ json.parse(model.getValue(), parseErrors, { allowTrailingComma: true, allowEmptyContent: true });`
`413`	`413`	`return parseErrors.length > 0;`
`414`	`414`	`}`
`415`	`415`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ export class JSONEditingService implements IJSONEditingService {`
`98`	`98`
`99`	`99`	`private hasParseErrors(model: ITextModel): boolean {`
`100`	`100`	`const parseErrors: json.ParseError[] = [];`
`101`		`- json.parse(model.getValue(), parseErrors);`
	`101`	`+ json.parse(model.getValue(), parseErrors, { allowTrailingComma: true, allowEmptyContent: true });`
`102`	`102`	`return parseErrors.length > 0;`
`103`	`103`	`}`
`104`	`104`