Skip to content

Commit e278c54

Browse files
author
Benjamin Pasero
committed
prevent sync access to install source on startup (for microsoft#39034)
1 parent 7866afa commit e278c54

10 files changed

Lines changed: 67 additions & 58 deletions

File tree

‎src/vs/code/electron-browser/sharedProcessMain.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,12 @@ function main(server: Server, initData: ISharedProcessInitData, configuration: I
107107

108108
const services = new ServiceCollection();
109109
const environmentService = accessor.get(IEnvironmentService);
110-
const { appRoot, extensionsPath, extensionDevelopmentPath, isBuilt, installSource } = environmentService;
110+
const { appRoot, extensionsPath, extensionDevelopmentPath, isBuilt, installSourcePath } = environmentService;
111111

112112
if (isBuilt && !extensionDevelopmentPath && !environmentService.args['disable-telemetry'] && product.enableTelemetry) {
113113
const config: ITelemetryServiceConfig = {
114114
appender,
115-
commonProperties: resolveCommonProperties(product.commit, pkg.version, installSource, configuration.machineId),
115+
commonProperties: resolveCommonProperties(product.commit, pkg.version, configuration.machineId, installSourcePath),
116116
piiPaths: [appRoot, extensionsPath]
117117
};
118118

‎src/vs/code/electron-main/app.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ export class CodeApplication {
308308
if (this.environmentService.isBuilt && !this.environmentService.isExtensionDevelopment && !this.environmentService.args['disable-telemetry'] && !!product.enableTelemetry) {
309309
const channel = getDelayedChannel<ITelemetryAppenderChannel>(this.sharedProcessClient.then(c => c.getChannel('telemetryAppender')));
310310
const appender = new TelemetryAppenderClient(channel);
311-
const commonProperties = resolveCommonProperties(product.commit, pkg.version, this.environmentService.installSource, machineId);
311+
const commonProperties = resolveCommonProperties(product.commit, pkg.version, machineId, this.environmentService.installSourcePath);
312312
const piiPaths = [this.environmentService.appRoot, this.environmentService.extensionsPath];
313313
const config: ITelemetryServiceConfig = { appender, commonProperties, piiPaths };
314314

‎src/vs/code/node/cliProcessMain.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
1616
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
1717
import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
1818
import { IEnvironmentService, ParsedArgs } from 'vs/platform/environment/common/environment';
19-
import { EnvironmentService, getInstallSourcePath } from 'vs/platform/environment/node/environmentService';
19+
import { EnvironmentService } from 'vs/platform/environment/node/environmentService';
2020
import { IExtensionManagementService, IExtensionGalleryService, IExtensionManifest, IGalleryExtension, LocalExtensionType } from 'vs/platform/extensionManagement/common/extensionManagement';
2121
import { ExtensionManagementService, validateLocalExtension } from 'vs/platform/extensionManagement/node/extensionManagementService';
2222
import { ExtensionGalleryService } from 'vs/platform/extensionManagement/node/extensionGalleryService';
@@ -78,9 +78,7 @@ class Main {
7878
}
7979

8080
private setInstallSource(installSource: string): TPromise<any> {
81-
const path = getInstallSourcePath(this.environmentService.userDataPath);
82-
83-
return writeFile(path, installSource.slice(0, 30));
81+
return writeFile(this.environmentService.installSourcePath, installSource.slice(0, 30));
8482
}
8583

8684
private listExtensions(showVersions: boolean): TPromise<any> {
@@ -187,7 +185,7 @@ export function main(argv: ParsedArgs): TPromise<void> {
187185
const stateService = accessor.get(IStateService);
188186

189187
return TPromise.join([envService.appSettingsHome, envService.extensionsPath].map(p => mkdirp(p))).then(() => {
190-
const { appRoot, extensionsPath, extensionDevelopmentPath, isBuilt, installSource } = envService;
188+
const { appRoot, extensionsPath, extensionDevelopmentPath, isBuilt, installSourcePath } = envService;
191189

192190
const services = new ServiceCollection();
193191
services.set(IConfigurationService, new SyncDescriptor(ConfigurationService));
@@ -209,7 +207,7 @@ export function main(argv: ParsedArgs): TPromise<void> {
209207

210208
const config: ITelemetryServiceConfig = {
211209
appender: combinedAppender(...appenders),
212-
commonProperties: resolveCommonProperties(product.commit, pkg.version, installSource, stateService.getItem('telemetry.machineId')),
210+
commonProperties: resolveCommonProperties(product.commit, pkg.version, stateService.getItem('telemetry.machineId'), installSourcePath),
213211
piiPaths: [appRoot, extensionsPath]
214212
};
215213

‎src/vs/platform/environment/common/environment.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export interface IEnvironmentService {
109109

110110
nodeCachedDataDir: string;
111111

112-
installSource: string;
112+
installSourcePath: string;
113113
disableUpdates: boolean;
114114
disableCrashReporter: boolean;
115115
}

‎src/vs/platform/environment/node/environmentService.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,6 @@ function getIPCHandle(userDataPath: string, type: string): string {
4040
}
4141
}
4242

43-
export function getInstallSourcePath(userDataPath: string): string {
44-
return path.join(userDataPath, 'installSource');
45-
}
46-
4743
export class EnvironmentService implements IEnvironmentService {
4844

4945
_serviceBrand: any;
@@ -92,6 +88,9 @@ export class EnvironmentService implements IEnvironmentService {
9288
@memoize
9389
get workspacesHome(): string { return path.join(this.userDataPath, 'Workspaces'); }
9490

91+
@memoize
92+
get installSourcePath(): string { return path.join(this.userDataPath, 'installSource'); }
93+
9594
@memoize
9695
get extensionsPath(): string { return parsePathArg(this._args['extensions-dir'], process) || process.env['VSCODE_EXTENSIONS'] || path.join(this.userHome, product.dataFolderName, 'extensions'); }
9796

@@ -132,8 +131,6 @@ export class EnvironmentService implements IEnvironmentService {
132131

133132
readonly machineUUID: string;
134133

135-
readonly installSource: string;
136-
137134
constructor(private _args: ParsedArgs, private _execPath: string) {
138135
const machineIdPath = path.join(this.userDataPath, 'machineid');
139136

@@ -152,12 +149,6 @@ export class EnvironmentService implements IEnvironmentService {
152149
// noop
153150
}
154151
}
155-
156-
try {
157-
this.installSource = fs.readFileSync(getInstallSourcePath(this.userDataPath), 'utf8').slice(0, 30);
158-
} catch (err) {
159-
this.installSource = '';
160-
}
161152
}
162153
}
163154

‎src/vs/platform/state/test/node/state.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ suite('StateService', () => {
1818
const storageFile = path.join(parentDir, 'storage.json');
1919

2020
teardown(done => {
21-
extfs.del(storageFile, os.tmpdir(), done);
21+
extfs.del(parentDir, os.tmpdir(), done);
2222
});
2323

2424
test('Basics', done => {

‎src/vs/platform/telemetry/node/commonProperties.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ import * as Platform from 'vs/base/common/platform';
77
import * as os from 'os';
88
import { TPromise } from 'vs/base/common/winjs.base';
99
import * as uuid from 'vs/base/common/uuid';
10+
import { readFile } from 'vs/base/node/pfs';
1011

11-
export function resolveCommonProperties(commit: string, version: string, source: string, machineId: string): TPromise<{ [name: string]: string; }> {
12+
export function resolveCommonProperties(commit: string, version: string, machineId: string, installSourcePath: string): TPromise<{ [name: string]: string; }> {
1213
const result: { [name: string]: string; } = Object.create(null);
1314
// __GDPR__COMMON__ "common.machineId" : { "classification": "EndUserPseudonymizedInformation", "purpose": "FeatureInsight" }
1415
result['common.machineId'] = machineId;
@@ -26,8 +27,6 @@ export function resolveCommonProperties(commit: string, version: string, source:
2627
result['common.nodePlatform'] = process.platform;
2728
// __GDPR__COMMON__ "common.nodeArch" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
2829
result['common.nodeArch'] = process.arch;
29-
// __GDPR__COMMON__ "common.source" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
30-
result['common.source'] = source;
3130

3231
// dynamic properties which value differs on each call
3332
let seq = 0;
@@ -50,5 +49,13 @@ export function resolveCommonProperties(commit: string, version: string, source:
5049
}
5150
});
5251

53-
return TPromise.as(result);
52+
return readFile(installSourcePath, 'utf8').then(contents => {
53+
54+
// __GDPR__COMMON__ "common.source" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
55+
result['common.source'] = contents.slice(0, 30);
56+
57+
return result;
58+
}, error => {
59+
return result;
60+
});
5461
}

‎src/vs/platform/telemetry/node/workbenchCommonProperties.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import * as uuid from 'vs/base/common/uuid';
99
import { IStorageService } from 'vs/platform/storage/common/storage';
1010
import { resolveCommonProperties } from '../node/commonProperties';
1111

12-
export function resolveWorkbenchCommonProperties(storageService: IStorageService, commit: string, version: string, source: string, machineId: string): TPromise<{ [name: string]: string }> {
13-
return resolveCommonProperties(commit, version, source, machineId).then(result => {
12+
export function resolveWorkbenchCommonProperties(storageService: IStorageService, commit: string, version: string, machineId: string, installSourcePath: string): TPromise<{ [name: string]: string }> {
13+
return resolveCommonProperties(commit, version, machineId, installSourcePath).then(result => {
1414
// __GDPR__COMMON__ "common.version.shell" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }
1515
result['common.version.shell'] = process.versions && (<any>process).versions['electron'];
1616
// __GDPR__COMMON__ "common.version.renderer" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" }

‎src/vs/platform/telemetry/test/electron-browser/commonProperties.test.ts‎

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,57 +5,70 @@
55
'use strict';
66

77
import * as assert from 'assert';
8+
import * as path from 'path';
9+
import * as os from 'os';
10+
import * as fs from 'fs';
811
import { TPromise } from 'vs/base/common/winjs.base';
912
import { resolveWorkbenchCommonProperties } from 'vs/platform/telemetry/node/workbenchCommonProperties';
1013
import { StorageService, InMemoryLocalStorage } from 'vs/platform/storage/common/storageService';
1114
import { TestWorkspace } from 'vs/platform/workspace/test/common/testWorkspace';
15+
import { getRandomTestPath } from 'vs/workbench/test/workbenchTestServices';
16+
import { del } from 'vs/base/node/extfs';
17+
import { mkdirp } from 'vs/base/node/pfs';
1218

1319
suite('Telemetry - common properties', function () {
20+
const parentDir = getRandomTestPath(os.tmpdir(), 'vsctests', 'telemetryservice');
21+
const installSource = path.join(parentDir, 'installSource');
1422

1523
const commit: string = void 0;
1624
const version: string = void 0;
17-
const source: string = void 0;
1825
let storageService: StorageService;
1926

2027
setup(() => {
2128
storageService = new StorageService(new InMemoryLocalStorage(), null, TestWorkspace.id);
2229
});
2330

24-
test('default', function () {
25-
26-
return resolveWorkbenchCommonProperties(storageService, commit, version, source, 'someMachineId').then(props => {
27-
28-
assert.ok('commitHash' in props);
29-
assert.ok('sessionID' in props);
30-
assert.ok('timestamp' in props);
31-
assert.ok('common.platform' in props);
32-
assert.ok('common.nodePlatform' in props);
33-
assert.ok('common.nodeArch' in props);
34-
assert.ok('common.timesincesessionstart' in props);
35-
assert.ok('common.sequence' in props);
36-
37-
// assert.ok('common.version.shell' in first.data); // only when running on electron
38-
// assert.ok('common.version.renderer' in first.data);
39-
assert.ok('common.osVersion' in props, 'osVersion');
40-
assert.ok('version' in props);
41-
assert.ok('common.source' in props);
42-
43-
assert.ok('common.firstSessionDate' in props, 'firstSessionDate');
44-
assert.ok('common.lastSessionDate' in props, 'lastSessionDate'); // conditional, see below, 'lastSessionDate'ow
45-
assert.ok('common.isNewSession' in props, 'isNewSession');
31+
teardown(done => {
32+
del(parentDir, os.tmpdir(), done);
33+
});
4634

47-
// machine id et al
48-
assert.ok('common.instanceId' in props, 'instanceId');
49-
assert.ok('common.machineId' in props, 'machineId');
35+
test('pasero default', function () {
36+
return mkdirp(parentDir).then(() => {
37+
fs.writeFileSync(installSource, 'my.install.source');
38+
39+
return resolveWorkbenchCommonProperties(storageService, commit, version, 'someMachineId', installSource).then(props => {
40+
assert.ok('commitHash' in props);
41+
assert.ok('sessionID' in props);
42+
assert.ok('timestamp' in props);
43+
assert.ok('common.platform' in props);
44+
assert.ok('common.nodePlatform' in props);
45+
assert.ok('common.nodeArch' in props);
46+
assert.ok('common.timesincesessionstart' in props);
47+
assert.ok('common.sequence' in props);
48+
49+
// assert.ok('common.version.shell' in first.data); // only when running on electron
50+
// assert.ok('common.version.renderer' in first.data);
51+
assert.ok('common.osVersion' in props, 'osVersion');
52+
assert.ok('version' in props);
53+
assert.equal(props['common.source'], 'my.install.source');
54+
55+
assert.ok('common.firstSessionDate' in props, 'firstSessionDate');
56+
assert.ok('common.lastSessionDate' in props, 'lastSessionDate'); // conditional, see below, 'lastSessionDate'ow
57+
assert.ok('common.isNewSession' in props, 'isNewSession');
58+
59+
// machine id et al
60+
assert.ok('common.instanceId' in props, 'instanceId');
61+
assert.ok('common.machineId' in props, 'machineId');
5062

63+
});
5164
});
5265
});
5366

5467
test('lastSessionDate when aviablale', function () {
5568

5669
storageService.store('telemetry.lastSessionDate', new Date().toUTCString());
5770

58-
return resolveWorkbenchCommonProperties(storageService, commit, version, source, 'someMachineId').then(props => {
71+
return resolveWorkbenchCommonProperties(storageService, commit, version, 'someMachineId', installSource).then(props => {
5972

6073
assert.ok('common.lastSessionDate' in props); // conditional, see below
6174
assert.ok('common.isNewSession' in props);
@@ -64,7 +77,7 @@ suite('Telemetry - common properties', function () {
6477
});
6578

6679
test('values chance on ask', function () {
67-
return resolveWorkbenchCommonProperties(storageService, commit, version, source, 'someMachineId').then(props => {
80+
return resolveWorkbenchCommonProperties(storageService, commit, version, 'someMachineId', installSource).then(props => {
6881
let value1 = props['common.sequence'];
6982
let value2 = props['common.sequence'];
7083
assert.ok(value1 !== value2, 'seq');

‎src/vs/workbench/electron-browser/shell.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ export class WorkbenchShell {
317317

318318
const config: ITelemetryServiceConfig = {
319319
appender: new TelemetryAppenderClient(channel),
320-
commonProperties: resolveWorkbenchCommonProperties(this.storageService, commit, version, this.environmentService.installSource, this.configuration.machineId),
320+
commonProperties: resolveWorkbenchCommonProperties(this.storageService, commit, version, this.configuration.machineId, this.environmentService.installSourcePath),
321321
piiPaths: [this.environmentService.appRoot, this.environmentService.extensionsPath]
322322
};
323323

0 commit comments

Comments
 (0)