From 580165d01c579c781c9ba0327f6ff7457201cea8 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Fri, 9 Dec 2022 05:17:25 -0800 Subject: [PATCH] bpo-43984: Allow winreg.SetValueEx to set -1 without treating it as an error (GH-25775) (cherry picked from commit a29a7b9b786d6b928c4bb4e6e683a3788e3ab1c1) Co-authored-by: Shreyan Avigyan --- Lib/test/test_winreg.py | 18 ++++- .../2021-05-02-15-29-33.bpo-43984.U92jiv.rst | 3 + PC/winreg.c | 76 +++++++++++-------- 3 files changed, 64 insertions(+), 33 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2021-05-02-15-29-33.bpo-43984.U92jiv.rst diff --git a/Lib/test/test_winreg.py b/Lib/test/test_winreg.py index 8157c2da6efaa6..769ab67b0f5611 100644 --- a/Lib/test/test_winreg.py +++ b/Lib/test/test_winreg.py @@ -113,7 +113,6 @@ def _write_test_data(self, root_key, subkeystr="sub_key", "does not close the actual key!") except OSError: pass - def _read_test_data(self, root_key, subkeystr="sub_key", OpenKey=OpenKey): # Check we can get default value for this key. val = QueryValue(root_key, test_key_name) @@ -340,6 +339,23 @@ def test_setvalueex_value_range(self): finally: DeleteKey(HKEY_CURRENT_USER, test_key_name) + def test_setvalueex_negative_one_check(self): + # Test for Issue #43984, check -1 was not set by SetValueEx. + # Py2Reg, which gets called by SetValueEx, wasn't checking return + # value by PyLong_AsUnsignedLong, thus setting -1 as value in the registry. + # The implementation now checks PyLong_AsUnsignedLong return value to assure + # the value set was not -1. + try: + with CreateKey(HKEY_CURRENT_USER, test_key_name) as ck: + with self.assertRaises(OverflowError): + SetValueEx(ck, "test_name_dword", None, REG_DWORD, -1) + SetValueEx(ck, "test_name_qword", None, REG_QWORD, -1) + self.assertRaises(FileNotFoundError, QueryValueEx, ck, "test_name_dword") + self.assertRaises(FileNotFoundError, QueryValueEx, ck, "test_name_qword") + + finally: + DeleteKey(HKEY_CURRENT_USER, test_key_name) + def test_queryvalueex_return_value(self): # Test for Issue #16759, return unsigned int from QueryValueEx. # Reg2Py, which gets called by QueryValueEx, was returning a value diff --git a/Misc/NEWS.d/next/Windows/2021-05-02-15-29-33.bpo-43984.U92jiv.rst b/Misc/NEWS.d/next/Windows/2021-05-02-15-29-33.bpo-43984.U92jiv.rst new file mode 100644 index 00000000000000..a5975b2d00c7bf --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2021-05-02-15-29-33.bpo-43984.U92jiv.rst @@ -0,0 +1,3 @@ +:meth:`winreg.SetValueEx` now leaves the target value untouched in the case of conversion errors. +Previously, ``-1`` would be written in case of such errors. + diff --git a/PC/winreg.c b/PC/winreg.c index 4fefcdcc942ff9..29b4029c320e9d 100644 --- a/PC/winreg.c +++ b/PC/winreg.c @@ -564,42 +564,54 @@ Py2Reg(PyObject *value, DWORD typ, BYTE **retDataBuf, DWORD *retDataSize) { Py_ssize_t i,j; switch (typ) { - case REG_DWORD: - if (value != Py_None && !PyLong_Check(value)) - return FALSE; - *retDataBuf = (BYTE *)PyMem_NEW(DWORD, 1); - if (*retDataBuf == NULL){ - PyErr_NoMemory(); - return FALSE; - } - *retDataSize = sizeof(DWORD); - if (value == Py_None) { - DWORD zero = 0; - memcpy(*retDataBuf, &zero, sizeof(DWORD)); - } - else { - DWORD d = PyLong_AsUnsignedLong(value); + case REG_DWORD: + { + if (value != Py_None && !PyLong_Check(value)) { + return FALSE; + } + DWORD d; + if (value == Py_None) { + d = 0; + } + else if (PyLong_Check(value)) { + d = PyLong_AsUnsignedLong(value); + if (d == (DWORD)(-1) && PyErr_Occurred()) { + return FALSE; + } + } + *retDataBuf = (BYTE *)PyMem_NEW(DWORD, 1); + if (*retDataBuf == NULL) { + PyErr_NoMemory(); + return FALSE; + } memcpy(*retDataBuf, &d, sizeof(DWORD)); + *retDataSize = sizeof(DWORD); + break; } - break; - case REG_QWORD: - if (value != Py_None && !PyLong_Check(value)) - return FALSE; - *retDataBuf = (BYTE *)PyMem_NEW(DWORD64, 1); - if (*retDataBuf == NULL){ - PyErr_NoMemory(); - return FALSE; - } - *retDataSize = sizeof(DWORD64); - if (value == Py_None) { - DWORD64 zero = 0; - memcpy(*retDataBuf, &zero, sizeof(DWORD64)); - } - else { - DWORD64 d = PyLong_AsUnsignedLongLong(value); + case REG_QWORD: + { + if (value != Py_None && !PyLong_Check(value)) { + return FALSE; + } + DWORD64 d; + if (value == Py_None) { + d = 0; + } + else if (PyLong_Check(value)) { + d = PyLong_AsUnsignedLongLong(value); + if (d == (DWORD64)(-1) && PyErr_Occurred()) { + return FALSE; + } + } + *retDataBuf = (BYTE *)PyMem_NEW(DWORD64, 1); + if (*retDataBuf == NULL) { + PyErr_NoMemory(); + return FALSE; + } memcpy(*retDataBuf, &d, sizeof(DWORD64)); + *retDataSize = sizeof(DWORD64); + break; } - break; case REG_SZ: case REG_EXPAND_SZ: {