Fix debugging when using "internalConsole" (#21068)

Closes #20828 Do case-insensitive merge of environment variables, always make sure to return the standard env key for an OS, similar to `process.env`.
microsoft · Apr 17, 2023 · 5a8fdf4 · 5a8fdf4
1 parent f531bb8
commit 5a8fdf4
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 23 deletions.
diff --git a/src/client/common/platform/fs-paths.ts b/src/client/common/platform/fs-paths.ts
@@ -145,7 +145,11 @@ export class FileSystemPathUtils implements IFileSystemPathUtils {
 }
 
 export function normCasePath(filePath: string): string {
-    return getOSType() === OSType.Windows ? nodepath.normalize(filePath).toUpperCase() : nodepath.normalize(filePath);
+    return normCase(nodepath.normalize(filePath));
+}
+
+export function normCase(s: string): string {
+    return getOSType() === OSType.Windows ? s.toUpperCase() : s;
 }
 
 /**

diff --git a/src/client/common/variables/environment.ts b/src/client/common/variables/environment.ts
@@ -10,6 +10,7 @@ import { EventName } from '../../telemetry/constants';
 import { IFileSystem } from '../platform/types';
 import { IPathUtils } from '../types';
 import { EnvironmentVariables, IEnvironmentVariablesService } from './types';
+import { normCase } from '../platform/fs-paths';
 
 @injectable()
 export class EnvironmentVariablesService implements IEnvironmentVariablesService {
@@ -61,6 +62,9 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
         if (!target) {
             return;
         }
+        const reference = target;
+        target = normCaseKeys(target);
+        source = normCaseKeys(source);
         const settingsNotToMerge = ['PYTHONPATH', this.pathVariable];
         Object.keys(source).forEach((setting) => {
             if (settingsNotToMerge.indexOf(setting) >= 0) {
@@ -70,6 +74,8 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
                 target[setting] = source[setting];
             }
         });
+        restoreKeys(target);
+        matchTarget(reference, target);
     }
 
     public appendPythonPath(vars: EnvironmentVariables, ...pythonPaths: string[]) {
@@ -80,18 +86,24 @@ export class EnvironmentVariablesService implements IEnvironmentVariablesService
         return this.appendPaths(vars, this.pathVariable, ...paths);
     }
 
-    private get pathVariable(): 'Path' | 'PATH' {
+    private get pathVariable(): string {
         if (!this._pathVariable) {
             this._pathVariable = this.pathUtils.getPathVariableName();
         }
-        return this._pathVariable!;
+        return normCase(this._pathVariable)!;
     }
 
-    private appendPaths(
-        vars: EnvironmentVariables,
-        variableName: 'PATH' | 'Path' | 'PYTHONPATH',
-        ...pathsToAppend: string[]
-    ) {
+    private appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) {
+        const reference = vars;
+        vars = normCaseKeys(vars);
+        variableName = normCase(variableName);
+        vars = this._appendPaths(vars, variableName, ...pathsToAppend);
+        restoreKeys(vars);
+        matchTarget(reference, vars);
+        return vars;
+    }
+
+    private _appendPaths(vars: EnvironmentVariables, variableName: string, ...pathsToAppend: string[]) {
         const valueToAppend = pathsToAppend
             .filter((item) => typeof item === 'string' && item.trim().length > 0)
             .map((item) => item.trim())
@@ -183,3 +195,40 @@ function substituteEnvVars(
 
     return value.replace(/\\\$/g, '$');
 }
+
+export function normCaseKeys(env: EnvironmentVariables): EnvironmentVariables {
+    const normalizedEnv: EnvironmentVariables = {};
+    Object.keys(env).forEach((key) => {
+        const normalizedKey = normCase(key);
+        normalizedEnv[normalizedKey] = env[key];
+    });
+    return normalizedEnv;
+}
+
+export function restoreKeys(env: EnvironmentVariables) {
+    const processEnvKeys = Object.keys(process.env);
+    processEnvKeys.forEach((processEnvKey) => {
+        const originalKey = normCase(processEnvKey);
+        if (originalKey !== processEnvKey && env[originalKey] !== undefined) {
+            env[processEnvKey] = env[originalKey];
+            delete env[originalKey];
+        }
+    });
+}
+
+export function matchTarget(reference: EnvironmentVariables, target: EnvironmentVariables): void {
+    Object.keys(reference).forEach((key) => {
+        if (target.hasOwnProperty(key)) {
+            reference[key] = target[key];
+        } else {
+            delete reference[key];
+        }
+    });
+
+    // Add any new keys from target to reference
+    Object.keys(target).forEach((key) => {
+        if (!reference.hasOwnProperty(key)) {
+            reference[key] = target[key];
+        }
+    });
+}
diff --git a/src/client/debugger/extension/configuration/resolvers/helper.ts b/src/client/debugger/extension/configuration/resolvers/helper.ts
@@ -48,7 +48,10 @@ export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariabl
         }
 
         // Append the PYTHONPATH and PATH variables.
-        this.envParser.appendPath(env, debugLaunchEnvVars[pathVariableName]);
+        this.envParser.appendPath(
+            env,
+            debugLaunchEnvVars[pathVariableName] ?? debugLaunchEnvVars[pathVariableName.toUpperCase()],
+        );
         this.envParser.appendPythonPath(env, debugLaunchEnvVars.PYTHONPATH);
 
         if (typeof env[pathVariableName] === 'string' && env[pathVariableName]!.length > 0) {

diff --git a/src/test/common/variables/envVarsService.unit.test.ts b/src/test/common/variables/envVarsService.unit.test.ts
@@ -10,17 +10,16 @@ import * as TypeMoq from 'typemoq';
 import { IFileSystem } from '../../../client/common/platform/types';
 import { IPathUtils } from '../../../client/common/types';
 import { EnvironmentVariablesService, parseEnvFile } from '../../../client/common/variables/environment';
+import { getSearchPathEnvVarNames } from '../../../client/common/utils/exec';
 
 use(chaiAsPromised);
 
 type PathVar = 'Path' | 'PATH';
-const PATHS = [
-    'Path', // Windows
-    'PATH', // non-Windows
-];
+const PATHS = getSearchPathEnvVarNames();
 
-suite('Environment Variables Service', () => {
+suite('xEnvironment Variables Service', () => {
     const filename = 'x/y/z/.env';
+    const processEnvPath = getSearchPathEnvVarNames()[0];
     let pathUtils: TypeMoq.IMock<IPathUtils>;
     let fs: TypeMoq.IMock<IFileSystem>;
     let variablesService: EnvironmentVariablesService;
@@ -208,7 +207,7 @@ PYTHON=${BINDIR}/python3\n\
                 expect(vars2).to.have.property('TWO', 'TWO', 'Incorrect value');
                 expect(vars2).to.have.property('THREE', '3', 'Variable not merged');
                 expect(vars2).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value');
-                expect(vars2).to.have.property(pathVariable, 'PATH', 'Incorrect value');
+                expect(vars2).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
                 verifyAll();
             });
 
@@ -226,7 +225,7 @@ PYTHON=${BINDIR}/python3\n\
                 expect(target).to.have.property('TWO', 'TWO', 'Incorrect value');
                 expect(target).to.have.property('THREE', '3', 'Variable not merged');
                 expect(target).to.have.property('PYTHONPATH', 'PYTHONPATH', 'Incorrect value');
-                expect(target).to.have.property(pathVariable, 'PATH', 'Incorrect value');
+                expect(target).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
                 verifyAll();
             });
         });
@@ -266,17 +265,17 @@ PYTHON=${BINDIR}/python3\n\
                 variablesService.appendPath(vars);
                 expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
                 expect(vars).to.have.property('ONE', '1', 'Incorrect value');
-                expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
+                expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
 
                 variablesService.appendPath(vars, '');
                 expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
                 expect(vars).to.have.property('ONE', '1', 'Incorrect value');
-                expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
+                expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
 
                 variablesService.appendPath(vars, ' ', '');
                 expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
                 expect(vars).to.have.property('ONE', '1', 'Incorrect value');
-                expect(vars).to.have.property(pathVariable, 'PATH', 'Incorrect value');
+                expect(vars).to.have.property(processEnvPath, 'PATH', 'Incorrect value');
 
                 verifyAll();
             });
@@ -291,7 +290,11 @@ PYTHON=${BINDIR}/python3\n\
 
                 expect(Object.keys(vars)).lengthOf(2, 'Incorrect number of variables');
                 expect(vars).to.have.property('ONE', '1', 'Incorrect value');
-                expect(vars).to.have.property(pathVariable, `PATH${path.delimiter}${pathToAppend}`, 'Incorrect value');
+                expect(vars).to.have.property(
+                    processEnvPath,
+                    `PATH${path.delimiter}${pathToAppend}`,
+                    'Incorrect value',
+                );
                 verifyAll();
             });
         });

diff --git a/src/test/debugger/envVars.test.ts b/src/test/debugger/envVars.test.ts
@@ -15,6 +15,7 @@ import { ConsoleType, LaunchRequestArguments } from '../../client/debugger/types
 import { isOs, OSType } from '../common';
 import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize';
 import { UnitTestIocContainer } from '../testing/serviceRegistry';
+import { normCase } from '../../client/common/platform/fs-paths';
 
 use(chaiAsPromised);
 
@@ -109,9 +110,9 @@ suite('Resolving Environment Variables when Debugging', () => {
     });
 
     async function testJsonEnvVariables(console: ConsoleType, expectedNumberOfVariables: number) {
-        const prop1 = shortid.generate();
-        const prop2 = shortid.generate();
-        const prop3 = shortid.generate();
+        const prop1 = normCase(shortid.generate());
+        const prop2 = normCase(shortid.generate());
+        const prop3 = normCase(shortid.generate());
         const env: Record<string, string> = {};
         env[prop1] = prop1;
         env[prop2] = prop2;