Repository navigation
get-childitem env: fails on Windows when an environment variable is defined twice, cased differently #6305
Description
Activity
Does not repro on macOS
Here is the fix we made for macOS/Linux:
Update: Linux/macOS actually support this scenario... Windows does not. This would only mask the issue NPM is creating.
We would need to verify that making this change for windows would not be breaking.
#6320 does fix the issue, but I suggest resolving it differently:
That
npmeven manages to create a separate variable that differs by case only is unfortunate - given that environment variable names are case-insensitive (but case-preserving) in all other contexts I am aware of.In effect, however, there's only one environment variable on retrieval by name, and - irrespective of what case variation in the name was used last -
the last value assigned wins - from what I can tell.Thus, I suggest sticking with the case-insensitive dictionary and simply replacing the value of the existing dictionary entry if a case variation is encountered in the process of enumerating
Environment.GetEnvironmentVariables()There is no benefit in listing all case variations of a given name - all it creates is confusion, given that the values of
earlier[other] case variants are unreachable by name.Michael Klement (@mklement0)
I agreed that there is little to no value retrieving both values. I'm open to the change if the cost of the change is justified, but the cost of the change is increased complexity and therefore maintenance.Our provider remains case-insensitive, case-preserving. You can never assign two variables with the same case (due to dotnet behavior and we have tests to verify this.) I chose this implementation over the other implementations because it was simpler with the only side effect occurring during the scenario we are fixing.
Fixing this while preserving the case insensitive dictionary would require additional specialized logic for windows, while this fix removed the specialized logic for windows. I believe the side-effects are reasonable unless I'm incorrect about them being limited to the scenario being fixed.
Travis Plunk (@TravisEz13): Fixing this in a Windows-friendly manner would be simple, I believe:
The only change needed is to replace:providerTable.Add((string)entry.Key, entry);
with:
try { providerTable.Add((string)entry.Key, entry); } catch (System.ArgumentException) { // Windows only: duplicate key (variable name that differs only in case) // NOTE: Even though this shouldn't happen, it can, e.g. when npm // creates duplicate environment variables that differ only in case - // see https://github.com/PowerShell/PowerShell/issues/6305. // However, because retrieval *by name* later is invariably // case-INsensitive, in effect only a *single* variable exists. // We simply ask Environment.GetEnvironmentVariable() which value is // the effective one, and use that. providerTable[(string)entry.Key] = new DictionaryEntry((string)entry.Key, Environment.GetEnvironmentVariable((string)entry.Key)); }
Note:
-
This assumes that the current distinction between a case-sensitive (Unix) and case-INsensitive dictionary (Windows) is retained.
-
No attempt is made to decide which particular case variant among the duplicate names is the "real" one associated with the "winning" value - I don't think we need to worry about that.
I've chosen the above approach because I've come to realize that it's not necessarily the definition order that determines the values that wins; here's a simplified reproduction of the original problem:
$env:FOO = 'first' node -pe @' env = {} env.PATH = process.env.PATH env.FOO = process.env.FOO env.foo = 'last' require('child_process').execSync(\"pwsh -nop -command $env:fOo; [environment]::getenvironmentvariable('fOo'); '---'; [environment]::getenvironmentvariables(); \", { env: env }).toString() '@
The above prints (note how the first definition remained in effect):
first first --- Name Value ---- ----- foo last USERPROFILE C:\Users\jdoe PROMPT $P$G PSModulePath C:\Users\jdoe\Documents\PowerShell\Modules;C:\Program Files\PowerShell\Modules;c:\program files\powershell\6.0.1\Modules LOGONSERVER \\MK-W10 COMSPEC C:\WINDOWS\system32\cmd.exe FOO first SYSTEMROOT C:\WINDOWS WINDIR C:\WINDOWS SYSTEMDRIVE C: HOMEPATH \Users\jdoe PATHEXT .COM;.EXE;.BAT;.CMD;.VBS;.JS;.WS;.MSC PATH C:\ProgramData\Oracle\Java\javapath;C:\Program Files\CMake\bin;C:\tools\ruby23\bin;C:\Python35\Scripts\;C:\Python35\;C:\WINDOWS\system32;C:\WINDO... USERDOMAIN JDOE-W10 TEMP C:\Users\jdoe\AppData\Local\Temp USERNAME jdoe HOMEDRIVE C:-
Michael Klement (@mklement0) The fix you were suggesting was easy enough to understand. The question is what is the benefit. Let us review the differences between the two changes:
My fix your fix case sensitivity Unaltered Unaltered what show up Both items One Item (potentially hiding a value) complexity versus original Reduced Increased So, is only seeing one item (hiding one value) in the environment provider really worth the increased complexity.
Your example also shows that one value could be hidden with your fix.
I hope we can agree that complexity of the implementation should not be the driving factor here, given that even my relatively more complex fix is, in the end, quite simple.
If we focus on the user experience:
-
My approach hides a value that for all intents and purposes doesn't exist, so hiding it is a good thing and indeed the main purpose of my fix.
-
Conversely, not hiding it is apt to create confusion - which value is in effect?
-
If someone truly wanted to get at the hidden value - and I don't see why - they can still use
[Environment]::GetEnvironmentVariables().
-
Let's consider a concrete example of the side effects of not hiding the case variants:
-
Assume that a "rogue" application has created case variants
FOO_bar,foo_barandfOo_bar, with distinct values1,2,3(to recap: only one of which is actually visible if you acess$env:foo_barin any case variation). -
Additionally, assume the existence of (single) var.
foo_otherwith value4. -
Now I want to get the values of these variables using a wildcard expression with
Get-Item:
# With your fix, you'd get *4* matches > Get-Item env:foo_* Name Value ---- ----- FOO_bar 1 foo_bar 2 fOo_bar 3 foo_other 4
Now, which one is the
foo_barvalue I'm looking for, which one is actually in effect for a lookup by name?
Even if you know what the underlying problem is, you would (a) have to anticipate it every time and (b) work around it in a cumbersome fashion.-
Michael Klement (@mklement0)
I found something that convinced me to make the change. Get-Item does become case sensitive. Thanks for your feedback.PS > node -pe @' >> env = {} >> env.FOO = 'first' >> require('child_process').execSync(\"pwsh -nop -command get-item env:FOO; get-item env:foo; $env:foo \", { env: en v }).toString() >> '@ Name Value ---- ----- FOO first firstReacted by Michael Klement and lukeb1961Michael Klement (@mklement0) I updated the PR. I didn't use try-catch, but the fix is equivalent. Assuming, we don't care which value ends up in the provider.
Travis Plunk (@TravisEz13): While we needn't worry what case variant of the name ends up being reported by the provider, I think it is crucial that we make sure that the value is the effective one, i.e., the one that
$env:fooreturns - hence my use ofEnvironment.GetEnvironmentVariable((string)entry.Key)to determine the value.Travis Plunk (@TravisEz13): To illustrate the previous point:
My first
nodesnippet demonstrated that it is - regrettably - possible to create case-variant-only environment variables with distinct values; e.g.,FOOwith valuefirstandfoowith valuelast.However, on retrieving the value, only one of them "wins", and we shouldn't speculate about which one it is, as that is an implementation detail (though it seems that whatever one is assigned first to the
envJS object in my example is the winner).Specifically, we want to avoid a scenario where
Get-Item env:fooreturnslast, whereas$env:fooreturnsfirst.By simply consulting
Environment.GetEnvironmentVariable((string)entry.Key)whenever a case variant is encountered during the enumeration, we ensure thatGet-Itemtoo reflects the "winning" value.The only goal here is to prevent the provider from crashing. This is beyond the scope of this issue. Please file a new issue to handle this feature request. I don't think your fix is as simple as it sounds.
The only goal here is to prevent the provider from crashing.
That being the only goal is regrettable, given that we already know that the fix you just merged introduces an insidious - if rare - inconsistency and that we've discussed a proper fix. (The rarity may actually heighten the insidiousness.)
Please file a new issue to handle this feature request.
Please see #6460
I don't think your fix is as simple as it sounds.
Please tell us why - over at #6460, which duplicates much of what we've discussed here.
All in all, not concluding the discussion here has ended up creating more work for everyone.
- added a commit that references this issue
on Mar 24, 2018 - added a commit that references this issue
on Mar 27, 2018 - added a commit that references this issue
on Apr 24, 2018 - added a commit that references this issue
on Feb 10, 2020 - added a commit that references this issue
on Aug 6, 2026
The problem:
get-childitem env:fails on Windows when an environment variable exists twice, each instance cased differently.The scenario:
On Windows, set the environment variable
NPM_CONFIG_PATHto control where the npm cache is created. And use npm to run unit tests, use powershell within a unit test, and from powershell invokeget-childitem env:. Note, it is common to usenpm testto build and test a nodejs project.It appears npm internally creates environment variables to manage state, and a lower-case variable
npm_config_pathis created. When powershell is invoked from the unit test, the environment variablesNPM_CONFIG_PATHandnpm_config_pathare both set.Steps to reproduce
On Windows, tested with npm v5.6.0 in my PATH.
Expected behavior
PowerShell should gracefully handle the duplicate environment variable. The
setbuiltin in cmd.exe handles it gracefully. .NETEnvironment.GetEnvironmentVariables()handles it gracefully.Actual behavior
Get-ChildItem fails with "An item with the same key has already been added. Key: NPM_CONFIG_PATH"
Environment data