diff --git a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts index 7cde80bda84..70bc6515af2 100644 --- a/src/vs/platform/userDataSync/common/abstractSynchronizer.ts +++ b/src/vs/platform/userDataSync/common/abstractSynchronizer.ts @@ -35,6 +35,16 @@ type IncompatibleSyncSourceClassification = { source: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; isMeasurement: true; comment: 'settings sync resource. eg., settings, keybindings...' }; }; +export function isRemoteUserData(thing: any): thing is IRemoteUserData { + if (thing + && (thing.ref !== undefined && typeof thing.ref === 'string' && thing.ref !== '') + && (thing.syncData !== undefined && (thing.syncData === null || isSyncData(thing.syncData)))) { + return true; + } + + return false; +} + export function isSyncData(thing: any): thing is ISyncData { if (thing && (thing.version !== undefined && typeof thing.version === 'number') @@ -595,11 +605,13 @@ export abstract class AbstractSynchroniser extends Disposable implements IUserDa let retrial = 1; while (syncData === undefined && retrial++ < 6 /* Retry 5 times */) { try { - const content = (await this.fileService.readFile(this.lastSyncResource)).value.toString(); - try { syncData = content ? JSON.parse(content) : null; } catch (e) { /* Ignore */ } - if (syncData && !isSyncData(syncData)) { - this.logService.info(`${this.syncResourceLogLabel}: Last sync data stored locally is invalid.`); - syncData = undefined; + const lastSyncStoredRemoteUserData = await this.readLastSyncStoredRemoteUserData(); + if (lastSyncStoredRemoteUserData) { + if (lastSyncStoredRemoteUserData.ref === lastSyncUserDataState.ref) { + syncData = lastSyncStoredRemoteUserData.syncData; + } else { + this.logService.info(`${this.syncResourceLogLabel}: Last sync data stored locally is not same as the last sync state.`); + } } break; } catch (error) { @@ -620,10 +632,10 @@ export abstract class AbstractSynchroniser extends Disposable implements IUserDa try { const content = await this.userDataSyncStoreService.resolveResourceContent(this.resource, lastSyncUserDataState.ref, this.collection, this.syncHeaders); syncData = content === null ? null : this.parseSyncData(content); - await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(syncData ? JSON.stringify(syncData) : '')); + await this.writeLastSyncStoredRemoteUserData({ ref: lastSyncUserDataState.ref, syncData }); } catch (error) { if (error instanceof UserDataSyncError && error.code === UserDataSyncErrorCode.NotFound) { - this.logService.info(`${this.syncResourceLogLabel}: Last sync resource does not exist on the server.`); + this.logService.info(`${this.syncResourceLogLabel}: .`); } else { throw error; } @@ -654,7 +666,24 @@ export abstract class AbstractSynchroniser extends Disposable implements IUserDa }; this.storageService.store(this.lastSyncUserDataStateKey, JSON.stringify(lastSyncUserDataState), StorageScope.APPLICATION, StorageTarget.MACHINE); - await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(lastSyncRemoteUserData.syncData ? JSON.stringify(lastSyncRemoteUserData.syncData) : '')); + await this.writeLastSyncStoredRemoteUserData(lastSyncRemoteUserData); + } + + private async readLastSyncStoredRemoteUserData(): Promise { + const content = (await this.fileService.readFile(this.lastSyncResource)).value.toString(); + try { + const lastSyncStoredRemoteUserData = content ? JSON.parse(content) : undefined; + if (isRemoteUserData(lastSyncStoredRemoteUserData)) { + return lastSyncStoredRemoteUserData; + } + } catch (e) { + this.logService.error(e); + } + return undefined; + } + + private async writeLastSyncStoredRemoteUserData(lastSyncRemoteUserData: IRemoteUserData): Promise { + await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(JSON.stringify(lastSyncRemoteUserData))); } private async migrateLastSyncUserData(): Promise { @@ -662,13 +691,12 @@ export abstract class AbstractSynchroniser extends Disposable implements IUserDa const content = await this.fileService.readFile(this.lastSyncResource); const userData = JSON.parse(content.value.toString()); await this.fileService.del(this.lastSyncResource); - if (userData.ref) { + if (userData.ref && userData.content !== undefined) { this.storageService.store(this.lastSyncUserDataStateKey, JSON.stringify({ ...userData, content: undefined, }), StorageScope.APPLICATION, StorageTarget.MACHINE); - await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(userData.content || '')); - this.logService.info(`${this.syncResourceLogLabel}: Migrated data from last sync resource to last sync state.`); + await this.writeLastSyncStoredRemoteUserData({ ref: userData.ref, syncData: userData.content === null ? null : JSON.parse(userData.content) }); } } catch (error) { if (error instanceof FileOperationError && error.fileOperationResult === FileOperationResult.FILE_NOT_FOUND) { @@ -878,6 +906,7 @@ export abstract class AbstractInitializer implements IUserDataInitializer { @IEnvironmentService protected readonly environmentService: IEnvironmentService, @ILogService protected readonly logService: ILogService, @IFileService protected readonly fileService: IFileService, + @IStorageService protected readonly storageService: IStorageService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { this.extUri = uriIdentityService.extUri; @@ -916,8 +945,18 @@ export abstract class AbstractInitializer implements IUserDataInitializer { } protected async updateLastSyncUserData(lastSyncRemoteUserData: IRemoteUserData, additionalProps: IStringDictionary = {}): Promise { - const lastSyncUserData: IUserData = { ref: lastSyncRemoteUserData.ref, content: lastSyncRemoteUserData.syncData ? JSON.stringify(lastSyncRemoteUserData.syncData) : null, ...additionalProps }; - await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(JSON.stringify(lastSyncUserData))); + if (additionalProps['ref'] || additionalProps['version']) { + throw new Error('Cannot have core properties as additional'); + } + + const lastSyncUserDataState: ILastSyncUserDataState = { + ref: lastSyncRemoteUserData.ref, + version: undefined, + ...additionalProps + }; + + this.storageService.store(`${this.resource}.lastSyncUserData`, JSON.stringify(lastSyncUserDataState), StorageScope.APPLICATION, StorageTarget.MACHINE); + await this.fileService.writeFile(this.lastSyncResource, VSBuffer.fromString(JSON.stringify(lastSyncRemoteUserData))); } protected abstract doInitialize(remoteUserData: IRemoteUserData): Promise; diff --git a/src/vs/platform/userDataSync/common/extensionsSync.ts b/src/vs/platform/userDataSync/common/extensionsSync.ts index 4e61627c14d..c40c3a42a88 100644 --- a/src/vs/platform/userDataSync/common/extensionsSync.ts +++ b/src/vs/platform/userDataSync/common/extensionsSync.ts @@ -542,9 +542,10 @@ export abstract class AbstractExtensionsInitializer extends AbstractInitializer @IUserDataProfilesService userDataProfilesService: IUserDataProfilesService, @IEnvironmentService environmentService: IEnvironmentService, @ILogService logService: ILogService, + @IStorageService storageService: IStorageService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { - super(SyncResource.Extensions, userDataProfilesService, environmentService, logService, fileService, uriIdentityService); + super(SyncResource.Extensions, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService); } protected async parseExtensions(remoteUserData: IRemoteUserData): Promise { diff --git a/src/vs/platform/userDataSync/common/globalStateSync.ts b/src/vs/platform/userDataSync/common/globalStateSync.ts index bb98c1821b0..ba15b967540 100644 --- a/src/vs/platform/userDataSync/common/globalStateSync.ts +++ b/src/vs/platform/userDataSync/common/globalStateSync.ts @@ -398,14 +398,14 @@ export class LocalGlobalStateProvider { export class GlobalStateInitializer extends AbstractInitializer { constructor( - @IStorageService private readonly storageService: IStorageService, + @IStorageService storageService: IStorageService, @IFileService fileService: IFileService, @IUserDataProfilesService userDataProfilesService: IUserDataProfilesService, @IEnvironmentService environmentService: IEnvironmentService, @IUserDataSyncLogService logService: IUserDataSyncLogService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { - super(SyncResource.GlobalState, userDataProfilesService, environmentService, logService, fileService, uriIdentityService); + super(SyncResource.GlobalState, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService); } async doInitialize(remoteUserData: IRemoteUserData): Promise { diff --git a/src/vs/platform/userDataSync/common/keybindingsSync.ts b/src/vs/platform/userDataSync/common/keybindingsSync.ts index 02c4382cf45..0858e96d8e2 100644 --- a/src/vs/platform/userDataSync/common/keybindingsSync.ts +++ b/src/vs/platform/userDataSync/common/keybindingsSync.ts @@ -345,9 +345,10 @@ export class KeybindingsInitializer extends AbstractInitializer { @IUserDataProfilesService userDataProfilesService: IUserDataProfilesService, @IEnvironmentService environmentService: IEnvironmentService, @IUserDataSyncLogService logService: IUserDataSyncLogService, + @IStorageService storageService: IStorageService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { - super(SyncResource.Keybindings, userDataProfilesService, environmentService, logService, fileService, uriIdentityService); + super(SyncResource.Keybindings, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService); } async doInitialize(remoteUserData: IRemoteUserData): Promise { diff --git a/src/vs/platform/userDataSync/common/settingsSync.ts b/src/vs/platform/userDataSync/common/settingsSync.ts index da07e78c29b..53ffc27a0cd 100644 --- a/src/vs/platform/userDataSync/common/settingsSync.ts +++ b/src/vs/platform/userDataSync/common/settingsSync.ts @@ -348,9 +348,10 @@ export class SettingsInitializer extends AbstractInitializer { @IUserDataProfilesService userDataProfilesService: IUserDataProfilesService, @IEnvironmentService environmentService: IEnvironmentService, @IUserDataSyncLogService logService: IUserDataSyncLogService, + @IStorageService storageService: IStorageService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { - super(SyncResource.Settings, userDataProfilesService, environmentService, logService, fileService, uriIdentityService); + super(SyncResource.Settings, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService); } async doInitialize(remoteUserData: IRemoteUserData): Promise { diff --git a/src/vs/platform/userDataSync/common/snippetsSync.ts b/src/vs/platform/userDataSync/common/snippetsSync.ts index 48555eb486d..788dd5a1e61 100644 --- a/src/vs/platform/userDataSync/common/snippetsSync.ts +++ b/src/vs/platform/userDataSync/common/snippetsSync.ts @@ -509,9 +509,10 @@ export class SnippetsInitializer extends AbstractInitializer { @IUserDataProfilesService userDataProfilesService: IUserDataProfilesService, @IEnvironmentService environmentService: IEnvironmentService, @IUserDataSyncLogService logService: IUserDataSyncLogService, + @IStorageService storageService: IStorageService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { - super(SyncResource.Snippets, userDataProfilesService, environmentService, logService, fileService, uriIdentityService); + super(SyncResource.Snippets, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService); } async doInitialize(remoteUserData: IRemoteUserData): Promise { diff --git a/src/vs/platform/userDataSync/common/tasksSync.ts b/src/vs/platform/userDataSync/common/tasksSync.ts index 8873af72210..f8af31027f6 100644 --- a/src/vs/platform/userDataSync/common/tasksSync.ts +++ b/src/vs/platform/userDataSync/common/tasksSync.ts @@ -254,9 +254,10 @@ export class TasksInitializer extends AbstractInitializer { @IUserDataProfilesService userDataProfilesService: IUserDataProfilesService, @IEnvironmentService environmentService: IEnvironmentService, @IUserDataSyncLogService logService: IUserDataSyncLogService, + @IStorageService storageService: IStorageService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { - super(SyncResource.Tasks, userDataProfilesService, environmentService, logService, fileService, uriIdentityService); + super(SyncResource.Tasks, userDataProfilesService, environmentService, logService, fileService, storageService, uriIdentityService); } async doInitialize(remoteUserData: IRemoteUserData): Promise { diff --git a/src/vs/platform/userDataSync/test/common/synchronizer.test.ts b/src/vs/platform/userDataSync/test/common/synchronizer.test.ts index 081cab1358c..1d7e3c68cad 100644 --- a/src/vs/platform/userDataSync/test/common/synchronizer.test.ts +++ b/src/vs/platform/userDataSync/test/common/synchronizer.test.ts @@ -1110,7 +1110,7 @@ suite('TestSynchronizer - Last Sync Data', () => { const actual = await testObject.getLastSyncUserData(); assert.deepStrictEqual(storageService.get('settings.lastSyncUserData', StorageScope.APPLICATION), JSON.stringify({ ref: '1' })); - assert.deepStrictEqual(JSON.parse((await fileService.readFile(testObject.getLastSyncResource())).value.toString()), { content: '0', machineId, version: 1 }); + assert.deepStrictEqual(JSON.parse((await fileService.readFile(testObject.getLastSyncResource())).value.toString()), { ref: '1', syncData: { version: 1, machineId, content: '0' } }); assert.deepStrictEqual(actual, { ref: '1', syncData: { @@ -1163,6 +1163,7 @@ suite('TestSynchronizer - Last Sync Data', () => { foo: 'bar' } }))); + server.reset(); const actual = await testObject.getLastSyncUserData(); assert.deepStrictEqual(storageService.get('settings.lastSyncUserData', StorageScope.APPLICATION), JSON.stringify({ ref: '1' })); @@ -1174,6 +1175,38 @@ suite('TestSynchronizer - Last Sync Data', () => { version: 1 }, }); + assert.deepStrictEqual(server.requests, [{ headers: {}, type: 'GET', url: 'http://host:3000/v1/resource/settings/1' }]); + })); + + test('last sync data is read from server after sync and stored sync data is tampered', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const storageService = client.instantiationService.get(IStorageService); + const fileService = client.instantiationService.get(IFileService); + const testObject: TestSynchroniser = disposableStore.add(client.instantiationService.createInstance(TestSynchroniser, { syncResource: SyncResource.Settings, profile: client.instantiationService.get(IUserDataProfilesService).defaultProfile }, undefined)); + testObject.syncBarrier.open(); + + await testObject.sync(await client.getResourceManifest()); + const machineId = await testObject.getMachineId(); + await fileService.writeFile(testObject.getLastSyncResource(), VSBuffer.fromString(JSON.stringify({ + ref: '2', + syncData: { + content: '0', + machineId, + version: 1 + } + }))); + server.reset(); + const actual = await testObject.getLastSyncUserData(); + + assert.deepStrictEqual(storageService.get('settings.lastSyncUserData', StorageScope.APPLICATION), JSON.stringify({ ref: '1' })); + assert.deepStrictEqual(actual, { + ref: '1', + syncData: { + content: '0', + machineId, + version: 1 + } + }); + assert.deepStrictEqual(server.requests, [{ headers: {}, type: 'GET', url: 'http://host:3000/v1/resource/settings/1' }]); })); test('reading last sync data: no requests are made to server when sync data is invalid', () => runWithFakedTimers({ useFakeTimers: true }, async () => { @@ -1202,6 +1235,22 @@ suite('TestSynchronizer - Last Sync Data', () => { assert.deepStrictEqual(server.requests, []); })); + test('reading last sync data: no requests are made to server when sync data is null', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const fileService = client.instantiationService.get(IFileService); + const testObject: TestSynchroniser = disposableStore.add(client.instantiationService.createInstance(TestSynchroniser, { syncResource: SyncResource.Settings, profile: client.instantiationService.get(IUserDataProfilesService).defaultProfile }, undefined)); + testObject.syncBarrier.open(); + + await testObject.sync(await client.getResourceManifest()); + server.reset(); + await fileService.writeFile(testObject.getLastSyncResource(), VSBuffer.fromString(JSON.stringify({ + ref: '1', + syncData: null, + }))); + await testObject.getLastSyncUserData(); + + assert.deepStrictEqual(server.requests, []); + })); + test('last sync data is null after sync if last sync state is deleted', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const storageService = client.instantiationService.get(IStorageService); const testObject: TestSynchroniser = disposableStore.add(client.instantiationService.createInstance(TestSynchroniser, { syncResource: SyncResource.Settings, profile: client.instantiationService.get(IUserDataProfilesService).defaultProfile }, undefined)); diff --git a/src/vs/workbench/contrib/extensions/electron-sandbox/remoteExtensionsInit.ts b/src/vs/workbench/contrib/extensions/electron-sandbox/remoteExtensionsInit.ts index 851e2d0c3f3..f323ded3750 100644 --- a/src/vs/workbench/contrib/extensions/electron-sandbox/remoteExtensionsInit.ts +++ b/src/vs/workbench/contrib/extensions/electron-sandbox/remoteExtensionsInit.ts @@ -105,9 +105,10 @@ class RemoteExtensionsInitializer extends AbstractExtensionsInitializer { @ILogService logService: ILogService, @IUriIdentityService uriIdentityService: IUriIdentityService, @IExtensionGalleryService private readonly extensionGalleryService: IExtensionGalleryService, + @IStorageService storageService: IStorageService, @IExtensionManifestPropertiesService private readonly extensionManifestPropertiesService: IExtensionManifestPropertiesService, ) { - super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, uriIdentityService); + super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, storageService, uriIdentityService); } protected override async doInitialize(remoteUserData: IRemoteUserData): Promise { diff --git a/src/vs/workbench/services/userData/browser/userDataInit.ts b/src/vs/workbench/services/userData/browser/userDataInit.ts index e7cfccfe177..a617395815a 100644 --- a/src/vs/workbench/services/userData/browser/userDataInit.ts +++ b/src/vs/workbench/services/userData/browser/userDataInit.ts @@ -272,10 +272,10 @@ export class UserDataInitializationService implements IUserDataInitializationSer private createSyncResourceInitializer(syncResource: SyncResource): IUserDataInitializer { switch (syncResource) { - case SyncResource.Settings: return new SettingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService); - case SyncResource.Keybindings: return new KeybindingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService); - case SyncResource.Tasks: return new TasksInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService); - case SyncResource.Snippets: return new SnippetsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService); + case SyncResource.Settings: return new SettingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService); + case SyncResource.Keybindings: return new KeybindingsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService); + case SyncResource.Tasks: return new TasksInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService); + case SyncResource.Snippets: return new SnippetsInitializer(this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.storageService, this.uriIdentityService); case SyncResource.GlobalState: return new GlobalStateInitializer(this.storageService, this.fileService, this.userDataProfilesService, this.environmentService, this.logService, this.uriIdentityService); } throw new Error(`Cannot create initializer for ${syncResource}`); @@ -296,9 +296,10 @@ class ExtensionsPreviewInitializer extends AbstractExtensionsInitializer { @IUserDataProfilesService userDataProfilesService: IUserDataProfilesService, @IEnvironmentService environmentService: IEnvironmentService, @IUserDataSyncLogService logService: IUserDataSyncLogService, + @IStorageService storageService: IStorageService, @IUriIdentityService uriIdentityService: IUriIdentityService, ) { - super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, uriIdentityService); + super(extensionManagementService, ignoredExtensionsManagementService, fileService, userDataProfilesService, environmentService, logService, storageService, uriIdentityService); } getPreview(): Promise {