Skip to content

Commit

Permalink
bpo-43984: Allow winreg.SetValueEx to set -1 without treating it as a…
Browse files Browse the repository at this point in the history
…n error (pythonGH-25775)

(cherry picked from commit a29a7b9)

Co-authored-by: Shreyan Avigyan <shreyan.avigyan@gmail.com>
  • Loading branch information
miss-islington and shreyanavigyan authored Dec 9, 2022
1 parent 8ef6045 commit 580165d
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 33 deletions.
18 changes: 17 additions & 1 deletion Lib/test/test_winreg.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

76 changes: 44 additions & 32 deletions PC/winreg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
{
Expand Down

0 comments on commit 580165d

Please sign in to comment.