Add a few tests for Env Vars edge cases

[adamsitnik]

Edit: #42029 turned out to be not reproducible using the supported versions of .NET Core (2.1, 3.1 and 5.0) Please see #42029 (comment) for full details.

[ghost]

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This is a proposal of a fix for #42029

Windows treats env-vars in a case insensitive way.

Kernel32.SetEnvironmentVariable("A", "b");
Console.WriteLine(GetEnvironmentVariableCore("A")); // prints "b"
Kernel32.SetEnvironmentVariable("a", "c");
Console.WriteLine(GetEnvironmentVariableCore("A")); // prints "c"

internal static class Kernel32
{
    [DllImport("kernel32.dll", EntryPoint = "SetEnvironmentVariableW", SetLastError = true, CharSet = CharSet.Unicode, BestFitMapping = false, ExactSpelling = true)]
    internal static extern bool SetEnvironmentVariable(string lpName, string? lpValue);

    [DllImport("kernel32.dll", EntryPoint = "GetEnvironmentVariableW", SetLastError = true, CharSet = CharSet.Unicode, ExactSpelling = true)]
    internal static extern unsafe uint GetEnvironmentVariable(string lpName, ref char lpBuffer, uint nSize);
}

private static void SetEnvironmentVariableCore(string variable, string? value)
{
    if (!Kernel32.SetEnvironmentVariable(variable, value))
    {
        int errorCode = Marshal.GetLastWin32Error();

        throw new Exception($"Error code: {errorCode}");
    }
}

private unsafe static string? GetEnvironmentVariableCore(string variable)
{
    char[] buffer = new char[1000];

    fixed (char* pinned = buffer)
    {
        uint size = Kernel32.GetEnvironmentVariable(variable, ref buffer[0], (uint)buffer.Length);

        return new string(pinned, 0, (int)size);
    }
}

The problem is, that by using CreateProcess API it's possible to create a process with duplicated env vars.

For such apps (example #42029), Environment.GetEnvironmentVariables returns data as-is (with duplicates), while ProcessStartInfo.Environment throws an exception:

Item has already been added. Key in dictionary: 'NPM_CONFIG_CACHE'  Key
being added: 'npm_config_cache' 
   at System.Collections.Hashtable.Insert(Object key, Object nvalue, Boolean add)
   at System.Diagnostics.ProcessStartInfo.get_EnvironmentVariables()
   at Microsoft.Build.Utilities.ToolTask.GetProcessStartInfo(String pathToTool,

We can't change the StringComparison used for building the dictionary in ProcessStartInfo:

runtime/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs

Line 87 in 995bbae

CaseSensitiveEnvironmentVariables ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase));

Because this would introduce a breaking change in scenarios, where users were so far overwriting the env var:

var startInfo1 = new ProcessStartInfo();
startInfo1.Environment["A"] = "b";
startInfo1.Environment["a"] = "c"; // was always resulting in the env var set to "c"

var startInfo2 = new ProcessStartInfo();
startInfo2.Environment["a"] = "c";
startInfo2.Environment["A"] = "b"; // was always resulting in the env var set to "b"

// with `StringComparison` changed it would be dependent on the order of items in the dictionary

I think that the best we can do is to not throw for cases like #42029 where there are simply two env var keys with the same value:

"NPM_CONFIG_CACHE=^"
"npm_config_cache=^"

And ignore such "duplicates" but throw (as we used to do so far) where the values are different.

Author:	adamsitnik
Assignees:	-
Labels:	area-System.Diagnostics.Process
Milestone:	6.0.0

[eiriktsarpalis]

LGTM