zooba · Jan 9, 2024 · Mar 14, 2023 · Mar 15, 2023 · Mar 17, 2023 · Aug 20, 2023
@@ -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, stderr = 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',
@@ -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_env(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'
@@ -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;'

diff --git a/Misc/NEWS.d/next/Windows/2023-03-15-23-53-45.gh-issue-87868.4C36oQ.rst b/Misc/NEWS.d/next/Windows/2023-03-15-23-53-45.gh-issue-87868.4C36oQ.rst
@@ -0,0 +1,2 @@
+Correctly sort and remove duplicate environment variables in
+``_winapi.CreateProcess``.
diff --git a/Modules/_winapi.c b/Modules/_winapi.c
@@ -774,12 +774,159 @@ 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 PyObject *
+normalize_environment_keys(PyObject *keys)
+{
+    PyObject *keyfunc = NULL, *kwnames = NULL, *result = NULL;
+    wchar_t *prev_key_string = NULL;
+
+    // Sort the keys.
+    keyfunc = PyCFunction_New(&sortenvironmentkey_def, NULL);
+    if (keyfunc == NULL) {
+        goto error;
+    }
+    kwnames = Py_BuildValue("(s)", "key");
+    if (kwnames == NULL) {
+        goto error;
+    }
+    PyObject *args[] = { keys, keyfunc };
+    if (PyObject_VectorcallMethod(&_Py_ID(sort), args, 1, kwnames) == NULL) {
+        goto error;
+    }
+
+    // Remove duplicated keys and only keep the last inserted one.
+    result = PyList_New(0);
+    if (result == NULL) {
+        goto error;
+    }
+    for (Py_ssize_t i=PyList_GET_SIZE(keys)-1; i>=0; i--) {
+        PyObject *key = PyList_GET_ITEM(keys, i);
+        if (key == NULL) {
+            goto error;
+        }
+        if (i == PyList_GET_SIZE(keys)) {
+            prev_key_string = PyUnicode_AsWideCharString(key, NULL);
+            if (prev_key_string == NULL) {
+                goto error;
+            }
+            if (PyList_Insert(result, 0, key) < 0) {
+                goto error;
+            }
+            continue;
+        }
+        wchar_t *key_string = PyUnicode_AsWideCharString(key, NULL);
+        if (key_string == NULL) {
+            goto error;
+        }
+        if (CompareStringOrdinal(prev_key_string, -1, key_string, -1, TRUE) == CSTR_EQUAL) {
+            PyMem_Free(key_string);
+            continue;
+        }
+        if (PyList_Insert(result, 0, key) < 0) {
+            goto error;
+        }
+        PyMem_Free(prev_key_string);
+        prev_key_string = key_string;
+    }
+
+    goto cleanup;
+error:
+    Py_XDECREF(result);
+    result = NULL;
+cleanup:
+    Py_XDECREF(keyfunc);
+    Py_XDECREF(kwnames);
+    if (prev_key_string != NULL) {
+        PyMem_Free(prev_key_string);
+    }
+
+    return result;
+}
+
+static PyObject *
+normalize_environment(PyObject* environment)
+{
+    PyObject *result = NULL, *keys = NULL, *normalized_keys = NULL;
+
+    keys = PyMapping_Keys(environment);
+    if (keys == NULL) {
+        return NULL;
+    }
+
+    normalized_keys = normalize_environment_keys(keys);
+    if (normalized_keys == NULL) {
+        goto error;
+    }
+
+    result = PyDict_New();
+    if (result == NULL) {
+        goto error;
+    }
+    for (int i=0; i<PyList_GET_SIZE(normalized_keys); i++) {
+        PyObject *key = PyList_GET_ITEM(normalized_keys, i);
+        if (key == NULL) {
+            goto error;
+        }
+        PyObject* value = PyObject_GetItem(environment, key);
+        if (value == NULL) {
+            goto error;
+        }
+
+        if (! PyUnicode_Check(key) || ! PyUnicode_Check(value)) {
+            PyErr_SetString(PyExc_TypeError,
+                "environment can only contain strings");
+            goto error;
+        }
+        if (PyUnicode_FindChar(key, '\0', 0, PyUnicode_GET_LENGTH(key), 1) != -1 ||
+            PyUnicode_FindChar(value, '\0', 0, PyUnicode_GET_LENGTH(value), 1) != -1)
+        {
+            PyErr_SetString(PyExc_ValueError, "embedded null character");
+            goto error;
+        }
+        /* Search from index 1 because on Windows starting '=' is allowed for
+           defining hidden environment variables. */
+        if (PyUnicode_GET_LENGTH(key) == 0 ||
+            PyUnicode_FindChar(key, '=', 1, PyUnicode_GET_LENGTH(key), 1) != -1)
+        {
+            PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
+            goto error;
+        }
+
+        if (PyObject_SetItem(result, key, value) < 0) {
+            goto error;
+        }
+    }
+
+    goto cleanup;
+error:
+    Py_XDECREF(result);
+    result = NULL;
+cleanup:
+    Py_XDECREF(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, *keys = NULL, *values = NULL;
 
     /* convert environment dictionary to windows environment string */
     if (! PyMapping_Check(environment)) {
@@ -788,11 +935,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;
     }
@@ -821,26 +973,6 @@ getenvironment(PyObject* environment)
         PyObject* value = PyList_GET_ITEM(values, i);
         Py_ssize_t size;
 
-        if (! PyUnicode_Check(key) || ! PyUnicode_Check(value)) {
-            PyErr_SetString(PyExc_TypeError,
-                "environment can only contain strings");
-            goto error;
-        }
-        if (PyUnicode_FindChar(key, '\0', 0, PyUnicode_GET_LENGTH(key), 1) != -1 ||
-            PyUnicode_FindChar(value, '\0', 0, PyUnicode_GET_LENGTH(value), 1) != -1)
-        {
-            PyErr_SetString(PyExc_ValueError, "embedded null character");
-            goto error;
-        }
-        /* Search from index 1 because on Windows starting '=' is allowed for
-           defining hidden environment variables. */
-        if (PyUnicode_GET_LENGTH(key) == 0 ||
-            PyUnicode_FindChar(key, '=', 1, PyUnicode_GET_LENGTH(key), 1) != -1)
-        {
-            PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
-            goto error;
-        }
-
         size = PyUnicode_AsWideChar(key, NULL, 0);
         assert(size > 1);
         if (totalsize > PY_SSIZE_T_MAX - size) {
@@ -884,6 +1016,7 @@ getenvironment(PyObject* environment)
 
 cleanup:
 error:
+    Py_XDECREF(normalized_environment);
     Py_XDECREF(keys);
     Py_XDECREF(values);
     return buffer;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Correctly sort and remove duplicate environment variables in
		``_winapi.CreateProcess``.
Comment thread aisk marked this conversation as resolved. Outdated