8000 gh-87868: Sort and remove duplicates in getenvironment() by aisk · Pull Request #102731 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-87868: Sort and remove duplicates in getenvironment() #102731

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

Merged
merged 63 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
aed39b7
gh-87868: correctly sort and remove duplicates in getenvironment()
aisk Mar 14, 2023
4de761b
Merge branch 'main' into getenvironment
AlexWaygood Mar 15, 2023
04f7d59
check null in normalize_environment
aisk Mar 17, 2023
c43abb2
Update Modules/_winapi.c
aisk Aug 20, 2023
3f6319a
Update Modules/_winapi.c
aisk Aug 20, 2023
8ef3b9c
Update Lib/test/test_subprocess.py
aisk Aug 20, 2023
46491c5
Merge remote-tracking branch 'upstream/main' into getenvironment
aisk Aug 20, 2023
ca13205
Adapt latest changes from main branch
aisk Aug 20, 2023
f727004
Fix test case
aisk Aug 20, 2023
6b9e873
update for review comments
aisk Sep 3, 2023
dd53528
add win32 related env validation tests
aisk Sep 3, 2023
37fa2f2
fix memory leak
aisk Sep 4, 2023
fcd8b88
Merge branch 'main' into getenvironment
aisk Sep 6, 2023
96754a1
Merge branch 'main' into getenvironment
aisk Sep 8, 2023
47155b5
follow PEP7
aisk Sep 8, 2023
a204740
add error check
aisk Sep 8, 2023
8ca7b8e
fix test
aisk Sep 11, 2023
b620bdb
better cleanup in error path
aisk Sep 11, 2023
65c8483
fix a mem leak
aisk Sep 13, 2023
2d2fd4d
refactor error handling
aisk Sep 13, 2023
2096209
fix one off iteration and add test
aisk Sep 19, 2023
da55b3b
fix one env test on non windows platforms
aisk Sep 19, 2023
5b17740
Merge branch 'main' into getenvironment
aisk Sep 24, 2023
bf5a555
using vector call
aisk Sep 24, 2023
66fddbe
fix test on none windows platforms
aisk Sep 24, 2023
2b82368
fix typo in test
aisk Sep 24, 2023
5c581ea
fix test on linux
aisk Sep 25, 2023
6b5d1da
revert line end modify and align codes with pep7
aisk Oct 4, 2023
bcd8910
Update Modules/_winapi.c
aisk Jan 6, 2024
3423e13
Update Modules/_winapi.c
aisk Jan 6, 2024
03a4f7e
Update Modules/_winapi.c
aisk Jan 6, 2024
186d718
Update Modules/_winapi.c
aisk Jan 6, 2024
8b074df
Update Modules/_winapi.c
aisk Jan 6, 2024
b675260
Update Modules/_winapi.c
aisk Jan 6, 2024
fb93eb1
Update Modules/_winapi.c
aisk Jan 6, 2024
f9d97eb
Update Modules/_winapi.c
aisk Jan 6, 2024
6f3b7ce
Update Modules/_winapi.c
aisk Jan 6, 2024
b9b8d6e
Update Modules/_winapi.c
aisk Jan 6, 2024
5fd7ebf
Update Modules/_winapi.c
aisk Jan 6, 2024
f87970a
Update Modules/_winapi.c
aisk Jan 6, 2024
cbdfa84
Update Lib/test/test_subprocess.py
aisk Jan 6, 2024
8748103
only keep the last duplicated environment key
aisk Jan 6, 2024
4aff3b4
split the normalize_environment function into two
aisk Jan 6, 2024
67b8731
Update Lib/test/test_subprocess.py
aisk Jan 7, 2024
d02319e
remove the error check for PyList_GET_ITEM
aisk Jan 7, 2024
d1c794b
decref the vectorcall result
aisk Jan 7, 2024
1763421
split normalze_keys into sort_keys and dedup_keys
aisk Jan 7, 2024
dbbaf1b
Update Modules/_winapi.c
aisk Jan 7, 2024
176e90d
refactor sort_environment_keys to remove the goto error stuff
aisk Jan 8, 2024
d088083
refactor the dedup_environment_keys to get rid of the goto error stuff
aisk Jan 8, 2024
471c8b8
fix a memory leak in `normalize_environment`
aisk Jan 8, 2024
a2dab93
revert the commit that moves key validation codes to the original place
aisk Jan 8, 2024
6a2bbee
fixed a memory leak
aisk Jan 8, 2024
22548cc
Update Modules/_winapi.c
aisk Jan 9, 2024
63cf753
Update Modules/_winapi.c
aisk Jan 9, 2024
91d091d
using int as return value rather than bool
aisk Jan 9, 2024
b053e2a
get rid of the goto error in normalize_environment
aisk Jan 9, 2024
5dd6215
Update Misc/NEWS.d/next/Windows/2023-03-15-23-53-45.gh-issue-87868.4C…
aisk Jan 9, 2024
be49dae
Update Modules/_winapi.c
aisk Jan 9, 2024
0aff1e9
Update Modules/_winapi.c
aisk Jan 9, 2024
72fc36d
Update Modules/_winapi.c
aisk Jan 9, 2024
725baec
Update Modules/_winapi.c
aisk Jan 9, 2024
0a308be
Update Modules/_winapi.c
aisk Jan 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,19 @@ def test_env(self):
stdout, stderr = p.communicate()
self.assertEqual(stdout, b"orange")

@unittest.skipUnless(sys.platform == "win32", "Windows only issue")
def test_win32_duplicate_envs(self):
newenv = os.environ.copy()
newenv["fRUit"] = "cherry"
newenv["fruit"] = "lemon"
newenv["FRUIT"] = "orange"
newenv["frUit"] = "banana"
with subprocess.Popen(["CMD", "/c", "SET", "fruit"],
stdout=subprocess.PIPE,
env=newenv) as p:
stdout, _ = p.communicate()
self.assertEqual(stdout.strip(), b"frUit=banana")

# Windows requires at least the SYSTEMROOT environment variable to start
# Python
@unittest.skipIf(sys.platform == 'win32',
Expand Down Expand Up @@ -817,6 +830,17 @@ def is_env_var_to_ignore(n):
if not is_env_var_to_ignore(k)]
self.assertEqual(child_env_names, [])

def test_one_environment_variable(self):
newenv = {'fruit': 'orange'}
cmd = [sys.executable, '-c',
'import sys,os;'
'sys.stdout.write("fruit="+os.getenv("fruit"))']
if sys.platform == "win32":
cmd = ["CMD", "/c", "SET", "fruit"]
with subprocess.Popen(cmd, stdout=subprocess.PIPE, env=newenv) as p:
stdout, _ = p.communicate()
self.assertTrue(stdout.startswith(b"fruit=orange"))

def test_invalid_cmd(self):
# null character in the command name
cmd = sys.executable + '\0'
Expand Down Expand Up @@ -857,6 +881,19 @@ def test_invalid_env(self):
stdout, stderr = p.communicate()
self.assertEqual(stdout, b"orange=lemon")

@unittest.skipUnless(sys.platform == "win32", "Windows only issue")
def test_win32_invalid_env(self):
# '=' in the environment variable name
newenv = os.environ.copy()
newenv["FRUIT=VEGETABLE"] = "cabbage"
with self.assertRaises(ValueError):
subprocess.Popen(ZERO_RETURN_CMD, env=newenv)

newenv = os.environ.copy()
newenv["==FRUIT"] = "cabbage"
with self.assertRaises(ValueError):
subprocess.Popen(ZERO_RETURN_CMD, env=newenv)

def test_communicate_stdin(self):
p = subprocess.Popen([sys.executable, "-c",
'import sys;'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Correctly sort and remove duplicate environment variables in
:py:func:`!_winapi.CreateProcess`.
159 changes: 155 additions & 4 deletions Modules/_winapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -774,12 +774,157 @@ gethandle(PyObject* obj, const char* name)
return ret;
}

static PyObject *
sortenvironmentkey(PyObject *module, PyObject *item)
{
return _winapi_LCMapStringEx_impl(NULL, LOCALE_NAME_INVARIANT,
LCMAP_UPPERCASE, item);
}

static PyMethodDef sortenvironmentkey_def = {
"sortenvironmentkey", _PyCFunction_CAST(sortenvironmentkey), METH_O, "",
};

static int
sort_environment_keys(PyObject *keys)
{
PyObject *keyfunc = PyCFunction_New(&sortenvironmentkey_def, NULL);
if (keyfunc == NULL) {
return -1;
}
PyObject *kwnames = Py_BuildValue("(s)", "key");
if (kwnames == NULL) {
Py_DECREF(keyfunc);
return -1;
}
PyObject *args[] = { keys, keyfunc };
PyObject *ret = PyObject_VectorcallMethod(&_Py_ID(sort), args, 1, kwnames);
Py_DECREF(keyfunc);
Py_DECREF(kwnames);
if (ret == NULL) {
return -1;
}
Py_DECREF(ret);

return 0;
}

static int
compare_string_ordinal(PyObject *str1, PyObject *str2, int *result)
{
wchar_t *s1 = PyUnicode_AsWideCharString(str1, NULL);
if (s1 == NULL) {
return -1;
}
wchar_t *s2 = PyUnicode_AsWideCharString(str2, NULL);
if (s2 == NULL) {
PyMem_Free(s1);
return -1;
}
*result = CompareStringOrdinal(s1, -1, s2, -1, TRUE);
PyMem_Free(s1);
PyMem_Free(s2);
return 0;
}

static PyObject *
dedup_environment_keys(PyObject *keys)
{
PyObject *result = PyList_New(0);
if (result == NULL) {
return NULL;
}

// Iterate over the pre-ordered keys, check whether the current key is equal
// to the next key (ignoring case), if different, insert the current value
// into the result list. If they are equal, do nothing because we always
// want to keep the last inserted one.
for (Py_ssize_t i = 0; i < PyList_GET_SIZE(keys); i++) {
PyObject *key = PyList_GET_ITEM(keys, i);

// The last key will always be kept.
if (i + 1 == PyList_GET_SIZE(keys)) {
if (PyList_Append(result, key) < 0) {
Py_DECREF(result);
return NULL;
}
continue;
}

PyObject *next_key = PyList_GET_ITEM(keys, i + 1);
int compare_result;
if (compare_string_ordinal(key, next_key, &compare_result) < 0) {
Py_DECREF(result);
return NULL;
}
if (compare_result == CSTR_EQUAL) {
continue;
}
if (PyList_Append(result, key) < 0) {
Py_DECREF(result);
return NULL;
}
}

return result;
}

static PyObject *
normalize_environment(PyObject *environment)
{
PyObject *keys = PyMapping_Keys(environment);
if (keys == NULL) {
return NULL;
}

if (sort_environment_keys(keys) < 0) {
Py_DECREF(keys);
return NULL;
}

PyObject *normalized_keys = dedup_environment_keys(keys);
Py_DECREF(keys);
if (normalized_keys == NULL) {
return NULL;
}

PyObject *result = PyDict_New();
if (result == NULL) {
Py_DECREF(normalized_keys);
return NULL;
}

for (int i = 0; i < PyList_GET_SIZE(normalized_keys); i++) {
PyObject *key = PyList_GET_ITEM(normalized_keys, i);
PyObject *value = PyObject_GetItem(environment, key);
if (value == NULL) {
Py_DECREF(normalized_keys);
Py_DECREF(result);
return NULL;
}

int ret = PyObject_SetItem(result, key, value);
Py_DECREF(value);
if (ret < 0) {
Py_DECREF(normalized_keys);
Py_DECREF(result);
return NULL;
}
}

Py_DECREF(normalized_keys);

return result;
}

static wchar_t *
getenvironment(PyObject* environment)
{
Py_ssize_t i, envsize, totalsize;
wchar_t *buffer = NULL, *p, *end;
PyObject *keys, *values;
PyObject *normalized_environment = NULL;
PyObject *keys = NULL;
PyObject *values = NULL;

/* convert environment dictionary to windows environment string */
if (! PyMapping_Check(environment)) {
Expand All @@ -788,11 +933,16 @@ getenvironment(PyObject* environment)
return NULL;
}

keys = PyMapping_Keys(environment);
if (!keys) {
normalized_environment = normalize_environment(environment);
if (normalize_environment == NULL) {
return NULL;
}
values = PyMapping_Values(environment);

keys = PyMapping_Keys(normalized_environment);
if (!keys) {
goto error;
}
values = PyMapping_Values(normalized_environment);
if (!values) {
goto error;
}
Expand Down Expand Up @@ -884,6 +1034,7 @@ getenvironment(PyObject* environment)

cleanup:
error:
Py_XDECREF(normalized_environment);
Py_XDECREF(keys);
Py_XDECREF(values);
return buffer;
Expand Down
0