Skip to content

Commit a29a7b9

Browse files
bpo-43984: Allow winreg.SetValueEx to set -1 without treating it as an error (GH-25775)
1 parent fb713b2 commit a29a7b9

File tree

3 files changed

+64
-33
lines changed

3 files changed

+64
-33
lines changed

Lib/test/test_winreg.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ def _write_test_data(self, root_key, subkeystr="sub_key",
113113
"does not close the actual key!")
114114
except OSError:
115115
pass
116-
117116
def _read_test_data(self, root_key, subkeystr="sub_key", OpenKey=OpenKey):
118117
# Check we can get default value for this key.
119118
val = QueryValue(root_key, test_key_name)
@@ -340,6 +339,23 @@ def test_setvalueex_value_range(self):
340339
finally:
341340
DeleteKey(HKEY_CURRENT_USER, test_key_name)
342341

342+
def test_setvalueex_negative_one_check(self):
343+
# Test for Issue #43984, check -1 was not set by SetValueEx.
344+
# Py2Reg, which gets called by SetValueEx, wasn't checking return
345+
# value by PyLong_AsUnsignedLong, thus setting -1 as value in the registry.
346+
# The implementation now checks PyLong_AsUnsignedLong return value to assure
347+
# the value set was not -1.
348+
try:
349+
with CreateKey(HKEY_CURRENT_USER, test_key_name) as ck:
350+
with self.assertRaises(OverflowError):
351+
SetValueEx(ck, "test_name_dword", None, REG_DWORD, -1)
352+
SetValueEx(ck, "test_name_qword", None, REG_QWORD, -1)
353+
self.assertRaises(FileNotFoundError, QueryValueEx, ck, "test_name_dword")
354+
self.assertRaises(FileNotFoundError, QueryValueEx, ck, "test_name_qword")
355+
356+
finally:
357+
DeleteKey(HKEY_CURRENT_USER, test_key_name)
358+
343359
def test_queryvalueex_return_value(self):
344360
# Test for Issue #16759, return unsigned int from QueryValueEx.
345361
# Reg2Py, which gets called by QueryValueEx, was returning a value
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:meth:`winreg.SetValueEx` now leaves the target value untouched in the case of conversion errors.
2+
Previously, ``-1`` would be written in case of such errors.
3+

PC/winreg.c

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -561,42 +561,54 @@ Py2Reg(PyObject *value, DWORD typ, BYTE **retDataBuf, DWORD *retDataSize)
561561
{
562562
Py_ssize_t i,j;
563563
switch (typ) {
564-
case REG_DWORD:
565-
if (value != Py_None && !PyLong_Check(value))
566-
return FALSE;
567-
*retDataBuf = (BYTE *)PyMem_NEW(DWORD, 1);
568-
if (*retDataBuf == NULL){
569-
PyErr_NoMemory();
570-
return FALSE;
571-
}
572-
*retDataSize = sizeof(DWORD);
573-
if (value == Py_None) {
574-
DWORD zero = 0;
575-
memcpy(*retDataBuf, &zero, sizeof(DWORD));
576-
}
577-
else {
578-
DWORD d = PyLong_AsUnsignedLong(value);
564+
case REG_DWORD:
565+
{
566+
if (value != Py_None && !PyLong_Check(value)) {
567+
return FALSE;
568+
}
569+
DWORD d;
570+
if (value == Py_None) {
571+
d = 0;
572+
}
573+
else if (PyLong_Check(value)) {
574+
d = PyLong_AsUnsignedLong(value);
575+
if (d == (DWORD)(-1) && PyErr_Occurred()) {
576+
return FALSE;
577+
}
578+
}
579+
*retDataBuf = (BYTE *)PyMem_NEW(DWORD, 1);
580+
if (*retDataBuf == NULL) {
581+
PyErr_NoMemory();
582+
return FALSE;
583+
}
579584
memcpy(*retDataBuf, &d, sizeof(DWORD));
585+
*retDataSize = sizeof(DWORD);
586+
break;
580587
}
581-
break;
582-
case REG_QWORD:
583-
if (value != Py_None && !PyLong_Check(value))
584-
return FALSE;
585-
*retDataBuf = (BYTE *)PyMem_NEW(DWORD64, 1);
586-
if (*retDataBuf == NULL){
587-
PyErr_NoMemory();
588-
return FALSE;
589-
}
590-
*retDataSize = sizeof(DWORD64);
591-
if (value == Py_None) {
592-
DWORD64 zero = 0;
593-
memcpy(*retDataBuf, &zero, sizeof(DWORD64));
594-
}
595-
else {
596-
DWORD64 d = PyLong_AsUnsignedLongLong(value);
588+
case REG_QWORD:
589+
{
590+
if (value != Py_None && !PyLong_Check(value)) {
591+
return FALSE;
592+
}
593+
DWORD64 d;
594+
if (value == Py_None) {
595+
d = 0;
596+
}
597+
else if (PyLong_Check(value)) {
598+
d = PyLong_AsUnsignedLongLong(value);
599+
if (d == (DWORD64)(-1) && PyErr_Occurred()) {
600+
return FALSE;
601+
}
602+
}
603+
*retDataBuf = (BYTE *)PyMem_NEW(DWORD64, 1);
604+
if (*retDataBuf == NULL) {
605+
PyErr_NoMemory();
606+
return FALSE;
607+
}
597608
memcpy(*retDataBuf, &d, sizeof(DWORD64));
609+
*retDataSize = sizeof(DWORD64);
610+
break;
598611
}
599-
break;
600612
case REG_SZ:
601613
case REG_EXPAND_SZ:
602614
{

0 commit comments

Comments
 (0)