product icon file validations, fixes #94209

src/vs/platform/theme/common/iconRegistry.ts

@@ -56,6 +56,11 @@ export interface IIconRegistry {
 	 */
 	getIcons(): IconContribution[];
 
+	/**
+	 * Get the icon for the given id
+	 */
+	getIcon(id: string): IconContribution | undefined;
+
 	/**
 	 * JSON schema for an object to assign icon values to one of the color contributions.
 	 */
@@ -130,6 +135,10 @@ class IconRegistry implements IIconRegistry {
 		return Object.keys(this.iconsById).map(id => this.iconsById[id]);
 	}
 
+	public getIcon(id: string): IconContribution | undefined {
+		return this.iconsById[id];
+	}
+
 	public getIconSchema(): IJSONSchema {
 		return this.iconSchema;
 	}

src/vs/workbench/services/themes/browser/productIconThemeData.ts

@@ -14,6 +14,11 @@ import { getParseErrorMessage } from 'vs/base/common/jsonErrorMessages';
 import { asCSSUrl } from 'vs/base/browser/dom';
 import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
 import { DEFAULT_PRODUCT_ICON_THEME_SETTING_VALUE } from 'vs/workbench/services/themes/common/themeConfiguration';
+import { fontIdRegex, fontWeightRegex, fontStyleRegex } from 'vs/workbench/services/themes/common/productIconThemeSchema';
+import { isString } from 'vs/base/common/types';
+import { ILogService } from 'vs/platform/log/common/log';
+import { getIconRegistry } from 'vs/platform/theme/common/iconRegistry';
+import { ThemeIcon } from 'vs/platform/theme/common/themeService';
 
 const PERSISTED_PRODUCT_ICON_THEME_STORAGE_KEY = 'productIconThemeData';
 
@@ -38,22 +43,26 @@ export class ProductIconThemeData implements IWorkbenchProductIconTheme {
 		this.isLoaded = false;
 	}
 
-	public ensureLoaded(fileService: IFileService): Promise<string | undefined> {
-		return !this.isLoaded ? this.load(fileService) : Promise.resolve(this.styleSheetContent);
+	public ensureLoaded(fileService: IFileService, logService: ILogService): Promise<string | undefined> {
+		return !this.isLoaded ? this.load(fileService, logService) : Promise.resolve(this.styleSheetContent);
 	}
 
-	public reload(fileService: IFileService): Promise<string | undefined> {
-		return this.load(fileService);
+	public reload(fileService: IFileService, logService: ILogService): Promise<string | undefined> {
+		return this.load(fileService, logService);
 	}
 
-	private load(fileService: IFileService): Promise<string | undefined> {
-		if (!this.location) {
+	private load(fileService: IFileService, logService: ILogService): Promise<string | undefined> {
+		const location = this.location;
+		if (!location) {
 			return Promise.resolve(this.styleSheetContent);
 		}
-		return _loadProductIconThemeDocument(fileService, this.location).then(iconThemeDocument => {
-			const result = _processIconThemeDocument(this.id, this.location!, iconThemeDocument);
+		return _loadProductIconThemeDocument(fileService, location).then(iconThemeDocument => {
+			const result = _processIconThemeDocument(this.id, location, iconThemeDocument);
 			this.styleSheetContent = result.content;
 			this.isLoaded = true;
+			if (result.warnings.length) {
+				logService.error(nls.localize('error.parseicondefs', "Problems processing product icons definitions in {0}:\n{1}", location.toString(), result.warnings.join('\n')));
+			}
 			return this.styleSheetContent;
 		});
 	}
@@ -174,9 +183,10 @@ function _loadProductIconThemeDocument(fileService: IFileService, location: URI)
 	});
 }
 
-function _processIconThemeDocument(id: string, iconThemeDocumentLocation: URI, iconThemeDocument: ProductIconThemeDocument): { content: string; } {
+function _processIconThemeDocument(id: string, iconThemeDocumentLocation: URI, iconThemeDocument: ProductIconThemeDocument): { content: string; warnings: string[] } {
 
-	const result = { content: '' };
+	const warnings: string[] = [];
+	const result = { content: '', warnings };
 
 	if (!iconThemeDocument.iconDefinitions || !Array.isArray(iconThemeDocument.fonts) || !iconThemeDocument.fonts.length) {
 		return result;
@@ -187,22 +197,72 @@ function _processIconThemeDocument(id: string, iconThemeDocumentLocation: URI, i
 		return resources.joinPath(iconThemeDocumentLocationDirname, path);
 	}
 
-	let cssRules: string[] = [];
+	const cssRules: string[] = [];
 
-	let fonts = iconThemeDocument.fonts;
+	const fonts = iconThemeDocument.fonts;
+	const fontIdMapping: { [id: string]: string } = {};
 	for (const font of fonts) {
 		const src = font.src.map(l => `${asCSSUrl(resolvePath(l.path))} format('${l.format}')`).join(', ');
-		cssRules.push(`@font-face { src: ${src}; font-family: '${font.id}'; font-weight: ${font.weight}; font-style: ${font.style}; }`);
+		if (isString(font.id) && font.id.match(fontIdRegex)) {
+			const fontId = `pi-` + font.id;
+			fontIdMapping[font.id] = fontId;
+
+			let fontWeight = '';
+			if (isString(font.weight) && font.weight.match(fontWeightRegex)) {
+				fontWeight = `font-weight: ${font.weight};`;
+			} else {
+				warnings.push(nls.localize('error.fontWeight', 'Invalid font weight in font \'{0}\'. Ignoring setting.', font.id));
+			}
+
+			let fontStyle = '';
+			if (isString(font.style) && font.style.match(fontStyleRegex)) {
+				fontStyle = `font-style: ${font.style};`;
+			} else {
+				warnings.push(nls.localize('error.fontStyle', 'Invalid font style in font \'{0}\'. Ignoring setting.', font.id));
+			}
+
+			cssRules.push(`@font-face { src: ${src}; font-family: '${fontId}';${fontWeight}${fontStyle} }`);
+		} else {
+			warnings.push(nls.localize('error.fontId', 'Missing or invalid font id \'{0}\'. Skipping font definition.', font.id));
+		}
 	}
 
-	let primaryFontId = fonts[0].id;
-	let iconDefinitions = iconThemeDocument.iconDefinitions;
-	for (const iconId in iconThemeDocument.iconDefinitions) {
-		const definition = iconDefinitions[iconId];
-		if (definition && definition.fontCharacter) {
-			cssRules.push(`.codicon-${iconId}:before { content: '${definition.fontCharacter}' !important; font-family: ${definition.fontId || primaryFontId} !important; }`);
+	const primaryFontId = fonts.length > 0 ? fontIdMapping[fonts[0].id] : '';
+
+	const iconDefinitions = iconThemeDocument.iconDefinitions;
+	const iconRegistry = getIconRegistry();
+
+
+	for (let iconContribution of iconRegistry.getIcons()) {
+		const iconId = iconContribution.id;
+
+		let definition = iconDefinitions[iconId];
+
+		// look if an inherited icon has a definition
+		while (!definition && ThemeIcon.isThemeIcon(iconContribution.defaults)) {
+			const ic = iconRegistry.getIcon(iconContribution.defaults.id);
+			if (ic) {
+				definition = iconDefinitions[ic.id];
+				iconContribution = ic;
+			} else {
+				break;
+			}
+		}
+
+		if (definition) {
+			if (isString(definition.fontCharacter)) {
+				const fontId = definition.fontId !== undefined ? fontIdMapping[definition.fontId] : primaryFontId;
+				if (fontId) {
+					cssRules.push(`.codicon-${iconId}:before { content: '${definition.fontCharacter}' !important; font-family: ${fontId} !important; }`);
+				} else {
+					warnings.push(nls.localize('error.icon.fontId', 'Skipping icon definition \'{0}\'. Unknown font.', iconId));
+				}
+			} else {
+				warnings.push(nls.localize('error.icon.fontCharacter', 'Skipping icon definition \'{0}\'. Unknown fontCharacter.', iconId));
+			}
 		}
 	}
 	result.content = cssRules.join('\n');
 	return result;
 }
+

src/vs/workbench/services/themes/browser/workbenchThemeService.ts

@@ -32,6 +32,7 @@ import { ThemeRegistry, registerColorThemeExtensionPoint, registerFileIconThemeE
 import { updateColorThemeConfigurationSchemas, updateFileIconThemeConfigurationSchemas, ThemeConfiguration, updateProductIconThemeConfigurationSchemas } from 'vs/workbench/services/themes/common/themeConfiguration';
 import { ProductIconThemeData, DEFAULT_PRODUCT_ICON_THEME_ID } from 'vs/workbench/services/themes/browser/productIconThemeData';
 import { registerProductIconThemeSchemas } from 'vs/workbench/services/themes/common/productIconThemeSchema';
+import { ILogService } from 'vs/platform/log/common/log';
 
 // implementation
 
@@ -97,7 +98,8 @@ export class WorkbenchThemeService implements IWorkbenchThemeService {
 		@IWorkbenchEnvironmentService private readonly environmentService: IWorkbenchEnvironmentService,
 		@IFileService private readonly fileService: IFileService,
 		@IExtensionResourceLoaderService private readonly extensionResourceLoaderService: IExtensionResourceLoaderService,
-		@IWorkbenchLayoutService readonly layoutService: IWorkbenchLayoutService
+		@IWorkbenchLayoutService readonly layoutService: IWorkbenchLayoutService,
+		@ILogService private readonly logService: ILogService
 	) {
 		this.container = layoutService.getWorkbenchContainer();
 		this.settings = new ThemeConfiguration(configurationService);
@@ -571,7 +573,7 @@ export class WorkbenchThemeService implements IWorkbenchThemeService {
 		}
 
 		const newThemeData = await this.productIconThemeRegistry.findThemeById(iconTheme) || ProductIconThemeData.defaultTheme;
-		await newThemeData.ensureLoaded(this.fileService);
+		await newThemeData.ensureLoaded(this.fileService, this.logService);
 
 		this.applyAndSetProductIconTheme(newThemeData);
 
@@ -585,7 +587,7 @@ export class WorkbenchThemeService implements IWorkbenchThemeService {
 	}
 
 	private async reloadCurrentProductIconTheme() {
-		await this.currentProductIconTheme.reload(this.fileService);
+		await this.currentProductIconTheme.reload(this.fileService, this.logService);
 		this.applyAndSetProductIconTheme(this.currentProductIconTheme);
 	}
 

src/vs/workbench/services/themes/common/fileIconThemeSchema.ts

@@ -7,6 +7,7 @@ import * as nls from 'vs/nls';
 import { Registry } from 'vs/platform/registry/common/platform';
 import { Extensions as JSONExtensions, IJSONContributionRegistry } from 'vs/platform/jsonschemas/common/jsonContributionRegistry';
 import { IJSONSchema } from 'vs/base/common/jsonSchema';
+import { fontWeightRegex, fontStyleRegex, fontSizeRegex, fontIdRegex } from 'vs/workbench/services/themes/common/productIconThemeSchema';
 
 const schemaId = 'vscode://schemas/icon-theme';
 const schema: IJSONSchema = {
@@ -110,7 +111,9 @@ const schema: IJSONSchema = {
 				properties: {
 					id: {
 						type: 'string',
-						description: nls.localize('schema.id', 'The ID of the font.')
+						description: nls.localize('schema.id', 'The ID of the font.'),
+						pattern: fontIdRegex,
+						patternErrorMessage: nls.localize('schema.id.formatError', 'The ID must only contain letter, numbers, underscore and minus.')
 					},
 					src: {
 						type: 'array',
@@ -124,7 +127,8 @@ const schema: IJSONSchema = {
 								},
 								format: {
 									type: 'string',
-									description: nls.localize('schema.font-format', 'The format of the font.')
+									description: nls.localize('schema.font-format', 'The format of the font.'),
+									enum: ['woff', 'woff2', 'truetype', 'opentype', 'embedded-opentype', 'svg']
 								}
 							},
 							required: [
@@ -135,15 +139,18 @@ const schema: IJSONSchema = {
 					},
 					weight: {
 						type: 'string',
-						description: nls.localize('schema.font-weight', 'The weight of the font.')
+						description: nls.localize('schema.font-weight', 'The weight of the font. See https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight for valid values.'),
+						pattern: fontWeightRegex
 					},
 					style: {
 						type: 'string',
-						description: nls.localize('schema.font-sstyle', 'The style of the font.')
+						description: nls.localize('schema.font-style', 'The style of the font. See https://developer.mozilla.org/en-US/docs/Web/CSS/font-style for valid values.'),
+						pattern: fontStyleRegex
 					},
 					size: {
 						type: 'string',
-						description: nls.localize('schema.font-size', 'The default size of the font.')
+						description: nls.localize('schema.font-size', 'The default size of the font. See https://developer.mozilla.org/en-US/docs/Web/CSS/font-size for valid values.'),
+						pattern: fontSizeRegex
 					}
 				},
 				required: [
@@ -174,7 +181,8 @@ const schema: IJSONSchema = {
 					},
 					fontSize: {
 						type: 'string',
-						description: nls.localize('schema.fontSize', 'When using a font: The font size in percentage to the text font. If not set, defaults to the size in the font definition.')
+						description: nls.localize('schema.fontSize', 'When using a font: The font size in percentage to the text font. If not set, defaults to the size in the font definition.'),
+						pattern: fontSizeRegex
 					},
 					fontId: {
 						type: 'string',

src/vs/workbench/services/themes/common/productIconThemeSchema.ts

@@ -9,6 +9,10 @@ import { Extensions as JSONExtensions, IJSONContributionRegistry } from 'vs/plat
 import { IJSONSchema } from 'vs/base/common/jsonSchema';
 import { iconsSchemaId } from 'vs/platform/theme/common/iconRegistry';
 
+export const fontIdRegex = '^([\\w-_]+)$';
+export const fontStyleRegex = '^(normal|italic|(oblique[ \\w\\s-]+))$';
+export const fontWeightRegex = '^(normal|bold|lighter|bolder|(\\d{0-1000}))$';
+export const fontSizeRegex = '^([\\w .%-_]+)$';
 
 const schemaId = 'vscode://schemas/product-icon-theme';
 const schema: IJSONSchema = {
@@ -18,13 +22,14 @@ const schema: IJSONSchema = {
 	properties: {
 		fonts: {
 			type: 'array',
-			description: nls.localize('schema.fonts', 'Fonts that are used in the icon definitions.'),
 			items: {
 				type: 'object',
 				properties: {
 					id: {
 						type: 'string',
-						description: nls.localize('schema.id', 'The ID of the font.')
+						description: nls.localize('schema.id', 'The ID of the font.'),
+						pattern: fontIdRegex,
+						patternErrorMessage: nls.localize('schema.id.formatError', 'The ID must only contain letters, numbers, underscore and minus.')
 					},
 					src: {
 						type: 'array',
@@ -38,7 +43,8 @@ const schema: IJSONSchema = {
 								},
 								format: {
 									type: 'string',
-									description: nls.localize('schema.font-format', 'The format of the font.')
+									description: nls.localize('schema.font-format', 'The format of the font.'),
+									enum: ['woff', 'woff2', 'truetype', 'opentype', 'embedded-opentype', 'svg']
 								}
 							},
 							required: [
@@ -49,15 +55,14 @@ const schema: IJSONSchema = {
 					},
 					weight: {
 						type: 'string',
-						description: nls.localize('schema.font-weight', 'The weight of the font.')
+						description: nls.localize('schema.font-weight', 'The weight of the font. See https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight for valid values.'),
+						pattern: fontWeightRegex
+
 					},
 					style: {
 						type: 'string',
-						description: nls.localize('schema.font-sstyle', 'The style of the font.')
-					},
-					size: {
-						type: 'string',
-						description: nls.localize('schema.font-size', 'The default size of the font.')
+						description: nls.localize('schema.font-style', 'The style of the font. See https://developer.mozilla.org/en-US/docs/Web/CSS/font-style for valid values.'),
+						pattern: fontStyleRegex
 					}
 				},
 				required: [