Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

#6320 (fix for #6305) is flawed, because it may expose incorrect environment-variable values via Get-Item #6460

Copy link
Copy link
@mklement0

Description

@mklement0
Issue body actions

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:foo may 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.

Reactions are currently unavailable

Metadata

Metadata

Assignees

Labels

Issue-BugIssue has been identified as a bug in the productIssue has been identified as a bug in the productResolution-FixedThe issue is fixed.The issue is fixed.

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

    Morty Proxy This is a proxified and sanitized view of the page, visit original site.