Skip to content

Commit

Permalink
Relax validation of environment path variable (#2172)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored and brettcannon committed Jul 17, 2018
1 parent b9796bb commit 176c975
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 33 deletions.
1 change: 1 addition & 0 deletions news/2 Fixes/2076.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Relax validation of the environment `Path` variable.
6 changes: 3 additions & 3 deletions src/client/application/diagnostics/checks/envPathVariable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import { DiagnosticCodes } from '../constants';
import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../promptHandler';
import { DiagnosticScope, IDiagnostic, IDiagnosticHandlerService } from '../types';

const InvalidEnvPathVariableMessage = 'The environment variable \'{0}\' seems to have some paths containing characters (\';\', \'"\' or \';;\').' +
' The existence of such characters are known to have caused the {1} extension to not load. If the extension fails to load please modify your paths to remove these characters.';
const InvalidEnvPathVariableMessage = 'The environment variable \'{0}\' seems to have some paths containing the \'"\' character.' +
' The existence of such a character is known to have caused the {1} extension to not load. If the extension fails to load please modify your paths to remove this \'"\' character.';

export class InvalidEnvironmentPathVariableDiagnostic extends BaseDiagnostic {
constructor(message) {
Expand Down Expand Up @@ -79,6 +79,6 @@ export class EnvironmentPathVariableDiagnosticsService extends BaseDiagnosticsSe
const pathValue = currentProc.env[this.platform.pathVariableName];
const pathSeparator = this.serviceContainer.get<IPathUtils>(IPathUtils).delimiter;
const paths = pathValue.split(pathSeparator);
return paths.filter((item, index) => item.indexOf('"') >= 0 || item.indexOf(';') >= 0 || (item.length === 0 && index !== paths.length - 1)).length > 0;
return paths.filter(item => item.indexOf('"') >= 0).length > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,36 +115,34 @@ suite('Application Diagnostics - Checks Env Path Variable', () => {
expect(diagnostics).to.be.deep.equal([]);
});
// Note: On windows, when a path contains a `;` then Windows encloses the path within `"`.
[';;', '"'].forEach(invalidCharacter => {
test(`Should return single diagnostics for Windows if path contains ${invalidCharacter}`, async () => {
platformService.setup(p => p.isWindows).returns(() => true);
const paths = [
path.join('one', 'two', `three${invalidCharacter}`),
path.join('one', 'two', 'four')
].join(pathDelimiter);
procEnv.setup(env => env[pathVariableName]).returns(() => paths);

const diagnostics = await diagnosticService.diagnose();

expect(diagnostics).to.be.lengthOf(1);
expect(diagnostics[0].code).to.be.equal(DiagnosticCodes.InvalidEnvironmentPathVariableDiagnostic);
expect(diagnostics[0].message).to.contain(extensionName);
expect(diagnostics[0].message).to.contain(pathVariableName);
expect(diagnostics[0].severity).to.be.equal(DiagnosticSeverity.Warning);
expect(diagnostics[0].scope).to.be.equal(DiagnosticScope.Global);
});
test('Should not return diagnostics for Windows if path ends with delimiter', async () => {
const paths = [
path.join('one', 'two', 'three'),
path.join('one', 'two', 'four')
].join(pathDelimiter) + pathDelimiter;
platformService.setup(p => p.isWindows).returns(() => true);
procEnv.setup(env => env[pathVariableName]).returns(() => paths);

const diagnostics = await diagnosticService.diagnose();

expect(diagnostics).to.be.lengthOf(0);
});
test('Should return single diagnostics for Windows if path contains \'"\'', async () => {
platformService.setup(p => p.isWindows).returns(() => true);
const paths = [
path.join('one', 'two', 'three"'),
path.join('one', 'two', 'four')
].join(pathDelimiter);
procEnv.setup(env => env[pathVariableName]).returns(() => paths);

const diagnostics = await diagnosticService.diagnose();

expect(diagnostics).to.be.lengthOf(1);
expect(diagnostics[0].code).to.be.equal(DiagnosticCodes.InvalidEnvironmentPathVariableDiagnostic);
expect(diagnostics[0].message).to.contain(extensionName);
expect(diagnostics[0].message).to.contain(pathVariableName);
expect(diagnostics[0].severity).to.be.equal(DiagnosticSeverity.Warning);
expect(diagnostics[0].scope).to.be.equal(DiagnosticScope.Global);
});
test('Should not return diagnostics for Windows if path ends with delimiter', async () => {
const paths = [
path.join('one', 'two', 'three'),
path.join('one', 'two', 'four')
].join(pathDelimiter) + pathDelimiter;
platformService.setup(p => p.isWindows).returns(() => true);
procEnv.setup(env => env[pathVariableName]).returns(() => paths);

const diagnostics = await diagnosticService.diagnose();

expect(diagnostics).to.be.lengthOf(0);
});
test('Should display three options in message displayed with 2 commands', async () => {
platformService.setup(p => p.isWindows).returns(() => true);
Expand Down

0 comments on commit 176c975

Please sign in to comment.