From 4760b2bcc64afbbbadba90d4c8babc90715d458a Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 17 Jul 2018 15:45:09 -0700 Subject: [PATCH] Relax validation of environment path variable (#2172) --- news/2 Fixes/2076.md | 1 + .../diagnostics/checks/envPathVariable.ts | 6 +- .../checks/envPathVariable.unit.test.ts | 58 +++++++++---------- 3 files changed, 32 insertions(+), 33 deletions(-) create mode 100644 news/2 Fixes/2076.md diff --git a/news/2 Fixes/2076.md b/news/2 Fixes/2076.md new file mode 100644 index 000000000000..70ce03782a26 --- /dev/null +++ b/news/2 Fixes/2076.md @@ -0,0 +1 @@ +Relax validation of the environment `Path` variable. diff --git a/src/client/application/diagnostics/checks/envPathVariable.ts b/src/client/application/diagnostics/checks/envPathVariable.ts index 9b00ed1303f1..bccb3a29c042 100644 --- a/src/client/application/diagnostics/checks/envPathVariable.ts +++ b/src/client/application/diagnostics/checks/envPathVariable.ts @@ -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) { @@ -79,6 +79,6 @@ export class EnvironmentPathVariableDiagnosticsService extends BaseDiagnosticsSe const pathValue = currentProc.env[this.platform.pathVariableName]; const pathSeparator = this.serviceContainer.get(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; } } diff --git a/src/test/application/diagnostics/checks/envPathVariable.unit.test.ts b/src/test/application/diagnostics/checks/envPathVariable.unit.test.ts index e71efc0406cd..a2f678f1f142 100644 --- a/src/test/application/diagnostics/checks/envPathVariable.unit.test.ts +++ b/src/test/application/diagnostics/checks/envPathVariable.unit.test.ts @@ -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);