[_winapi] correctly sort and remove duplicates in getenvironment()

[eryksun]

BPO	43702
Nosy	@pfmoore, @tjguk, @zware, @eryksun, @zooba, @aisk

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-04-02.04:16:14.463>
labels = ['type-bug', '3.9', '3.10', '3.11', 'extension-modules', 'OS-windows']
title = '[Windows] correctly sort and remove duplicates in _winapi getenvironment()'
updated_at = <Date 2022-03-19.18:03:11.308>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2022-03-19.18:03:11.308>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'Windows']
creation = <Date 2021-04-02.04:16:14.463>
creator = 'eryksun'
dependencies = []
files = []
hgrepos = []
issue_num = 43702
keywords = []
message_count = 3.0
messages = ['390038', '415560', '415563']
nosy_count = 6.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'asaka']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'needs patch'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue43702'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

Linked PRs

• gh-87868: Sort and remove duplicates in getenvironment() #102731
• [3.12] gh-87868: Sort and remove duplicates in getenvironment() (GH-102731) #113865
• [3.11] gh-87868: Sort and remove duplicates in getenvironment() (GH-102731) #113866
• gh-87868: Skip some test in test_subprocess if python is configured with --enable-shared #113867

[eryksun]

The getenvironment() function in "Modules/_winapi.c" needs to sort variables in the environment block and remove duplicates case insensitively¹.

The sort order used to matter with SetEnvironmentVairableW(). As soon as it reached a name in the environment block that compared greater than the target name, it would insert a new variable. Nowadays, SetEnvironmentVairableW() searches the entire environment block before inserting a new value. Regardless, at the very least, getenvironment() is not well-behaved and is not setting the environment in the documented sort order that users, and possibly other programs, expect.

Case-insensitive sorting in Windows uses upper case. The variable names in the mapping can be added to a list and sorted with a key function that's based on LCMapStringEx(), with the flag LCMAP_UPPERCASE. Loop over the sorted list to create the environment block. Remove duplicates by skipping a name that compares equal to the previously stored name according to CompareStringOrdinal().

Footnotes

1. Changing Environment Variables ↩

[aisk]

I have a question, how to determine which name should be stored if they are duplicated with case insensitive?

[eryksun]

which name should be stored if they are duplicated with case insensitive?

Ideally os.environ would preserve the original case of the process environment, and os.environ.copy() would return a copy that's also case insensitive. That would prevent most problems with duplicates keys. See msg387676 in bpo-28824, and msg414319 in bpo-15373.

In msg390038 I suggested keeping the first key that's encountered. However, dicts preserve insertion order nowadays, so one could assume that the last one is the one that the caller wants to keep.

[GalaxySnail]

This issue can be reproduced on msys2 (also cygwin, I think) easily:

msys2 shell $ ABC=1 abc=2 python
Python 3.11.0b1 (main, May  7 2022, 22:58:47) [MSC v.1931 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> print(os.environ["ABC"], os.environ["abc"])
2 2
>>> import subprocess
>>> subprocess.run(["bash", "-c", "echo $ABC $abc"])
1 2
CompletedProcess(args=['bash', '-c', 'echo $ABC $abc'], returncode=0)
>>> 
>>> os.environ["abc"] = "3"
>>> print(os.environ["ABC"], os.environ["abc"])
3 3
>>> subprocess.run(["bash", "-c", "echo $ABC $abc"])
3 2
CompletedProcess(args=['bash', '-c', 'echo $ABC $abc'], returncode=0)

msys2 shell $ ABC=1 abc=2 cmd
cmd > echo %ABC% %abc%
1 1
cmd > bash -c "echo $ABC $abc"
1 2
cmd > set abc=3
cmd > echo %ABC% %abc%
3 3
cmd > bash -c "echo $ABC $abc"
3 2

msys2 shell $ ABC=1 abc=2 powershell
PS > echo $Env:ABC $Env:abc
1
1
PS > bash -c "echo $ABC $abc"
1 2
PS > $Env:abc = 3
PS > echo $Env:ABC $Env:abc
3
3
PS > bash -c 'echo $ABC $abc'
3 2

Python behaves differently from both cmd.exe and powershell.

Case-sensitive environment variables are in nt.environ, but they don't affect the real environment:

msys2 shell $ ABC=1 abc=2 python
Python 3.11.0b1 (main, May  7 2022, 22:58:47) [MSC v.1931 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import nt, os
>>> print(nt.environ["ABC"], nt.environ["abc"])
1 2
>>> print(os.environ["ABC"], os.environ["abc"])
2 2

This is because environment vairiables are not sorted correctly in os._convertenviron:

https://github.com/python/cpython/blob/v3.11.0b1/Lib/os.py#L750

Related to GH-91018, GH-73010.

[aisk]

This PR is opened for a while, could you please help me review it by any chance? @eryksun @zooba

[gpshead]

i'll apply the test skip PR into the existing 3.12 and 3.11 backport branches.