gh-74020: Raise ValueError for negative values converted to unsigned

Doc/c-api/arg.rst

@@ -249,6 +249,10 @@ small to receive the value.
    Convert a nonnegative Python integer to an unsigned tiny integer, stored in a C
    :c:expr:`unsigned char`.
 
+   .. versionchanged:: next
+      A negative Python integer now raises :exc:`ValueError`,
+      not :exc:`OverflowError`.
+
 ``B`` (:class:`int`) [unsigned char]
    Convert a Python integer to a tiny integer without overflow checking, stored in a C
    :c:expr:`unsigned char`.

Doc/c-api/long.rst

@@ -286,12 +286,15 @@ distinguished from a number.  Use :c:func:`PyErr_Occurred` to disambiguate.
    Return a C :c:expr:`unsigned long` representation of *pylong*.  *pylong*
    must be an instance of :c:type:`PyLongObject`.
 
-   Raise :exc:`OverflowError` if the value of *pylong* is out of range for a
-   :c:expr:`unsigned long`.
+   Raise :exc:`ValueError` if the value of *pylong* is negative and
+   :exc:`OverflowError` if it is out of range for a :c:expr:`unsigned long`.
 
    Returns ``(unsigned long)-1`` on error.
    Use :c:func:`PyErr_Occurred` to disambiguate.
 
+   .. versionchanged:: next
+      A negative *pylong* now raises :exc:`ValueError`, not :exc:`OverflowError`.
+
 
 .. c:function:: size_t PyLong_AsSize_t(PyObject *pylong)
 
@@ -302,12 +305,15 @@ distinguished from a number.  Use :c:func:`PyErr_Occurred` to disambiguate.
    Return a C :c:type:`size_t` representation of *pylong*.  *pylong* must be
    an instance of :c:type:`PyLongObject`.
 
-   Raise :exc:`OverflowError` if the value of *pylong* is out of range for a
-   :c:type:`size_t`.
+   Raise :exc:`ValueError` if the value of *pylong* is negative and
+   :exc:`OverflowError` if it is out of range for a :c:type:`size_t`.
 
    Returns ``(size_t)-1`` on error.
    Use :c:func:`PyErr_Occurred` to disambiguate.
 
+   .. versionchanged:: next
+      A negative *pylong* now raises :exc:`ValueError`, not :exc:`OverflowError`.
+
 
 .. c:function:: unsigned long long PyLong_AsUnsignedLongLong(PyObject *pylong)
 
@@ -317,15 +323,18 @@ distinguished from a number.  Use :c:func:`PyErr_Occurred` to disambiguate.
    Return a C :c:expr:`unsigned long long` representation of *pylong*.  *pylong*
    must be an instance of :c:type:`PyLongObject`.
 
-   Raise :exc:`OverflowError` if the value of *pylong* is out of range for an
-   :c:expr:`unsigned long long`.
+   Raise :exc:`ValueError` if the value of *pylong* is negative and
+   :exc:`OverflowError` if it is out of range for a :c:expr:`unsigned long long`.
 
    Returns ``(unsigned long long)-1`` on error.
    Use :c:func:`PyErr_Occurred` to disambiguate.
 
    .. versionchanged:: 3.1
       A negative *pylong* now raises :exc:`OverflowError`, not :exc:`TypeError`.
 
+   .. versionchanged:: next
+      A negative *pylong* now raises :exc:`ValueError`, not :exc:`OverflowError`.
+
 
 .. c:function:: unsigned long PyLong_AsUnsignedLongMask(PyObject *obj)
 

Doc/whatsnew/3.14.rst

@@ -1920,6 +1920,10 @@ Others
   integer must implement either :meth:`~object.__int__` or
   :meth:`~object.__index__`. (Contributed by Mark Dickinson in :gh:`119743`.)
 
+* Converting negative Python integer to a C unsigned integer type now raises
+  :exc:`ValueError`, not :exc:`OverflowError`.
+  (Contributed by Serhiy Storchaka in :gh:`74020`.)
+
 
 CPython Bytecode Changes
 ========================

Lib/test/test_array.py

@@ -1326,8 +1326,9 @@ def check_overflow(self, lower, upper):
         a = array.array(self.typecode, [lower])
         a[0] = lower
         # should overflow assigning less than lower limit
-        self.assertRaises(OverflowError, array.array, self.typecode, [lower-1])
-        self.assertRaises(OverflowError, a.__setitem__, 0, lower-1)
+        exc = ValueError if int(lower) == 0 else OverflowError
+        self.assertRaises(exc, array.array, self.typecode, [lower-1])
+        self.assertRaises(exc, a.__setitem__, 0, lower-1)
         # should not overflow assigning upper limit
         a = array.array(self.typecode, [upper])
         a[0] = upper

Lib/test/test_capi/test_getargs.py

@@ -182,7 +182,7 @@ def test_b(self):
         self.assertRaises(TypeError, getargs_b, BadInt2())
         self.assertEqual(0, getargs_b(BadInt3()))
 
-        self.assertRaises(OverflowError, getargs_b, -1)
+        self.assertRaises(ValueError, getargs_b, -1)
         self.assertEqual(0, getargs_b(0))
         self.assertEqual(UCHAR_MAX, getargs_b(UCHAR_MAX))
         self.assertRaises(OverflowError, getargs_b, UCHAR_MAX + 1)

Lib/test/test_capi/test_long.py

@@ -165,8 +165,7 @@ def test_long_fromunicodeobject(self):
 
     def check_long_asint(self, func, min_val, max_val, *,
                          use_index=True,
-                         mask=False,
-                         negative_value_error=OverflowError):
+                         mask=False):
         # round trip (object -> C integer -> object)
         values = (0, 1, 512, 1234, max_val)
         if min_val < 0:
@@ -190,6 +189,7 @@ def check_long_asint(self, func, min_val, max_val, *,
             self.assertEqual(func(-1 << 1000), 0)
             self.assertEqual(func(1 << 1000), 0)
         else:
+            negative_value_error = ValueError if min_val == 0 else OverflowError
             self.assertRaises(negative_value_error, func, min_val - 1)
             self.assertRaises(negative_value_error, func, -1 << 1000)
             self.assertRaises(OverflowError, func, max_val + 1)
@@ -236,8 +236,7 @@ def test_long_asunsignedlong(self):
         # Test PyLong_AsUnsignedLong() and PyLong_FromUnsignedLong()
         asunsignedlong = _testlimitedcapi.pylong_asunsignedlong
         from _testcapi import ULONG_MAX
-        self.check_long_asint(asunsignedlong, 0, ULONG_MAX,
-                                      use_index=False)
+        self.check_long_asint(asunsignedlong, 0, ULONG_MAX, use_index=False)
 
     def test_long_asunsignedlongmask(self):
         # Test PyLong_AsUnsignedLongMask()
@@ -704,15 +703,13 @@ def test_long_asuint32(self):
         # Test PyLong_AsUInt32() and PyLong_FromUInt32()
         as_uint32 = _testlimitedcapi.pylong_asuint32
         from _testcapi import UINT32_MAX
-        self.check_long_asint(as_uint32, 0, UINT32_MAX,
-                              negative_value_error=ValueError)
+        self.check_long_asint(as_uint32, 0, UINT32_MAX)
 
     def test_long_asuint64(self):
         # Test PyLong_AsUInt64() and PyLong_FromUInt64()
         as_uint64 = _testlimitedcapi.pylong_asuint64
         from _testcapi import UINT64_MAX
-        self.check_long_asint(as_uint64, 0, UINT64_MAX,
-                              negative_value_error=ValueError)
+        self.check_long_asint(as_uint64, 0, UINT64_MAX)
 
     def test_long_layout(self):
         # Test PyLong_GetNativeLayout()

Lib/test/test_long.py

@@ -1377,8 +1377,8 @@ def equivalent_python(n, length, byteorder, signed=False):
         self.assertRaises(OverflowError, (256).to_bytes, 1, 'big', signed=True)
         self.assertRaises(OverflowError, (256).to_bytes, 1, 'little', signed=False)
         self.assertRaises(OverflowError, (256).to_bytes, 1, 'little', signed=True)
-        self.assertRaises(OverflowError, (-1).to_bytes, 2, 'big', signed=False)
-        self.assertRaises(OverflowError, (-1).to_bytes, 2, 'little', signed=False)
+        self.assertRaises(ValueError, (-1).to_bytes, 2, 'big', signed=False)
+        self.assertRaises(ValueError, (-1).to_bytes, 2, 'little', signed=False)
         self.assertEqual((0).to_bytes(0, 'big'), b'')
         self.assertEqual((1).to_bytes(5, 'big'), b'\x00\x00\x00\x00\x01')
         self.assertEqual((0).to_bytes(5, 'big'), b'\x00\x00\x00\x00\x00')

Lib/test/test_lzma.py

@@ -657,9 +657,9 @@ def test_init_bad_preset(self):
             LZMAFile(BytesIO(), "w", preset=10)
         with self.assertRaises(LZMAError):
             LZMAFile(BytesIO(), "w", preset=23)
-        with self.assertRaises(OverflowError):
+        with self.assertRaises(ValueError):
             LZMAFile(BytesIO(), "w", preset=-1)
-        with self.assertRaises(OverflowError):
+        with self.assertRaises(ValueError):
             LZMAFile(BytesIO(), "w", preset=-7)
         with self.assertRaises(TypeError):
             LZMAFile(BytesIO(), "w", preset="foo")

Lib/test/test_os.py

@@ -3564,7 +3564,7 @@ def test_waitstatus_to_exitcode_windows(self):
         # invalid values
         with self.assertRaises(ValueError):
             os.waitstatus_to_exitcode((max_exitcode + 1) << 8)
-        with self.assertRaises(OverflowError):
+        with self.assertRaises(ValueError):
             os.waitstatus_to_exitcode(-1)
 
     # Skip the test on Windows

Lib/test/test_socket.py

@@ -1169,7 +1169,7 @@ def testInterfaceNameIndex(self):
     @support.skip_android_selinux('if_indextoname')
     def testInvalidInterfaceIndexToName(self):
         self.assertRaises(OSError, socket.if_indextoname, 0)
-        self.assertRaises(OverflowError, socket.if_indextoname, -1)
+        self.assertRaises(ValueError, socket.if_indextoname, -1)
         self.assertRaises(OverflowError, socket.if_indextoname, 2**1000)
         self.assertRaises(TypeError, socket.if_indextoname, '_DEADBEEF')
         if hasattr(socket, 'if_nameindex'):
@@ -1231,18 +1231,23 @@ def testNtoHErrors(self):
         import _testcapi
         s_good_values = [0, 1, 2, 0xffff]
         l_good_values = s_good_values + [0xffffffff]
-        l_bad_values = [-1, -2, 1<<32, 1<<1000]
+        neg_values = [-1, -2, -(1<<15)-1, -(1<<31)-1, -(1<<63)-1, -1<<1000]
+        l_bad_values = [1<<32, 1<<1000]
         s_bad_values = (
             l_bad_values +
-            [_testcapi.INT_MIN-1, _testcapi.INT_MAX+1] +
-            [1 << 16, _testcapi.INT_MAX]
+            [1 << 16, _testcapi.INT_MAX, _testcapi.INT_MAX+1]
         )
         for k in s_good_values:
             socket.ntohs(k)
             socket.htons(k)
         for k in l_good_values:
             socket.ntohl(k)
             socket.htonl(k)
+        for k in neg_values:
+            self.assertRaises(ValueError, socket.ntohs, k)
+            self.assertRaises(ValueError, socket.htons, k)
+            self.assertRaises(ValueError, socket.ntohl, k)
+            self.assertRaises(ValueError, socket.htonl, k)
         for k in s_bad_values:
             self.assertRaises(OverflowError, socket.ntohs, k)
             self.assertRaises(OverflowError, socket.htons, k)

Lib/test/test_ssl.py

@@ -986,7 +986,7 @@ def test_options(self):
         self.assertEqual(0, ctx.options & ~ssl.OP_NO_SSLv3)
 
         # invalid options
-        with self.assertRaises(OverflowError):
+        with self.assertRaises(ValueError):
             ctx.options = -1
         with self.assertRaises(OverflowError):
             ctx.options = 2 ** 100

Lib/test/test_winreg.py

@@ -359,8 +359,9 @@ def test_setvalueex_negative_one_check(self):
         # the value set was not -1.
         try:
             with CreateKey(HKEY_CURRENT_USER, test_key_name) as ck:
-                with self.assertRaises(OverflowError):
+                with self.assertRaises(ValueError):
                     SetValueEx(ck, "test_name_dword", None, REG_DWORD, -1)
+                with self.assertRaises(ValueError):
                     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")

Misc/NEWS.d/next/Core_and_Builtins/2024-06-28-10-51-56.gh-issue-74020.FcEAzj.rst

@@ -0,0 +1,2 @@
+Converting negative Python integer to a C unsigned integer type now raises
+:exc:`ValueError`, not :exc:`OverflowError`.

Modules/_struct.c

@@ -633,7 +633,9 @@ np_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f)
     unsigned long x;
     unsigned int y;
     if (get_ulong(state, v, &x) < 0) {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError) ||
+            PyErr_ExceptionMatches(PyExc_ValueError))
+        {
             RANGE_ERROR(state, f, 1);
         }
         return -1;
@@ -666,7 +668,9 @@ np_ulong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f)
 {
     unsigned long x;
     if (get_ulong(state, v, &x) < 0) {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError) ||
+            PyErr_ExceptionMatches(PyExc_ValueError))
+        {
             RANGE_ERROR(state, f, 1);
         }
         return -1;
@@ -694,7 +698,9 @@ np_size_t(_structmodulestate *state, char *p, PyObject *v, const formatdef *f)
 {
     size_t x;
     if (get_size_t(state, v, &x) < 0) {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError) ||
+            PyErr_ExceptionMatches(PyExc_ValueError))
+        {
             RANGE_ERROR(state, f, 1);
         }
         return -1;
@@ -726,7 +732,9 @@ np_ulonglong(_structmodulestate *state, char *p, PyObject *v, const formatdef *f
 {
     unsigned long long x;
     if (get_ulonglong(state, v, &x) < 0) {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError) ||
+            PyErr_ExceptionMatches(PyExc_ValueError))
+        {
             PyErr_Format(state->StructError,
                          "'%c' format requires 0 <= number <= %llu",
                          f->format,
@@ -1058,7 +1066,9 @@ bp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f)
     Py_ssize_t i;
     unsigned char *q = (unsigned char *)p;
     if (get_ulong(state, v, &x) < 0) {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError) ||
+            PyErr_ExceptionMatches(PyExc_ValueError))
+        {
             RANGE_ERROR(state, f, 1);
         }
         return -1;
@@ -1391,7 +1401,9 @@ lp_uint(_structmodulestate *state, char *p, PyObject *v, const formatdef *f)
     Py_ssize_t i;
     unsigned char *q = (unsigned char *)p;
     if (get_ulong(state, v, &x) < 0) {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError) ||
+            PyErr_ExceptionMatches(PyExc_ValueError))
+        {
             RANGE_ERROR(state, f, 1);
         }
         return -1;
@@ -2213,9 +2225,13 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset,
                 *res = Py_SAFE_DOWNCAST(n, Py_ssize_t, unsigned char);
             } else {
                 if (e->pack(state, res, v, e) < 0) {
-                    if (PyLong_Check(v) && PyErr_ExceptionMatches(PyExc_OverflowError))
+                    if (PyLong_Check(v) &&
+                        (PyErr_ExceptionMatches(PyExc_OverflowError) ||
+                         PyErr_ExceptionMatches(PyExc_ValueError)))
+                    {
                         PyErr_SetString(state->StructError,
                                         "int too large to convert");
+                    }
                     return -1;
                 }
             }

Modules/_testlimitedcapi/testcapi_long.h

@@ -96,10 +96,10 @@ TESTNAME(PyObject *error(const char*))
         if (uout != (unsigned TYPENAME)-1 || !PyErr_Occurred())
             return error(
                 "PyLong_AsUnsignedXXX(-1) didn't complain");
-        if (!PyErr_ExceptionMatches(PyExc_OverflowError))
+        if (!PyErr_ExceptionMatches(PyExc_ValueError))
             return error(
                 "PyLong_AsUnsignedXXX(-1) raised "
-                "something other than OverflowError");
+                "something other than ValueError");
         PyErr_Clear();
         UNBIND(x);
 

Modules/arraymodule.c

@@ -214,20 +214,23 @@ b_getitem(arrayobject *ap, Py_ssize_t i)
 static int
 b_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
 {
-    short x;
-    /* PyArg_Parse's 'b' formatter is for an unsigned char, therefore
-       must use the next size up that is signed ('h') and manually do
-       the overflow checking */
-    if (!PyArg_Parse(v, "h;array item must be integer", &x))
+    int overflow;
+    long x = PyLong_AsLongAndOverflow(v, &overflow);
+    if (x == -1 && PyErr_Occurred()) {
+        if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+            PyErr_SetString(PyExc_TypeError,
+                "array item must be integer");
+        }
         return -1;
-    else if (x < -128) {
+    }
+    if (overflow > 0 || x > 127) {
         PyErr_SetString(PyExc_OverflowError,
-            "signed char is less than minimum");
+            "signed char is greater than maximum");
         return -1;
     }
-    else if (x > 127) {
+    if (overflow < 0 || x < -128) {
         PyErr_SetString(PyExc_OverflowError,
-            "signed char is greater than maximum");
+            "signed char is less than minimum");
         return -1;
     }
     if (i >= 0)
@@ -355,21 +358,25 @@ HH_getitem(arrayobject *ap, Py_ssize_t i)
 static int
 HH_setitem(arrayobject *ap, Py_ssize_t i, PyObject *v)
 {
-    int x;
-    /* PyArg_Parse's 'h' formatter is for a signed short, therefore
-       must use the next size up and manually do the overflow checking */
-    if (!PyArg_Parse(v, "i;array item must be integer", &x))
-        return -1;
-    else if (x < 0) {
-        PyErr_SetString(PyExc_OverflowError,
-            "unsigned short is less than minimum");
+    int overflow;
+    long x = PyLong_AsLongAndOverflow(v, &overflow);
+    if (x == -1 && PyErr_Occurred()) {
+        if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+            PyErr_SetString(PyExc_TypeError,
+                "array item must be integer");
+        }
         return -1;
     }
-    else if (x > USHRT_MAX) {
+    if (overflow > 0 || x > USHRT_MAX) {
         PyErr_SetString(PyExc_OverflowError,
             "unsigned short is greater than maximum");
         return -1;
     }
+    if (overflow < 0 || x < 0) {
+        PyErr_SetString(PyExc_ValueError,
+            "unsigned short is less than minimum");
+        return -1;
+    }
     if (i >= 0)
         ((short *)ap->ob_item)[i] = (short)x;
     return 0;