From a4add6ac2efb29608f00c7abe4e02fcae2621c53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20Mar=C3=A9chal?= Date: Thu, 28 May 2020 12:24:19 -0400 Subject: [PATCH] workspace: do not load an invalid workspace file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace service will fail to open a workspace when the specified workspace file is not actually one. This commit prevents Theia from loading invalid workspace files. Signed-off-by: Paul Maréchal Co-authored-by: Vincent Fugnitto --- .../src/browser/workspace-service.spec.ts | 73 ++++++++++++++----- .../src/browser/workspace-service.ts | 48 +++++++++--- 2 files changed, 92 insertions(+), 29 deletions(-) diff --git a/packages/workspace/src/browser/workspace-service.spec.ts b/packages/workspace/src/browser/workspace-service.spec.ts index f0e73c1bbefbd..e6e56ea2fda04 100644 --- a/packages/workspace/src/browser/workspace-service.spec.ts +++ b/packages/workspace/src/browser/workspace-service.spec.ts @@ -27,12 +27,13 @@ import { WindowService } from '@theia/core/lib/browser/window/window-service'; import { DefaultWindowService } from '@theia/core/lib/browser/window/default-window-service'; import { EnvVariablesServer } from '@theia/core/lib/common/env-variables'; import { MockEnvVariablesServerImpl } from '@theia/core/lib/browser/test/mock-env-variables-server'; -import { WorkspaceServer } from '../common'; +import { WorkspaceServer, THEIA_EXT, VSCODE_EXT } from '../common'; import { DefaultWorkspaceServer } from '../node/default-workspace-server'; import { Emitter, Disposable, DisposableCollection, ILogger, Logger } from '@theia/core'; import { PreferenceServiceImpl, PreferenceSchemaProvider } from '@theia/core/lib/browser'; import { WorkspacePreferences } from './workspace-preferences'; import { createMockPreferenceProxy } from '@theia/core/lib/browser/preferences/test'; +import { MessageService } from '@theia/core/lib/common'; import * as jsoncparser from 'jsonc-parser'; import * as sinon from 'sinon'; import * as chai from 'chai'; @@ -116,6 +117,7 @@ describe('WorkspaceService', () => { testContainer.bind(EnvVariablesServer).toConstantValue(new MockEnvVariablesServerImpl(FileUri.create(track.mkdirSync()))); testContainer.bind(PreferenceServiceImpl).toConstantValue(mockPreferenceServiceImpl); testContainer.bind(PreferenceSchemaProvider).toConstantValue(mockPreferenceSchemaProvider); + testContainer.bind(MessageService).toConstantValue({} as MessageService); // stub the updateTitle() & reloadWindow() function because `document` and `window` are unavailable updateTitleStub = sinon.stub(WorkspaceService.prototype, 'updateTitle').callsFake(() => { }); @@ -193,7 +195,7 @@ describe('WorkspaceService', () => { 'should set the exposed roots and workspace to the folders listed in the workspace file returned by the server, ' + 'and start watching the workspace file and all the folders', async () => { - const workspaceFilePath = '/home/workspaceFile'; + const workspaceFilePath = '/home/workspaceFile.theia-workspace'; const workspaceFileUri = 'file://' + workspaceFilePath; const workspaceFileStat = { uri: workspaceFileUri, @@ -232,7 +234,7 @@ describe('WorkspaceService', () => { it( 'should resolve a relative workspace root path to a normalized root path', async () => { - const workspaceFilePath = '/home/workspaceFile'; + const workspaceFilePath = '/home/workspaceFile.theia-workspace'; const workspaceFileUri = 'file://' + workspaceFilePath; const workspaceFileStat = { uri: workspaceFileUri, @@ -262,7 +264,7 @@ describe('WorkspaceService', () => { }); it('should set the exposed roots an empty array if the workspace file stores invalid workspace data', async () => { - const workspaceFileUri = 'file:///home/workspaceFile'; + const workspaceFileUri = 'file:///home/workspaceFile.theia-workspace'; const workspaceFileStat = { uri: workspaceFileUri, lastModification: 0, @@ -313,7 +315,7 @@ describe('WorkspaceService', () => { }); it('should send server the uri of current workspace if there is workspace opened', () => { - const uri = 'file:///home/testUri'; + const uri = 'file:///home/testUri.theia-workspace'; wsService['_workspace'] = { uri, lastModification: 0, @@ -480,7 +482,7 @@ describe('WorkspaceService', () => { it('should throw an error if the added uri points to a file instead of a folder', done => { (mockFilesystem.getFileStat).resolves({ - uri: 'file:///home/file', + uri: 'file:///home/file.theia-workspace', lastModification: 0, isDirectory: false }); @@ -510,7 +512,7 @@ describe('WorkspaceService', () => { it('should write new data into the workspace file when the workspace data is stored in a file', async () => { const workspaceFileStat = { - uri: 'file:///home/file', + uri: 'file:///home/file.theia-workspace', lastModification: 0, isDirectory: false }; @@ -531,7 +533,7 @@ describe('WorkspaceService', () => { describe('save() function', () => { it('should leave the current workspace unchanged if the passed in uri points to the current workspace', async () => { const file = { - uri: 'file:///home/file', + uri: 'file:///home/file.theia-workspace', lastModification: 0, isDirectory: false }; @@ -552,12 +554,12 @@ describe('WorkspaceService', () => { it('should create a new workspace file, save the workspace data into that new file, and update the title of theia', async () => { const oldFile = { - uri: 'file:///home/oldfile', + uri: 'file:///home/oldfile.theia-workspace', lastModification: 0, isDirectory: false }; const newFile = { - uri: 'file:///home/newfile', + uri: 'file:///home/newfile.theia-workspace', lastModification: 0, isDirectory: false }; @@ -582,22 +584,22 @@ describe('WorkspaceService', () => { it('should use relative paths or translate relative paths to absolute path when necessary before saving, and emit "savedLocationChanged" event', done => { const oldFile = { - uri: 'file:///home/oldFolder/oldFile', + uri: 'file:///home/oldFolder/oldFile.theia-workspace', lastModification: 0, isDirectory: false }; const newFile = { - uri: 'file:///home/newFolder/newFile', + uri: 'file:///home/newFolder/newFile.theia-workspace', lastModification: 0, isDirectory: false }; const folder1 = { - uri: 'file:///home/thirdFolder/folder1', + uri: 'file:///home/thirdFolder/folder1.theia-workspace', lastModification: 0, isDirectory: true }; const folder2 = { - uri: 'file:///home/newFolder/folder2', + uri: 'file:///home/newFolder/folder2.theia-workspace', lastModification: 0, isDirectory: true }; @@ -628,7 +630,7 @@ describe('WorkspaceService', () => { describe('saved status', () => { it('should be true if there is an opened workspace, and the opened workspace is not a folder, otherwise false', () => { const file = { - uri: 'file:///home/file', + uri: 'file:///home/file.theia-workspace', lastModification: 0, isDirectory: false }; @@ -647,7 +649,7 @@ describe('WorkspaceService', () => { expect(wsService.isMultiRootWorkspaceEnabled).to.be.false; const file = { - uri: 'file:///home/file', + uri: 'file:///home/file.theia-workspace', lastModification: 0, isDirectory: false }; @@ -665,7 +667,7 @@ describe('WorkspaceService', () => { expect(wsService.isMultiRootWorkspaceOpened).to.be.false; const file = { - uri: 'file:///home/file', + uri: 'file:///home/file.theia-workspace', lastModification: 0, isDirectory: false }; @@ -731,9 +733,9 @@ describe('WorkspaceService', () => { expect(stubWriteWorkspaceFile.called).to.be.false; }); - it('should update the working space file with remaining folders', async () => { + it('should update the workspace file with remaining folders', async () => { const file = { - uri: 'file:///home/oneFile', + uri: 'file:///home/oneFile.theia-workspace', lastModification: 0, isDirectory: false }; @@ -854,7 +856,7 @@ describe('WorkspaceService', () => { }).timeout(2000); it('should emit updated roots when workspace file is changed', done => { - const workspaceFileUri = 'file:///home/workspaceFile'; + const workspaceFileUri = 'file:///home/workspaceFile.theia-workspace'; const workspaceFileStat = { uri: workspaceFileUri, lastModification: 0, @@ -899,4 +901,35 @@ describe('WorkspaceService', () => { mockFileChangeEmitter.fire([{ uri: new URI(workspaceFileUri), type: FileChangeType.UPDATED }]); }); }).timeout(2000); + + describe('isWorkspaceFile()', () => { + it(`should return true for '${THEIA_EXT}' files`, async () => { + const uri = `/home/foo/bar.${THEIA_EXT}`; + const stat = { + uri: uri, + lastModification: 0, + isDirectory: false + }; + expect(await wsService['isWorkspaceFile'](stat)).equal(true); + }); + it(`should return true for '${VSCODE_EXT}' files`, async () => { + const uri = `/home/foo/bar.${VSCODE_EXT}`; + const stat = { + uri: uri, + lastModification: 0, + isDirectory: false + }; + expect(await wsService['isWorkspaceFile'](stat)).equal(true); + }); + it(`should return false if not a '${THEIA_EXT}' or '${VSCODE_EXT} file`, async () => { + const uri = '/home/foo/foo.bar'; + const stat = { + uri: uri, + lastModification: 0, + isDirectory: false + }; + expect(await wsService['isWorkspaceFile'](stat)).equal(false); + }); + }); + }); diff --git a/packages/workspace/src/browser/workspace-service.ts b/packages/workspace/src/browser/workspace-service.ts index ace89e95c4f78..a184dffa9b5d8 100644 --- a/packages/workspace/src/browser/workspace-service.ts +++ b/packages/workspace/src/browser/workspace-service.ts @@ -25,7 +25,7 @@ import { } from '@theia/core/lib/browser'; import { Deferred } from '@theia/core/lib/common/promise-util'; import { EnvVariablesServer } from '@theia/core/lib/common/env-variables'; -import { ILogger, Disposable, DisposableCollection, Emitter, Event, MaybePromise } from '@theia/core'; +import { ILogger, Disposable, DisposableCollection, Emitter, Event, MaybePromise, MessageService } from '@theia/core'; import { WorkspacePreferences } from './workspace-preferences'; import * as jsoncparser from 'jsonc-parser'; import * as Ajv from 'ajv'; @@ -69,6 +69,9 @@ export class WorkspaceService implements FrontendApplicationContribution { @inject(EnvVariablesServer) protected readonly envVariableServer: EnvVariablesServer; + @inject(MessageService) + protected readonly messageService: MessageService; + protected applicationName: string; @postConstruct() @@ -102,11 +105,21 @@ export class WorkspaceService implements FrontendApplicationContribution { * to a non-existing location. */ protected getDefaultWorkspaceUri(): MaybePromise { + return this.doGetDefaultWorkspaceUri(); + } + + protected async doGetDefaultWorkspaceUri(): Promise { // Prefer the workspace path specified as the URL fragment, if present. if (window.location.hash.length > 1) { // Remove the leading # and decode the URI. const wpPath = decodeURI(window.location.hash.substring(1)); - return new URI().withPath(wpPath).withScheme('file').toString(); + const workspaceUri = new URI().withPath(wpPath).withScheme('file'); + const workspaceStat = await this.fileSystem.getFileStat(workspaceUri.toString()); + if (workspaceStat && !workspaceStat.isDirectory && !await this.isWorkspaceFile(workspaceStat)) { + this.messageService.error(`Not a valid workspace file: ${workspaceUri}`); + return undefined; + } + return workspaceUri.toString(); } else { // Else, ask the server for its suggested workspace (usually the one // specified on the CLI, or the most recent). @@ -229,14 +242,17 @@ export class WorkspaceService implements FrontendApplicationContribution { return { folders: [{ path: this._workspace.uri }] }; + } else if (await this.isWorkspaceFile(this._workspace)) { + const { stat, content } = await this.fileSystem.resolveContent(this._workspace.uri); + const strippedContent = jsoncparser.stripComments(content); + const data = jsoncparser.parse(strippedContent); + if (data && WorkspaceData.is(data)) { + return WorkspaceData.transformToAbsolute(data, stat); + } + this.logger.error(`Unable to retrieve workspace data from the file: '${this._workspace.uri}'. Please check if the file is corrupted.`); + } else { + this.logger.warn(`Not a valid workspace file: ${this._workspace.uri}`); } - const { stat, content } = await this.fileSystem.resolveContent(this._workspace.uri); - const strippedContent = jsoncparser.stripComments(content); - const data = jsoncparser.parse(strippedContent); - if (data && WorkspaceData.is(data)) { - return WorkspaceData.transformToAbsolute(data, stat); - } - this.logger.error(`Unable to retrieve workspace data from the file: '${this._workspace.uri}'. Please check if the file is corrupted.`); } } @@ -306,6 +322,11 @@ export class WorkspaceService implements FrontendApplicationContribution { const rootUri = uri.toString(); const stat = await this.toFileStat(rootUri); if (stat) { + if (!stat.isDirectory && !await this.isWorkspaceFile(stat)) { + const message = `Not a valid workspace file: ${uri}`; + this.messageService.error(message); + throw new Error(message); + } // The same window has to be preserved too (instead of opening a new one), if the workspace root is not yet available and we are setting it for the first time. // Option passed as parameter has the highest priority (for api developers), then the preference, then the default. await this.roots; @@ -587,6 +608,15 @@ export class WorkspaceService implements FrontendApplicationContribution { return uris.every(uri => rootUris.has(uri.toString())); } + /** + * Check if the file should be considered as a workspace file. + * + * Example: We should not try to read the contents of an .exe file. + */ + protected async isWorkspaceFile(fileStat: FileStat): Promise { + return fileStat.uri.endsWith(`.${THEIA_EXT}`) || fileStat.uri.endsWith(`.${VSCODE_EXT}`); + } + } export interface WorkspaceInput {