From 0c17e38449910c8239d499085e94f5d8bdaa334c Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 8 Mar 2021 07:23:11 +0100 Subject: [PATCH] sandbox - migrate backups from md5 to our hash algorithm --- .../electron-browser/backupTracker.test.ts | 2 +- .../sandbox.simpleservices.ts | 25 ---- .../backup/browser/backupFileService.ts | 9 -- .../backup/common/backupFileService.ts | 118 +++++++++++++----- .../backupFileService.ts | 15 --- .../backupFileService.test.ts | 83 +++++++++--- src/vs/workbench/workbench.desktop.main.ts | 1 - src/vs/workbench/workbench.sandbox.main.ts | 1 + 8 files changed, 160 insertions(+), 94 deletions(-) rename src/vs/workbench/services/backup/{electron-browser => electron-sandbox}/backupFileService.ts (76%) diff --git a/src/vs/workbench/contrib/backup/test/electron-browser/backupTracker.test.ts b/src/vs/workbench/contrib/backup/test/electron-browser/backupTracker.test.ts index 1029ccfd112..abb2ae3bcc2 100644 --- a/src/vs/workbench/contrib/backup/test/electron-browser/backupTracker.test.ts +++ b/src/vs/workbench/contrib/backup/test/electron-browser/backupTracker.test.ts @@ -11,7 +11,7 @@ import { join } from 'vs/base/common/path'; import { rimraf, writeFile } from 'vs/base/node/pfs'; import { URI } from 'vs/base/common/uri'; import { flakySuite, getRandomTestPath } from 'vs/base/test/node/testUtils'; -import { hashPath } from 'vs/workbench/services/backup/electron-browser/backupFileService'; +import { hashPath } from 'vs/workbench/services/backup/common/backupFileService'; import { NativeBackupTracker } from 'vs/workbench/contrib/backup/electron-sandbox/backupTracker'; import { TextFileEditorModelManager } from 'vs/workbench/services/textfile/common/textFileEditorModelManager'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; diff --git a/src/vs/workbench/electron-sandbox/sandbox.simpleservices.ts b/src/vs/workbench/electron-sandbox/sandbox.simpleservices.ts index 69437ff0ee9..f888979d92d 100644 --- a/src/vs/workbench/electron-sandbox/sandbox.simpleservices.ts +++ b/src/vs/workbench/electron-sandbox/sandbox.simpleservices.ts @@ -12,8 +12,6 @@ import { Event } from 'vs/base/common/event'; import { IAddressProvider } from 'vs/platform/remote/common/remoteAgentConnection'; import { SimpleConfigurationService as BaseSimpleConfigurationService } from 'vs/editor/standalone/browser/simpleServices'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; -import { IBackupFileService, IResolvedBackup } from 'vs/workbench/services/backup/common/backup'; -import { ITextSnapshot } from 'vs/editor/common/model'; import { IExtensionService, NullExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { isWindows } from 'vs/base/common/platform'; import { IWebviewService, WebviewContentOptions, WebviewElement, WebviewExtensionDescription, WebviewOptions, WebviewOverlay } from 'vs/workbench/contrib/webview/browser/webview'; @@ -386,29 +384,6 @@ module.exports = testRunner;`); //#endregion -//#region Backup File - -class SimpleBackupFileService implements IBackupFileService { - - declare readonly _serviceBrand: undefined; - - async hasBackups(): Promise { return false; } - async discardResourceBackup(resource: URI): Promise { } - async discardAllWorkspaceBackups(): Promise { } - toBackupResource(resource: URI): URI { return resource; } - hasBackupSync(resource: URI, versionId?: number): boolean { return false; } - async getBackups(): Promise { return []; } - async resolve(resource: URI): Promise | undefined> { return undefined; } - async backup(resource: URI, content?: ITextSnapshot, versionId?: number, meta?: T): Promise { } - async discardBackup(resource: URI): Promise { } - async discardBackups(): Promise { } -} - -registerSingleton(IBackupFileService, SimpleBackupFileService); - -//#endregion - - //#region Extensions class SimpleExtensionService extends NullExtensionService { } diff --git a/src/vs/workbench/services/backup/browser/backupFileService.ts b/src/vs/workbench/services/backup/browser/backupFileService.ts index afd98b6c77f..cdc7d84d247 100644 --- a/src/vs/workbench/services/backup/browser/backupFileService.ts +++ b/src/vs/workbench/services/backup/browser/backupFileService.ts @@ -7,9 +7,6 @@ import { IFileService } from 'vs/platform/files/common/files'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { ILogService } from 'vs/platform/log/common/log'; import { BackupFileService } from 'vs/workbench/services/backup/common/backupFileService'; -import { hash } from 'vs/base/common/hash'; -import { Schemas } from 'vs/base/common/network'; -import { URI } from 'vs/base/common/uri'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IBackupFileService } from 'vs/workbench/services/backup/common/backup'; import { joinPath } from 'vs/base/common/resources'; @@ -27,12 +24,6 @@ export class BrowserBackupFileService extends BackupFileService { ) { super(joinPath(environmentService.userRoamingDataHome, 'Backups', contextService.getWorkspace().id), fileService, logService); } - - protected hashPath(resource: URI): string { - const str = resource.scheme === Schemas.file || resource.scheme === Schemas.untitled ? resource.fsPath : resource.toString(); - - return hash(str).toString(16); - } } registerSingleton(IBackupFileService, BrowserBackupFileService); diff --git a/src/vs/workbench/services/backup/common/backupFileService.ts b/src/vs/workbench/services/backup/common/backupFileService.ts index 16546d87a09..bee8c36871c 100644 --- a/src/vs/workbench/services/backup/common/backupFileService.ts +++ b/src/vs/workbench/services/backup/common/backupFileService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { join } from 'vs/base/common/path'; -import { joinPath } from 'vs/base/common/resources'; +import { basename, isEqual, joinPath } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { coalesce } from 'vs/base/common/arrays'; import { equals, deepClone } from 'vs/base/common/objects'; @@ -19,14 +19,19 @@ import { TextSnapshotReadable, stringToSnapshot } from 'vs/workbench/services/te import { Disposable } from 'vs/base/common/lifecycle'; import { ILogService } from 'vs/platform/log/common/log'; import { CancellationToken } from 'vs/base/common/cancellation'; +import { Schemas } from 'vs/base/common/network'; +import { hash } from 'vs/base/common/hash'; export interface IBackupFilesModel { - resolve(backupRoot: URI): Promise; + resolve(backupRoot: URI): Promise; + + get(): URI[]; + has(resource: URI, versionId?: number, meta?: object): boolean; add(resource: URI, versionId?: number, meta?: object): void; - has(resource: URI, versionId?: number, meta?: object): boolean; - get(): URI[]; remove(resource: URI): void; + move(source: URI, target: URI): void; + count(): number; clear(): void; @@ -43,7 +48,7 @@ export class BackupFilesModel implements IBackupFilesModel { constructor(private fileService: IFileService) { } - async resolve(backupRoot: URI): Promise { + async resolve(backupRoot: URI): Promise { try { const backupRootStat = await this.fileService.resolve(backupRoot); if (backupRootStat.children) { @@ -63,8 +68,6 @@ export class BackupFilesModel implements IBackupFilesModel { } catch (error) { // ignore any errors } - - return this; } add(resource: URI, versionId = 0, meta?: object): void { @@ -100,6 +103,14 @@ export class BackupFilesModel implements IBackupFilesModel { this.cache.delete(resource); } + move(source: URI, target: URI): void { + const entry = this.cache.get(source); + if (entry) { + this.cache.delete(source); + this.cache.set(target, entry); + } + } + clear(): void { this.cache.clear(); } @@ -119,7 +130,9 @@ export abstract class BackupFileService implements IBackupFileService { this.impl = this.initialize(backupWorkspaceHome); } - protected abstract hashPath(resource: URI): string; + private hashPath(resource: URI): string { + return hashPath(resource); + } private initialize(backupWorkspaceHome: URI | undefined): BackupFileServiceImpl | InMemoryBackupFileService { if (backupWorkspaceHome) { @@ -206,10 +219,46 @@ class BackupFileServiceImpl extends Disposable implements IBackupFileService { this.ready = this.doInitialize(); } - private doInitialize(): Promise { + private async doInitialize(): Promise { this.model = new BackupFilesModel(this.fileService); - return this.model.resolve(this.backupWorkspacePath); + // Resolve backup model + await this.model.resolve(this.backupWorkspacePath); + + // Migrate hashes as needed. We used to hash with a MD5 + // sum of the path but switched to our own simpler hash + // to avoid a node.js dependency. We still want to + // support the older hash so we: + // - iterate over all backups + // - detect if the file name length is 32 (MD5 length) + // - read the backup's target file path + // - rename the backup to the new hash + // - update the backup in our model + // + // TODO@bpasero remove me eventually + for (const backupResource of this.model.get()) { + if (basename(backupResource).length !== 32) { + continue; // not a MD5 hash, already uses new hash function + } + + try { + const resource = await this.readUri(backupResource); + if (!resource) { + this.logService.warn(`Backup: Unable to read target URI of backup ${backupResource} for migration to new hash.`); + continue; + } + + const expectedBackupResource = this.toBackupResource(resource); + if (!isEqual(expectedBackupResource, backupResource)) { + await this.fileService.move(backupResource, expectedBackupResource, true); + this.model.move(backupResource, expectedBackupResource); + } + } catch (error) { + this.logService.error(`Backup: Unable to migrate backup ${backupResource} to new hash.`); + } + } + + return this.model; } async hasBackups(): Promise { @@ -300,29 +349,31 @@ class BackupFileServiceImpl extends Disposable implements IBackupFileService { async getBackups(): Promise { const model = await this.ready; - const backups = await Promise.all(model.get().map(async backupResource => { - const backupPreamble = await this.readToMatchingString(backupResource, BackupFileServiceImpl.PREAMBLE_END_MARKER, BackupFileServiceImpl.PREAMBLE_MAX_LENGTH); - if (!backupPreamble) { - return undefined; - } - - // Preamble with metadata: URI + META-START + Meta + END - const metaStartIndex = backupPreamble.indexOf(BackupFileServiceImpl.PREAMBLE_META_SEPARATOR); - if (metaStartIndex > 0) { - return URI.parse(backupPreamble.substring(0, metaStartIndex)); - } - - // Preamble without metadata: URI + END - else { - return URI.parse(backupPreamble); - } - })); + const backups = await Promise.all(model.get().map(backupResource => this.readUri(backupResource))); return coalesce(backups); } - private async readToMatchingString(file: URI, matchingString: string, maximumBytesToRead: number): Promise { - const contents = (await this.fileService.readFile(file, { length: maximumBytesToRead })).value.toString(); + private async readUri(backupResource: URI): Promise { + const backupPreamble = await this.readToMatchingString(backupResource, BackupFileServiceImpl.PREAMBLE_END_MARKER, BackupFileServiceImpl.PREAMBLE_MAX_LENGTH); + if (!backupPreamble) { + return undefined; + } + + // Preamble with metadata: URI + META-START + Meta + END + const metaStartIndex = backupPreamble.indexOf(BackupFileServiceImpl.PREAMBLE_META_SEPARATOR); + if (metaStartIndex > 0) { + return URI.parse(backupPreamble.substring(0, metaStartIndex)); + } + + // Preamble without metadata: URI + END + else { + return URI.parse(backupPreamble); + } + } + + private async readToMatchingString(backupResource: URI, matchingString: string, maximumBytesToRead: number): Promise { + const contents = (await this.fileService.readFile(backupResource, { length: maximumBytesToRead })).value.toString(); const matchingStringIndex = contents.indexOf(matchingString); if (matchingStringIndex >= 0) { @@ -449,3 +500,12 @@ export class InMemoryBackupFileService implements IBackupFileService { return URI.file(join(resource.scheme, this.hashPath(resource))); } } + +/* + * Exported only for testing + */ +export function hashPath(resource: URI): string { + const str = resource.scheme === Schemas.file || resource.scheme === Schemas.untitled ? resource.fsPath : resource.toString(); + + return hash(str).toString(16); +} diff --git a/src/vs/workbench/services/backup/electron-browser/backupFileService.ts b/src/vs/workbench/services/backup/electron-sandbox/backupFileService.ts similarity index 76% rename from src/vs/workbench/services/backup/electron-browser/backupFileService.ts rename to src/vs/workbench/services/backup/electron-sandbox/backupFileService.ts index efbbeec9e0a..5f4dbeff4b6 100644 --- a/src/vs/workbench/services/backup/electron-browser/backupFileService.ts +++ b/src/vs/workbench/services/backup/electron-sandbox/backupFileService.ts @@ -5,8 +5,6 @@ import { BackupFileService } from 'vs/workbench/services/backup/common/backupFileService'; import { URI } from 'vs/base/common/uri'; -import { Schemas } from 'vs/base/common/network'; -import * as crypto from 'crypto'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IBackupFileService } from 'vs/workbench/services/backup/common/backup'; import { IFileService } from 'vs/platform/files/common/files'; @@ -22,19 +20,6 @@ export class NativeBackupFileService extends BackupFileService { ) { super(environmentService.configuration.backupPath ? URI.file(environmentService.configuration.backupPath).with({ scheme: environmentService.userRoamingDataHome.scheme }) : undefined, fileService, logService); } - - protected hashPath(resource: URI): string { - return hashPath(resource); - } -} - -/* - * Exported only for testing - */ -export function hashPath(resource: URI): string { - const str = resource.scheme === Schemas.file || resource.scheme === Schemas.untitled ? resource.fsPath : resource.toString(); - - return crypto.createHash('md5').update(str).digest('hex'); } registerSingleton(IBackupFileService, NativeBackupFileService); diff --git a/src/vs/workbench/services/backup/test/electron-browser/backupFileService.test.ts b/src/vs/workbench/services/backup/test/electron-browser/backupFileService.test.ts index 7bdf144cad8..7b979c03b49 100644 --- a/src/vs/workbench/services/backup/test/electron-browser/backupFileService.test.ts +++ b/src/vs/workbench/services/backup/test/electron-browser/backupFileService.test.ts @@ -5,13 +5,12 @@ import * as assert from 'assert'; import { isWindows } from 'vs/base/common/platform'; -import { createHash } from 'crypto'; import { tmpdir } from 'os'; -import { promises, existsSync, readFileSync, writeFileSync } from 'fs'; +import { promises, existsSync, readFileSync, writeFileSync, mkdirSync } from 'fs'; import { dirname, join } from 'vs/base/common/path'; import { readdirSync, rimraf, writeFile } from 'vs/base/node/pfs'; import { URI } from 'vs/base/common/uri'; -import { BackupFilesModel } from 'vs/workbench/services/backup/common/backupFileService'; +import { BackupFilesModel, hashPath } from 'vs/workbench/services/backup/common/backupFileService'; import { createTextBufferFactory } from 'vs/editor/common/model/textModel'; import { createTextModel } from 'vs/editor/test/common/editorTestUtils'; import { getRandomTestPath } from 'vs/base/test/node/testUtils'; @@ -23,13 +22,15 @@ import { DiskFileSystemProvider } from 'vs/platform/files/node/diskFileSystemPro import { NativeWorkbenchEnvironmentService } from 'vs/workbench/services/environment/electron-browser/environmentService'; import { snapshotToString } from 'vs/workbench/services/textfile/common/textfiles'; import { IFileService } from 'vs/platform/files/common/files'; -import { hashPath, NativeBackupFileService } from 'vs/workbench/services/backup/electron-browser/backupFileService'; +import { NativeBackupFileService } from 'vs/workbench/services/backup/electron-sandbox/backupFileService'; import { FileUserDataProvider } from 'vs/workbench/services/userData/common/fileUserDataProvider'; import { VSBuffer } from 'vs/base/common/buffer'; import { TestWorkbenchConfiguration } from 'vs/workbench/test/electron-browser/workbenchTestServices'; import { TestProductService } from 'vs/workbench/test/browser/workbenchTestServices'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; import { insert } from 'vs/base/common/arrays'; +import { hash } from 'vs/base/common/hash'; +import { isEqual } from 'vs/base/common/resources'; class TestWorkbenchEnvironmentService extends NativeWorkbenchEnvironmentService { @@ -118,6 +119,7 @@ suite('BackupFileService', () => { let fooBackupPath: string; let barBackupPath: string; let untitledBackupPath: string; + let customFileBackupPath: string; let service: NodeTestBackupFileService; @@ -134,9 +136,10 @@ suite('BackupFileService', () => { backupHome = join(testDir, 'Backups'); workspacesJsonPath = join(backupHome, 'workspaces.json'); workspaceBackupPath = join(backupHome, hashPath(workspaceResource)); - fooBackupPath = join(workspaceBackupPath, 'file', hashPath(fooFile)); - barBackupPath = join(workspaceBackupPath, 'file', hashPath(barFile)); - untitledBackupPath = join(workspaceBackupPath, 'untitled', hashPath(untitledFile)); + fooBackupPath = join(workspaceBackupPath, fooFile.scheme, hashPath(fooFile)); + barBackupPath = join(workspaceBackupPath, barFile.scheme, hashPath(barFile)); + untitledBackupPath = join(workspaceBackupPath, untitledFile.scheme, hashPath(untitledFile)); + customFileBackupPath = join(workspaceBackupPath, customFile.scheme, hashPath(customFile)); service = new NodeTestBackupFileService(testDir, workspaceBackupPath); @@ -157,8 +160,8 @@ suite('BackupFileService', () => { }); const actual = hashPath(uri); // If these hashes change people will lose their backed up files! - assert.strictEqual(actual, '13264068d108c6901b3592ea654fcd57'); - assert.strictEqual(actual, createHash('md5').update(uri.fsPath).digest('hex')); + assert.strictEqual(actual, '-7f9c1a2e'); + assert.strictEqual(actual, hash(uri.fsPath).toString(16)); }); test('should correctly hash the path for file scheme URIs', () => { @@ -166,11 +169,22 @@ suite('BackupFileService', () => { const actual = hashPath(uri); // If these hashes change people will lose their backed up files! if (isWindows) { - assert.strictEqual(actual, 'dec1a583f52468a020bd120c3f01d812'); + assert.strictEqual(actual, '20ffaa13'); } else { - assert.strictEqual(actual, '1effb2475fcfba4f9e8b8a1dbc8f3caf'); + assert.strictEqual(actual, '20eb3560'); } - assert.strictEqual(actual, createHash('md5').update(uri.fsPath).digest('hex')); + assert.strictEqual(actual, hash(uri.fsPath).toString(16)); + }); + + test('should correctly hash the path for custom scheme URIs', () => { + const uri = URI.from({ + scheme: 'vscode-custom', + path: 'somePath' + }); + const actual = hashPath(uri); + // If these hashes change people will lose their backed up files! + assert.strictEqual(actual, '-44972d98'); + assert.strictEqual(actual, hash(uri.toString()).toString(16)); }); }); @@ -636,6 +650,11 @@ suite('BackupFileService', () => { assert.strictEqual(model.has(resource4), true); assert.strictEqual(model.has(resource4, undefined, { foo: 'bar' }), true); assert.strictEqual(model.has(resource4, undefined, { bar: 'foo' }), false); + + const resource5 = URI.file('test4.html'); + model.move(resource4, resource5); + assert.strictEqual(model.has(resource4), false); + assert.strictEqual(model.has(resource5), true); }); test('resolve', async () => { @@ -643,8 +662,8 @@ suite('BackupFileService', () => { writeFileSync(fooBackupPath, 'foo'); const model = new BackupFilesModel(service.fileService); - const resolvedModel = await model.resolve(URI.file(workspaceBackupPath)); - assert.strictEqual(resolvedModel.has(URI.file(fooBackupPath)), true); + await model.resolve(URI.file(workspaceBackupPath)); + assert.strictEqual(model.has(URI.file(fooBackupPath)), true); }); test('get', () => { @@ -664,4 +683,40 @@ suite('BackupFileService', () => { }); }); + suite('Hash migration', () => { + + test('works', async () => { + + // Prepare backups of the old MD5 hash format + mkdirSync(join(workspaceBackupPath, fooFile.scheme), { recursive: true }); + mkdirSync(join(workspaceBackupPath, untitledFile.scheme), { recursive: true }); + mkdirSync(join(workspaceBackupPath, customFile.scheme), { recursive: true }); + writeFileSync(join(workspaceBackupPath, fooFile.scheme, '8a8589a2f1c9444b89add38166f50229'), `${fooFile.toString()}\ntest file`); + writeFileSync(join(workspaceBackupPath, untitledFile.scheme, '13264068d108c6901b3592ea654fcd57'), `${untitledFile.toString()}\ntest untitled`); + writeFileSync(join(workspaceBackupPath, customFile.scheme, 'bf018572af7b38746b502893bd0adf6c'), `${customFile.toString()}\ntest custom`); + + service.reinitialize(URI.file(workspaceBackupPath)); + + const backups = await service.getBackups(); + assert.strictEqual(backups.length, 3); + assert.ok(backups.some(backup => isEqual(backup, fooFile))); + assert.ok(backups.some(backup => isEqual(backup, untitledFile))); + assert.ok(backups.some(backup => isEqual(backup, customFile))); + + assert.strictEqual(readdirSync(join(workspaceBackupPath, fooFile.scheme)).length, 1); + assert.strictEqual(existsSync(fooBackupPath), true); + assert.strictEqual(readFileSync(fooBackupPath).toString(), `${fooFile.toString()}\ntest file`); + assert.ok(service.hasBackupSync(fooFile)); + + assert.strictEqual(readdirSync(join(workspaceBackupPath, untitledFile.scheme)).length, 1); + assert.strictEqual(existsSync(untitledBackupPath), true); + assert.strictEqual(readFileSync(untitledBackupPath).toString(), `${untitledFile.toString()}\ntest untitled`); + assert.ok(service.hasBackupSync(untitledFile)); + + assert.strictEqual(readdirSync(join(workspaceBackupPath, customFile.scheme)).length, 1); + assert.strictEqual(existsSync(customFileBackupPath), true); + assert.strictEqual(readFileSync(customFileBackupPath).toString(), `${customFile.toString()}\ntest custom`); + assert.ok(service.hasBackupSync(customFile)); + }); + }); }); diff --git a/src/vs/workbench/workbench.desktop.main.ts b/src/vs/workbench/workbench.desktop.main.ts index 30b1cf11730..f5a069507f2 100644 --- a/src/vs/workbench/workbench.desktop.main.ts +++ b/src/vs/workbench/workbench.desktop.main.ts @@ -60,7 +60,6 @@ import 'vs/workbench/electron-browser/desktop.main'; import 'vs/workbench/services/search/electron-browser/searchService'; import 'vs/workbench/services/textfile/electron-browser/nativeTextFileService'; import 'vs/workbench/services/extensions/electron-browser/extensionService'; -import 'vs/workbench/services/backup/electron-browser/backupFileService'; import 'vs/workbench/services/remote/electron-browser/tunnelServiceImpl'; diff --git a/src/vs/workbench/workbench.sandbox.main.ts b/src/vs/workbench/workbench.sandbox.main.ts index 6c530dbf49e..08d287636af 100644 --- a/src/vs/workbench/workbench.sandbox.main.ts +++ b/src/vs/workbench/workbench.sandbox.main.ts @@ -64,6 +64,7 @@ import 'vs/workbench/services/timer/electron-sandbox/timerService'; import 'vs/workbench/services/environment/electron-sandbox/shellEnvironmentService'; import 'vs/workbench/services/integrity/electron-sandbox/integrityService'; import 'vs/platform/checksum/electron-sandbox/checksumService'; +import 'vs/workbench/services/backup/electron-sandbox/backupFileService'; import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { IUserDataInitializationService, UserDataInitializationService } from 'vs/workbench/services/userData/browser/userDataInit';