Skip to content

Commit

Permalink
workspace: do not load an invalid workspace file
Browse files Browse the repository at this point in the history 
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 <paul.marechal@ericsson.com>
Co-authored-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
  • Loading branch information
@paul-marechal @vince-fugnitto
paul-marechal and vince-fugnitto committed Jun 2, 2020
1 parent 586180a commit d5507de
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 29 deletions.
73 changes: 53 additions & 20 deletions packages/workspace/src/browser/workspace-service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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, <any>'updateTitle').callsFake(() => { });
Expand Down Expand Up @@ -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 = <FileStat>{
uri: workspaceFileUri,
Expand Down Expand Up @@ -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 = <FileStat>{
uri: workspaceFileUri,
Expand Down Expand Up @@ -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 = <FileStat>{
uri: workspaceFileUri,
lastModification: 0,
Expand Down Expand Up @@ -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'] = <FileStat>{
uri,
lastModification: 0,
Expand Down Expand Up @@ -480,7 +482,7 @@ describe('WorkspaceService', () => {

it('should throw an error if the added uri points to a file instead of a folder', done => {
(<sinon.SinonStub>mockFilesystem.getFileStat).resolves(<FileStat>{
uri: 'file:///home/file',
uri: 'file:///home/file.theia-workspace',
lastModification: 0,
isDirectory: false
});
Expand Down Expand Up @@ -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 = <FileStat>{
uri: 'file:///home/file',
uri: 'file:///home/file.theia-workspace',
lastModification: 0,
isDirectory: false
};
Expand All @@ -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 = <FileStat>{
uri: 'file:///home/file',
uri: 'file:///home/file.theia-workspace',
lastModification: 0,
isDirectory: false
};
Expand All @@ -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 = <FileStat>{
uri: 'file:///home/oldfile',
uri: 'file:///home/oldfile.theia-workspace',
lastModification: 0,
isDirectory: false
};
const newFile = <FileStat>{
uri: 'file:///home/newfile',
uri: 'file:///home/newfile.theia-workspace',
lastModification: 0,
isDirectory: false
};
Expand All @@ -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 = <FileStat>{
uri: 'file:///home/oldFolder/oldFile',
uri: 'file:///home/oldFolder/oldFile.theia-workspace',
lastModification: 0,
isDirectory: false
};
const newFile = <FileStat>{
uri: 'file:///home/newFolder/newFile',
uri: 'file:///home/newFolder/newFile.theia-workspace',
lastModification: 0,
isDirectory: false
};
const folder1 = <FileStat>{
uri: 'file:///home/thirdFolder/folder1',
uri: 'file:///home/thirdFolder/folder1.theia-workspace',
lastModification: 0,
isDirectory: true
};
const folder2 = <FileStat>{
uri: 'file:///home/newFolder/folder2',
uri: 'file:///home/newFolder/folder2.theia-workspace',
lastModification: 0,
isDirectory: true
};
Expand Down Expand Up @@ -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 = <FileStat>{
uri: 'file:///home/file',
uri: 'file:///home/file.theia-workspace',
lastModification: 0,
isDirectory: false
};
Expand All @@ -647,7 +649,7 @@ describe('WorkspaceService', () => {
expect(wsService.isMultiRootWorkspaceEnabled).to.be.false;

const file = <FileStat>{
uri: 'file:///home/file',
uri: 'file:///home/file.theia-workspace',
lastModification: 0,
isDirectory: false
};
Expand All @@ -665,7 +667,7 @@ describe('WorkspaceService', () => {
expect(wsService.isMultiRootWorkspaceOpened).to.be.false;

const file = <FileStat>{
uri: 'file:///home/file',
uri: 'file:///home/file.theia-workspace',
lastModification: 0,
isDirectory: false
};
Expand Down Expand Up @@ -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 = <FileStat>{
uri: 'file:///home/oneFile',
uri: 'file:///home/oneFile.theia-workspace',
lastModification: 0,
isDirectory: false
};
Expand Down Expand Up @@ -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 = <FileStat>{
uri: workspaceFileUri,
lastModification: 0,
Expand Down Expand Up @@ -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 = <FileStat>{
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 = <FileStat>{
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 = <FileStat>{
uri: uri,
lastModification: 0,
isDirectory: false
};
expect(await wsService['isWorkspaceFile'](stat)).equal(false);
});
});

});
48 changes: 39 additions & 9 deletions packages/workspace/src/browser/workspace-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -102,11 +105,21 @@ export class WorkspaceService implements FrontendApplicationContribution {
* to a non-existing location.
*/
protected getDefaultWorkspaceUri(): MaybePromise<string | undefined> {
return this.doGetDefaultWorkspaceUri();
}

protected async doGetDefaultWorkspaceUri(): Promise<string | undefined> {
// 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).
Expand Down Expand Up @@ -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.`);
}
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<boolean> {
return fileStat.uri.endsWith(`.${THEIA_EXT}`) || fileStat.uri.endsWith(`.${VSCODE_EXT}`);
}

}

export interface WorkspaceInput {
Expand Down

0 comments on commit d5507de

Please sign in to comment.