From b109e8ce4e228aab614300d160cb0e4be8978c48 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 22 Jan 2020 14:01:56 +0100 Subject: [PATCH] bpo-39413: os.unsetenv() uses _wputenv() on Windows os.unsetenv() uses _wputenv() rather than SetEnvironmentVariableW(): _wputenv() updates the CRT, whereas SetEnvironmentVariableW() does not. Replace also lambda functions with regular functions to get named functions, to ease debug. --- Lib/os.py | 8 ++- Lib/test/test_os.py | 10 +-- Modules/clinic/posixmodule.c.h | 10 +-- Modules/posixmodule.c | 108 +++++++++++++++------------------ 4 files changed, 65 insertions(+), 71 deletions(-) diff --git a/Lib/os.py b/Lib/os.py index ca418edbc57366..7e83dc26229f84 100644 --- a/Lib/os.py +++ b/Lib/os.py @@ -715,7 +715,9 @@ def setdefault(self, key, value): try: _putenv = putenv except NameError: - _putenv = lambda key, value: None + def _putenv(key, value): + # do nothing + return else: if "putenv" not in __all__: __all__.append("putenv") @@ -723,7 +725,9 @@ def setdefault(self, key, value): try: _unsetenv = unsetenv except NameError: - _unsetenv = lambda key: _putenv(key, "") + def _unsetenv(key): + # set the environment variable to an empty string + _putenv(key, "") else: if "unsetenv" not in __all__: __all__.append("unsetenv") diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 50535da0a2bfc1..5acacc5da0ea15 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -955,10 +955,12 @@ def test_environb(self): # On OS X < 10.6, unsetenv() doesn't return a value (bpo-13415). @support.requires_mac_ver(10, 6) - def test_unset_error(self): - # "=" is not allowed in a variable name - key = 'key=' - self.assertRaises(OSError, os.environ.__delitem__, key) + def test_unsetenv_error(self): + self.assertRaises(OSError, os.unsetenv, '=key') + # On Windows, only names starting with "=" are invalid + if sys.platform != "win32": + self.assertRaises(OSError, os.unsetenv, 'ke=y') + self.assertRaises(OSError, os.unsetenv, 'key=') def test_key_type(self): missing = 'missingkey' diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index 661d91afb7eb6f..acba241bcf6db1 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -6034,7 +6034,7 @@ os_posix_fadvise(PyObject *module, PyObject *const *args, Py_ssize_t nargs) #endif /* (defined(HAVE_POSIX_FADVISE) && !defined(POSIX_FADVISE_AIX_BUG)) */ -#if defined(HAVE_PUTENV) && defined(MS_WINDOWS) +#if defined(MS_WINDOWS) PyDoc_STRVAR(os_putenv__doc__, "putenv($module, name, value, /)\n" @@ -6080,9 +6080,9 @@ os_putenv(PyObject *module, PyObject *const *args, Py_ssize_t nargs) return return_value; } -#endif /* defined(HAVE_PUTENV) && defined(MS_WINDOWS) */ +#endif /* defined(MS_WINDOWS) */ -#if defined(HAVE_PUTENV) && !defined(MS_WINDOWS) +#if (defined(HAVE_PUTENV) && !defined(MS_WINDOWS)) PyDoc_STRVAR(os_putenv__doc__, "putenv($module, name, value, /)\n" @@ -6123,7 +6123,7 @@ os_putenv(PyObject *module, PyObject *const *args, Py_ssize_t nargs) return return_value; } -#endif /* defined(HAVE_PUTENV) && !defined(MS_WINDOWS) */ +#endif /* (defined(HAVE_PUTENV) && !defined(MS_WINDOWS)) */ #if defined(MS_WINDOWS) @@ -8809,4 +8809,4 @@ os__remove_dll_directory(PyObject *module, PyObject *const *args, Py_ssize_t nar #ifndef OS__REMOVE_DLL_DIRECTORY_METHODDEF #define OS__REMOVE_DLL_DIRECTORY_METHODDEF #endif /* !defined(OS__REMOVE_DLL_DIRECTORY_METHODDEF) */ -/*[clinic end generated code: output=6e739a2715712e88 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=773b0e49560f08dc input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 5a0c8a311a790e..13d1d95c3fd316 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -10079,53 +10079,31 @@ posix_putenv_dict_setitem(PyObject *name, PyObject *value) #endif /* PY_PUTENV_DICT */ -#ifdef HAVE_PUTENV - #ifdef MS_WINDOWS -/*[clinic input] -os.putenv - - name: unicode - value: unicode - / - -Change or add an environment variable. -[clinic start generated code]*/ - -static PyObject * -os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) -/*[clinic end generated code: output=d29a567d6b2327d2 input=ba586581c2e6105f]*/ +static PyObject* +py_win_putenv(PyObject *name, PyObject *value) { - const wchar_t *env; - Py_ssize_t size; - - /* Search from index 1 because on Windows starting '=' is allowed for - defining hidden environment variables. */ - if (PyUnicode_GET_LENGTH(name) == 0 || - PyUnicode_FindChar(name, '=', 1, PyUnicode_GET_LENGTH(name), 1) != -1) - { - PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); - return NULL; - } PyObject *unicode = PyUnicode_FromFormat("%U=%U", name, value); if (unicode == NULL) { return NULL; } - env = PyUnicode_AsUnicodeAndSize(unicode, &size); - if (env == NULL) + const wchar_t *env; + Py_ssize_t size; + /* PyUnicode_AsWideCharString() rejects embedded null characters */ + env = PyUnicode_AsWideCharString(unicode, &size); + if (env == NULL) { goto error; + } if (size > _MAX_ENV) { PyErr_Format(PyExc_ValueError, "the environment variable is longer than %u characters", _MAX_ENV); goto error; } - if (wcslen(env) != (size_t)size) { - PyErr_SetString(PyExc_ValueError, "embedded null character"); - goto error; - } + /* Use _wputenv() rather than SetEnvironmentVariableW(): _wputenv() + updates the CRT, whereas SetEnvironmentVariableW() does not. */ if (_wputenv(env)) { posix_error(); goto error; @@ -10138,7 +10116,37 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) Py_DECREF(unicode); return NULL; } -#else /* MS_WINDOWS */ +#endif + + +#ifdef MS_WINDOWS +/*[clinic input] +os.putenv + + name: unicode + value: unicode + / + +Change or add an environment variable. +[clinic start generated code]*/ + +static PyObject * +os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) +/*[clinic end generated code: output=d29a567d6b2327d2 input=ba586581c2e6105f]*/ +{ + /* Search from index 1 because on Windows starting '=' is allowed for + defining hidden environment variables. */ + if (PyUnicode_GET_LENGTH(name) == 0 || + PyUnicode_FindChar(name, '=', 1, PyUnicode_GET_LENGTH(name), 1) != -1) + { + PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); + return NULL; + } + + return py_win_putenv(name, value); +} +/* repeat !defined(MS_WINDOWS) to workaround an Argument Clinic issue */ +#elif defined(HAVE_PUTENV) && !defined(MS_WINDOWS) /*[clinic input] os.putenv @@ -10176,8 +10184,7 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) posix_putenv_dict_setitem(name, bytes); Py_RETURN_NONE; } -#endif /* MS_WINDOWS */ -#endif /* HAVE_PUTENV */ +#endif /* defined(HAVE_PUTENV) && !defined(MS_WINDOWS) */ #ifdef MS_WINDOWS @@ -10193,35 +10200,16 @@ static PyObject * os_unsetenv_impl(PyObject *module, PyObject *name) /*[clinic end generated code: output=54c4137ab1834f02 input=4d6a1747cc526d2f]*/ { - /* PyUnicode_AsWideCharString() rejects embedded null characters */ - wchar_t *name_str = PyUnicode_AsWideCharString(name, NULL); - if (name_str == NULL) { + PyObject *empty = PyUnicode_New(0, 0); + if (empty == NULL) { return NULL; } - BOOL ok = SetEnvironmentVariableW(name_str, NULL); - PyMem_Free(name_str); - - if (!ok) { - return PyErr_SetFromWindowsErr(0); - } + /* _wputenv("name=") removes the environment variable "name" */ + PyObject *res = py_win_putenv(name, empty); + Py_DECREF(empty); -#ifdef PY_PUTENV_DICT - /* Remove the key from putenv_dict; - * this will cause it to be collected. This has to - * happen after the real unsetenv() call because the - * old value was still accessible until then. - */ - if (PyDict_DelItem(_posixstate(module)->putenv_dict, name)) { - /* really not much we can do; just leak */ - if (!PyErr_ExceptionMatches(PyExc_KeyError)) { - return NULL; - } - PyErr_Clear(); - } -#endif - - Py_RETURN_NONE; + return res; } /* repeat !defined(MS_WINDOWS) to workaround an Argument Clinic issue */ #elif defined(HAVE_UNSETENV) && !defined(MS_WINDOWS)