Use freeze instead of clone to prevent users to tamper data

src/vs/platform/configuration/common/configurationModels.ts

@@ -31,22 +31,8 @@ export class ConfigurationModel implements IConfiguraionModel {
 		return this._keys;
 	}
 
-	public getContentsFor<V>(section: string): V {
-		return objects.clone(this.contents[section]);
-	}
-
-	public override<V>(identifier: string): ConfigurationModel {
-		const result = new ConfigurationModel();
-		const contents = objects.clone<any>(this.contents);
-		if (this._overrides) {
-			for (const override of this._overrides) {
-				if (override.identifiers.indexOf(identifier) !== -1) {
-					merge(contents, override.contents, true);
-				}
-			}
-		}
-		result._contents = contents;
-		return result;
+	public getSectionContents<V>(section: string): V {
+		return this.contents[section];
 	}
 
 	public setValue(key: string, value: any) {
@@ -64,6 +50,20 @@ export class ConfigurationModel implements IConfiguraionModel {
 		}
 	}
 
+	public override<V>(identifier: string): ConfigurationModel {
+		const result = new ConfigurationModel();
+		const contents = objects.clone<any>(this.contents);
+		if (this._overrides) {
+			for (const override of this._overrides) {
+				if (override.identifiers.indexOf(identifier) !== -1) {
+					merge(contents, override.contents, true);
+				}
+			}
+		}
+		result._contents = contents;
+		return result;
+	}
+
 	public merge(other: ConfigurationModel, overwrite: boolean = true): ConfigurationModel {
 		const mergedModel = new ConfigurationModel();
 		this.doMerge(mergedModel, this, overwrite);
@@ -262,13 +262,12 @@ export class Configuration {
 
 	getSection<C>(section: string = '', overrides: IConfigurationOverrides = {}): C {
 		const configModel = this.getConsolidateConfigurationModel(overrides);
-		return section ? configModel.getContentsFor<C>(section) : configModel.contents;
+		return Object.freeze(section ? configModel.getSectionContents<C>(section) : configModel.contents);
 	}
 
 	getValue(key: string, overrides: IConfigurationOverrides = {}): any {
-		// make sure to clone the configuration so that the receiver does not tamper with the values
 		const consolidateConfigurationModel = this.getConsolidateConfigurationModel(overrides);
-		return objects.clone(getConfigurationValue<any>(consolidateConfigurationModel.contents, key));
+		return Object.freeze(getConfigurationValue<any>(consolidateConfigurationModel.contents, key));
 	}
 
 	updateValue(key: string, value: any, overrides: IConfigurationOverrides = {}): void {
@@ -297,18 +296,17 @@ export class Configuration {
 		memory?: C
 		value: C,
 	} {
-		// make sure to clone the configuration so that the receiver does not tamper with the values
 		const consolidateConfigurationModel = this.getConsolidateConfigurationModel(overrides);
 		const folderConfigurationModel = this.getFolderConfigurationModelForResource(overrides.resource);
 		const memoryConfigurationModel = overrides.resource ? this._memoryConfigurationByResource.get(overrides.resource) || this._memoryConfiguration : this._memoryConfiguration;
-		return {
-			default: objects.clone(getConfigurationValue<C>(overrides.overrideIdentifier ? this._defaults.override(overrides.overrideIdentifier).contents : this._defaults.contents, key)),
-			user: objects.clone(getConfigurationValue<C>(overrides.overrideIdentifier ? this._user.override(overrides.overrideIdentifier).contents : this._user.contents, key)),
-			workspace: objects.clone(this._workspace ? getConfigurationValue<C>(overrides.overrideIdentifier ? this._workspaceConfiguration.override(overrides.overrideIdentifier).contents : this._workspaceConfiguration.contents, key) : void 0), //Check on workspace exists or not because _workspaceConfiguration is never null
-			workspaceFolder: objects.clone(folderConfigurationModel ? getConfigurationValue<C>(overrides.overrideIdentifier ? folderConfigurationModel.override(overrides.overrideIdentifier).contents : folderConfigurationModel.contents, key) : void 0),
-			memory: objects.clone(getConfigurationValue<C>(overrides.overrideIdentifier ? memoryConfigurationModel.override(overrides.overrideIdentifier).contents : memoryConfigurationModel.contents, key)),
-			value: objects.clone(getConfigurationValue<C>(consolidateConfigurationModel.contents, key))
-		};
+		return Object.freeze({
+			default: getConfigurationValue<C>(overrides.overrideIdentifier ? this._defaults.override(overrides.overrideIdentifier).contents : this._defaults.contents, key),
+			user: getConfigurationValue<C>(overrides.overrideIdentifier ? this._user.override(overrides.overrideIdentifier).contents : this._user.contents, key),
+			workspace: this._workspace ? getConfigurationValue<C>(overrides.overrideIdentifier ? this._workspaceConfiguration.override(overrides.overrideIdentifier).contents : this._workspaceConfiguration.contents, key) : void 0, //Check on workspace exists or not because _workspaceConfiguration is never null
+			workspaceFolder: folderConfigurationModel ? getConfigurationValue<C>(overrides.overrideIdentifier ? folderConfigurationModel.override(overrides.overrideIdentifier).contents : folderConfigurationModel.contents, key) : void 0,
+			memory: getConfigurationValue<C>(overrides.overrideIdentifier ? memoryConfigurationModel.override(overrides.overrideIdentifier).contents : memoryConfigurationModel.contents, key),
+			value: getConfigurationValue<C>(consolidateConfigurationModel.contents, key)
+		});
 	}
 
 	keys(): {

src/vs/platform/configuration/test/common/configuration.model.test.ts

@@ -67,16 +67,16 @@ suite('Configuration', () => {
 
 	test('Test contents while getting an existing property', () => {
 		let testObject = new ConfigurationModel({ 'a': 1 });
-		assert.deepEqual(testObject.getContentsFor('a'), 1);
+		assert.deepEqual(testObject.getSectionContents('a'), 1);
 
 		testObject = new ConfigurationModel({ 'a': { 'b': 1 } });
-		assert.deepEqual(testObject.getContentsFor('a'), { 'b': 1 });
+		assert.deepEqual(testObject.getSectionContents('a'), { 'b': 1 });
 	});
 
 	test('Test contents are undefined for non existing properties', () => {
 		const testObject = new ConfigurationModel({ awesome: true });
 
-		assert.deepEqual(testObject.getContentsFor('unknownproperty'), undefined);
+		assert.deepEqual(testObject.getSectionContents('unknownproperty'), undefined);
 	});
 
 	test('Test override gives all content merged with overrides', () => {

src/vs/platform/configuration/test/common/configurationModel.test.ts

@@ -61,10 +61,10 @@ suite('Configuration', () => {
 
 	test('Test contents while getting an existing property', () => {
 		let testObject = new CustomConfigurationModel(JSON.stringify({ 'a': 1 }));
-		assert.deepEqual(testObject.getContentsFor('a'), 1);
+		assert.deepEqual(testObject.getSectionContents('a'), 1);
 
 		testObject = new CustomConfigurationModel(JSON.stringify({ 'a': { 'b': 1 } }));
-		assert.deepEqual(testObject.getContentsFor('a'), { 'b': 1 });
+		assert.deepEqual(testObject.getSectionContents('a'), { 'b': 1 });
 	});
 
 	test('Test contents are undefined for non existing properties', () => {
@@ -72,13 +72,13 @@ suite('Configuration', () => {
 			awesome: true
 		}));
 
-		assert.deepEqual(testObject.getContentsFor('unknownproperty'), undefined);
+		assert.deepEqual(testObject.getSectionContents('unknownproperty'), undefined);
 	});
 
 	test('Test contents are undefined for undefined config', () => {
 		const testObject = new CustomConfigurationModel(null);
 
-		assert.deepEqual(testObject.getContentsFor('unknownproperty'), undefined);
+		assert.deepEqual(testObject.getSectionContents('unknownproperty'), undefined);
 	});
 
 	test('Test configWithOverrides gives all content merged with overrides', () => {
@@ -125,7 +125,7 @@ suite('Configuration', () => {
 				}
 			}
 		});
-		assert.equal(true, new DefaultConfigurationModel().getContentsFor('a'));
+		assert.equal(true, new DefaultConfigurationModel().getSectionContents('a'));
 	});
 
 	test('Test registering the language property', () => {
@@ -142,7 +142,7 @@ suite('Configuration', () => {
 				}
 			}
 		});
-		assert.equal(undefined, new DefaultConfigurationModel().getContentsFor('[a]'));
+		assert.equal(undefined, new DefaultConfigurationModel().getSectionContents('[a]'));
 	});
 
 });
\ No newline at end of file

src/vs/workbench/services/configuration/test/common/configurationModels.test.ts

@@ -17,7 +17,7 @@ suite('ConfigurationService - Model', () => {
 
 		const testObject = new FolderConfigurationModel(settingsConfig, [], ConfigurationScope.WINDOW);
 
-		assert.equal(testObject.getContentsFor('task'), undefined);
+		assert.equal(testObject.getSectionContents('task'), undefined);
 	});
 
 	test('Test consolidate (settings and tasks)', () => {