Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workspace: do not load an invalid workspace file #7922

Merged
merged 1 commit into from
Jun 2, 2020
Merged

workspace: do not load an invalid workspace file #7922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 53 additions & 20 deletions packages/workspace/src/browser/workspace-service.spec.ts
View file
Open in desktop
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
Open in desktop
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