The Wayback Machine - https://web.archive.org/web/20240109162750/https://github.com/python/cpython/issues/87868
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
eryksun opened this issue Apr 2, 2021 · 5 comments · Fixed by #102731
Closed

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

eryksun opened this issue Apr 2, 2021 · 5 comments · Fixed by #102731
Labels
3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes extension-modules C modules in the Modules dir OS-windows type-bug An unexpected behavior, bug, or error

Comments

@eryksun
Copy link
Contributor

eryksun commented Apr 2, 2021

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

@eryksun
Copy link
Contributor Author

eryksun commented Apr 2, 2021

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

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

@eryksun eryksun added 3.8 only security fixes 3.10 only security fixes 3.9 only security fixes extension-modules C modules in the Modules dir OS-windows type-bug An unexpected behavior, bug, or error labels Apr 2, 2021
@eryksun eryksun added 3.11 bug and security fixes and removed 3.8 only security fixes labels Feb 26, 2022
@aisk
Copy link
Mannequin

aisk mannequin commented Mar 19, 2022

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

@eryksun
Copy link
Contributor Author

eryksun commented Mar 19, 2022

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.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@GalaxySnail
Copy link
Contributor

GalaxySnail commented May 14, 2022

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.

@eryksun eryksun changed the title [Windows] correctly sort and remove duplicates in _winapi getenvironment() [_winapi] correctly sort and remove duplicates in getenvironment() Aug 8, 2022
@aisk
Copy link
Contributor

aisk commented Sep 7, 2023

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

zooba pushed a commit that referenced this issue Jan 9, 2024
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 9, 2024
…nGH-102731)

(cherry picked from commit c31be58)

Co-authored-by: AN Long <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 9, 2024
…nGH-102731)

(cherry picked from commit c31be58)

Co-authored-by: AN Long <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Pieter Eendebak <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes extension-modules C modules in the Modules dir OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants