Skip to content

subprocess makes environment blocks with duplicate keys on Windows #91018

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

Closed
benrg mannequin opened this issue Feb 26, 2022 · 4 comments
Closed

subprocess makes environment blocks with duplicate keys on Windows #91018

benrg mannequin opened this issue Feb 26, 2022 · 4 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@benrg
Copy link
Mannequin

benrg mannequin commented Feb 26, 2022

BPO 46862
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba

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 2022-02-26.04:19:39.329>
labels = ['type-bug', '3.9', '3.10', '3.11', 'library', 'OS-windows']
title = 'subprocess makes environment blocks with duplicate keys on Windows'
updated_at = <Date 2022-02-26.06:07:22.207>
user = 'https://bugs.python.org/benrg'

bugs.python.org fields:

activity = <Date 2022-02-26.06:07:22.207>
actor = 'eryksun'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2022-02-26.04:19:39.329>
creator = 'benrg'
dependencies = []
files = []
hgrepos = []
issue_num = 46862
keywords = []
message_count = 3.0
messages = ['414068', '414074', '414075']
nosy_count = 6.0
nosy_names = ['paul.moore', 'tim.golden', 'benrg', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue46862'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

@benrg
Copy link
Mannequin Author

benrg mannequin commented Feb 26, 2022

On Windows, if one writes

    env = os.environ.copy()
    env['http_proxy'] = 'whatever'

or either of the documented equivalents ({**os.environ, ...} or (os.environ | {...})), and passes the resulting environment to subprocess.run or subprocess.Popen, the spawned process may get an environment containing both HTTP_PROXY and http_proxy. Most Win32 software will see only the first one, which contains the unmodified value from os.environ.

Because os.environ forces all keys to upper case, it's possible to work around this by using only upper case keys in the update, but that behavior of os.environ is nonstandard (bpo-46861), and subprocess shouldn't depend on it always being true, nor should end users have to.

Since dicts preserve order, the user's (presumable) intent is preserved in the env argument. I think subprocess should do something like

    env = {k.upper(): (k, v) for k, v in env.items()}
    env = dict(env.values())

to discard duplicate keys, keeping only the rightmost one.

@benrg benrg mannequin added 3.10 only security fixes stdlib Python modules in the Lib dir OS-windows type-bug An unexpected behavior, bug, or error labels Feb 26, 2022
@eryksun
Copy link
Contributor

eryksun commented Feb 26, 2022

This should be handled in _winapi.CreateProcess(). An environment block is technically required to be sorted. (Ages ago this was a MUST requirement for getting and setting variables to work correctly, since the implementation depended on the sort order, but I think nowadays it's a SHOULD requirement.) For example, see the documentation of CreateProcessW() [1]:

If an application provides an environment block, ... explicitly create
these environment variable strings, sort them alphabetically (because
the system uses a sorted environment)

"Changing Environment Variables" is more specific [2]:

All strings in the environment block must be sorted alphabetically by name.
The sort is case-insensitive, Unicode order, without regard to locale.

CompareStringOrdinal() [3] implements a case-insensitive ordinal comparison. When a key compares as equal, either keep the current one in the sorted list, or replace it with the new key.

---

[1] https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw
[2] https://docs.microsoft.com/en-us/windows/win32/procthread/changing-environment-variables
[3] https://docs.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-comparestringordinal

@eryksun eryksun added 3.9 only security fixes 3.11 only security fixes labels Feb 26, 2022
@eryksun
Copy link
Contributor

eryksun commented Feb 26, 2022

I suggest closing this issue as a duplicate of bpo-43702.

@zooba
Copy link
Member

zooba commented Aug 25, 2022

Duplicate of #87868

@zooba zooba marked this as a duplicate of #87868 Aug 25, 2022
@zooba zooba closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2022
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 only security fixes OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants