#6320 (fix for #6305) is flawed, because it may expose incorrect environment-variable values via Get-Item #6460
Description
A follow-up from #6305 (comment), as requested by @TravisEz13:
#6320 attempts to fix #6305, but only does so superficially:
-
while it fixes the immediate problem by preventing an error from surfacing when you use
Get-item env:while case-variant environment variables are unexpectedly defined, -
it can end up exposing an effectively hidden value that neither
$env:<name>nor[environment]::GetEnvironmentVariable('<name>')see intermittently; in other words:Get-Item env:foomay intermittently report an incorrect value for$env:foo
Steps to reproduce
Note: Run this in PowerShell Core on Windows, with Node.js installed and #6320 applied (without it, Get-Item will fail, complaining about a duplicate key).
$env:FOO = 'A'
$tries = 0
do {
Write-Host '.' -NoNewline
++$tries
$res = node -pe @'
env = {}
env.fOo = 'B' // redefine with a case variant name and different value
env.FOO = process.env.FOO // also include the original case variant with its original value.
// Contrast $env:foo with Get-Item env:foo
require('child_process').execSync(\"pwsh -nop -command $env:foo; (Get-Item env:foo).Value\", { env: env }).toString()
'@
} while ($res[0] -eq $res[1])
"
Values differed after $tries tries: '$($res[0])' vs. '$($res[1])'"Expected behavior
The loop should never exit, because $env:foo and Get-Item env:foo should always report the same value.
Actual behavior
Because Get-Item env:foo intermittently reports B rather than A, the loop exits after a few tries with, e.g.:
Values differed after 3 tries: 'A' vs. 'B'
Fix
As proposed in #6305 (comment), in https://github.com/PowerShell/PowerShell/pull/6320/files#diff-1bee4b279bb56aca25fb479994641529, replace:
// 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.TryAdd((string)entry.Key, entry);(Note: the comments, taken from my original proposal, which still stands below, don't match the code - the comments explain what the code should do, but doesn't - see below.)
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() for the effective value
// and use that.
providerTable[(string)entry.Key] = new DictionaryEntry((string)entry.Key, Environment.GetEnvironmentVariable((string)entry.Key));
}Environment data
PowerShell Core post-v6.0.2 with #6320 applied.