gh-123378: fix some corner cases of start and end values in PyUnicodeErrorObject

Doc/c-api/exceptions.rst

@@ -857,8 +857,8 @@ The following functions are used to create and modify Unicode exceptions from C.
                 int PyUnicodeEncodeError_SetStart(PyObject *exc, Py_ssize_t start)
                 int PyUnicodeTranslateError_SetStart(PyObject *exc, Py_ssize_t start)
 
-   Set the *start* attribute of the given exception object to *start*.  Return
-   ``0`` on success, ``-1`` on failure.
+   Set the *start* attribute of the given exception object to *start*.  *start*
+   must be non-negative. Return ``0`` on success, ``-1`` on failure.
 
 .. c:function:: int PyUnicodeDecodeError_GetEnd(PyObject *exc, Py_ssize_t *end)
                 int PyUnicodeEncodeError_GetEnd(PyObject *exc, Py_ssize_t *end)

Doc/library/exceptions.rst

@@ -642,7 +642,8 @@ The following exceptions are the exceptions that are usually raised.
 
    .. attribute:: start
 
-       The first index of invalid data in :attr:`object`.
+       The first index of invalid data in :attr:`object`. This value
+       must be non-negative.
 
    .. attribute:: end
 

Include/cpython/pyerrors.h

@@ -44,7 +44,7 @@ typedef struct {
     PyException_HEAD
     PyObject *encoding;
     PyObject *object;
-    Py_ssize_t start;
+    Py_ssize_t start;  // should be >= 0
     Py_ssize_t end;
     PyObject *reason;
 } PyUnicodeErrorObject;

Lib/test/test_capi/test_exceptions.py

@@ -415,6 +415,69 @@ def test_err_formatunraisable(self):
         # CRASHES formatunraisable(NULL, NULL)
 
 
+class PyUnicodeTranslateError(UnicodeTranslateError):
+    # UnicodeTranslateError takes 4 arguments instead of 5,
+    # so we just make a UnicodeTranslateError class that is
+    # compatible with the UnicodeError.__init__.
+    def __init__(self, encoding, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+
+class TestUnicodeError(unittest.TestCase):
+
+    def test_unicode_error_start_value(self):
+        # negative start is not allowed
+        for exc_type, literal in [
+            (UnicodeEncodeError, 'x'),
+            (UnicodeDecodeError, b'x'),
+            (PyUnicodeTranslateError, 'x'),
+        ]:
+            for obj_len in [0, 1, 2]:
+                s = literal * obj_len
+                with self.subTest(exc_type=exc_type, obj_len=obj_len):
+                    self.assertRaises(ValueError, exc_type, 'utf-8', s, -1, 0, '?')
+
+    def test_unicode_encode_error_get_start(self):
+        test_func = _testcapi.unicode_encode_get_start
+        self._test_unicode_error_get_start('x', UnicodeEncodeError, test_func)
+
+    def test_unicode_decode_error_get_start(self):
+        test_func = _testcapi.unicode_decode_get_start
+        self._test_unicode_error_get_start(b'x', UnicodeDecodeError, test_func)
+
+    def test_unicode_translate_error_get_start(self):
+        test_func = _testcapi.unicode_translate_get_start
+        self._test_unicode_error_get_start('x', PyUnicodeTranslateError, test_func)
+
+    def _test_unicode_error_get_start(self, literal, exc_type, test_func):
+        for obj_len, py_start, c_start in [
+            # normal cases
+            (5, 0, 0),
+            (5, 1, 1),
+            (5, 2, 2),
+            # out of range start is clamped to max(0, obj_len - 1)
+            (0, 0, 0),
+            (0, 1, 0),
+            (0, 10, 0),
+            (5, 5, 4),
+            (5, 10, 4),
+        ]:
+            c_start_computed = py_start
+            if c_start_computed >= obj_len:
+                if obj_len == 0:
+                    c_start_computed = 0
+                else:
+                    c_start_computed = obj_len - 1
+
+            s = literal * obj_len
+            py_end = py_start + 1
+
+            with self.subTest(s, exc_type=exc_type, py_start=py_start, c_start=c_start):
+                self.assertEqual(c_start, c_start_computed)
+                exc = exc_type('utf-8', s, py_start, py_end, 'why')
+                c_start_actual = test_func(exc)
+                self.assertEqual(c_start_actual, c_start)
+
 class Test_PyUnstable_Exc_PrepReraiseStar(ExceptionIsLikeMixin, unittest.TestCase):
 
     def setUp(self):

Misc/NEWS.d/next/C API/2024-08-27-09-07-56.gh-issue-123378.JJ6n_u.rst

@@ -0,0 +1,5 @@
+Ensure that *start* is correctly set on :exc:`UnicodeEncodeError` objects.
+A negative *start* is not allowed by :c:func:`PyUnicodeEncodeError_SetStart`
+and will not be returned by :c:func:`PyUnicodeEncodeError_GetStart`. Similar
+arguments apply to :exc:`UnicodeDecodeError` and :exc:`UnicodeTranslateError`
+and their corresponding C interface. Patch by Bénédikt Tran.

Modules/_testcapi/exceptions.c

@@ -359,6 +359,39 @@ _testcapi_unstable_exc_prep_reraise_star_impl(PyObject *module,
     return PyUnstable_Exc_PrepReraiseStar(orig, excs);
 }
 
+/* Test PyUnicodeEncodeError_GetStart */
+static PyObject *
+unicode_encode_get_start(PyObject *Py_UNUSED(module), PyObject *arg)
+{
+    Py_ssize_t start;
+    if (PyUnicodeEncodeError_GetStart(arg, &start) < 0) {
+        return NULL;
+    }
+    RETURN_SIZE(start);
+}
+
+/* Test PyUnicodeDecodeError_GetStart */
+static PyObject *
+unicode_decode_get_start(PyObject *Py_UNUSED(module), PyObject *arg)
+{
+    Py_ssize_t start;
+    if (PyUnicodeDecodeError_GetStart(arg, &start) < 0) {
+        return NULL;
+    }
+    RETURN_SIZE(start);
+}
+
+/* Test PyUnicodeTranslateError_GetStart */
+static PyObject *
+unicode_translate_get_start(PyObject *Py_UNUSED(module), PyObject *arg)
+{
+    Py_ssize_t start;
+    if (PyUnicodeTranslateError_GetStart(arg, &start) < 0) {
+        return NULL;
+    }
+    RETURN_SIZE(start);
+}
+
 
 /*
  * Define the PyRecurdingInfinitelyError_Type
@@ -403,6 +436,9 @@ static PyMethodDef test_methods[] = {
     _TESTCAPI_SET_EXCEPTION_METHODDEF
     _TESTCAPI_TRACEBACK_PRINT_METHODDEF
     _TESTCAPI_UNSTABLE_EXC_PREP_RERAISE_STAR_METHODDEF
+    {"unicode_encode_get_start", unicode_encode_get_start,       METH_O},
+    {"unicode_decode_get_start", unicode_decode_get_start,       METH_O},
+    {"unicode_translate_get_start", unicode_translate_get_start, METH_O},
     {NULL},
 };
 

Objects/exceptions.c

@@ -2741,17 +2741,17 @@ PyUnicodeTranslateError_GetObject(PyObject *exc)
 int
 PyUnicodeEncodeError_GetStart(PyObject *exc, Py_ssize_t *start)
 {
-    Py_ssize_t size;
     PyObject *obj = get_unicode(((PyUnicodeErrorObject *)exc)->object,
                                 "object");
-    if (!obj)
+    if (!obj) {
         return -1;
+    }
+    Py_ssize_t size = PyUnicode_GET_LENGTH(obj);
     *start = ((PyUnicodeErrorObject *)exc)->start;
-    size = PyUnicode_GET_LENGTH(obj);
-    if (*start<0)
-        *start = 0; /*XXX check for values <0*/
-    if (*start>=size)
-        *start = size-1;
+    assert(*start >= 0);
+    if (*start >= size) {
+        *start = size ? size - 1 : 0;
+    }
     Py_DECREF(obj);
     return 0;
 }
@@ -2760,16 +2760,16 @@ PyUnicodeEncodeError_GetStart(PyObject *exc, Py_ssize_t *start)
 int
 PyUnicodeDecodeError_GetStart(PyObject *exc, Py_ssize_t *start)
 {
-    Py_ssize_t size;
     PyObject *obj = get_string(((PyUnicodeErrorObject *)exc)->object, "object");
-    if (!obj)
+    if (!obj) {
         return -1;
-    size = PyBytes_GET_SIZE(obj);
+    }
+    Py_ssize_t size = PyBytes_GET_SIZE(obj);
     *start = ((PyUnicodeErrorObject *)exc)->start;
-    if (*start<0)
-        *start = 0;
-    if (*start>=size)
-        *start = size-1;
+    assert(*start >= 0);
+    if (*start >= size) {
+        *start = size ? size - 1 : 0;
+    }
     Py_DECREF(obj);
     return 0;
 }
@@ -2785,6 +2785,10 @@ PyUnicodeTranslateError_GetStart(PyObject *exc, Py_ssize_t *start)
 int
 PyUnicodeEncodeError_SetStart(PyObject *exc, Py_ssize_t start)
 {
+    if (start < 0) {
+        PyErr_SetString(PyExc_ValueError, "'start' must be >= 0");
+        return -1;
+    }
     ((PyUnicodeErrorObject *)exc)->start = start;
     return 0;
 }
@@ -2793,6 +2797,10 @@ PyUnicodeEncodeError_SetStart(PyObject *exc, Py_ssize_t start)
 int
 PyUnicodeDecodeError_SetStart(PyObject *exc, Py_ssize_t start)
 {
+    if (start < 0) {
+        PyErr_SetString(PyExc_ValueError, "'start' must be >= 0");
+        return -1;
+    }
     ((PyUnicodeErrorObject *)exc)->start = start;
     return 0;
 }
@@ -2801,6 +2809,10 @@ PyUnicodeDecodeError_SetStart(PyObject *exc, Py_ssize_t start)
 int
 PyUnicodeTranslateError_SetStart(PyObject *exc, Py_ssize_t start)
 {
+    if (start < 0) {
+        PyErr_SetString(PyExc_ValueError, "'start' must be >= 0");
+        return -1;
+    }
     ((PyUnicodeErrorObject *)exc)->start = start;
     return 0;
 }
@@ -2980,15 +2992,22 @@ UnicodeEncodeError_init(PyObject *self, PyObject *args, PyObject *kwds)
     if (!PyArg_ParseTuple(args, "UUnnU",
                           &err->encoding, &err->object,
                           &err->start, &err->end, &err->reason)) {
-        err->encoding = err->object = err->reason = NULL;
-        return -1;
+        goto error;
+    }
+
+    if (err->start < 0) {
+        PyErr_SetString(PyExc_ValueError, "'start' must be >= 0");
+        goto error;
     }
 
     Py_INCREF(err->encoding);
     Py_INCREF(err->object);
     Py_INCREF(err->reason);
 
     return 0;
+error:
+    err->encoding = err->object = err->reason = NULL;
+    return -1;
 }
 
 static PyObject *
@@ -3086,6 +3105,11 @@ UnicodeDecodeError_init(PyObject *self, PyObject *args, PyObject *kwds)
     Py_INCREF(ude->object);
     Py_INCREF(ude->reason);
 
+    if (ude->start < 0) {
+        PyErr_SetString(PyExc_ValueError, "'start' must be >= 0");
+        goto error;
+    }
+
     if (!PyBytes_Check(ude->object)) {
         Py_buffer view;
         if (PyObject_GetBuffer(ude->object, &view, PyBUF_SIMPLE) != 0)
@@ -3190,14 +3214,21 @@ UnicodeTranslateError_init(PyUnicodeErrorObject *self, PyObject *args,
     if (!PyArg_ParseTuple(args, "UnnU",
                           &self->object,
                           &self->start, &self->end, &self->reason)) {
-        self->object = self->reason = NULL;
-        return -1;
+        goto error;
+    }
+
+    if (self->start < 0) {
+        PyErr_SetString(PyExc_ValueError, "'start' must be >= 0");
+        goto error;
     }
 
     Py_INCREF(self->object);
     Py_INCREF(self->reason);
 
     return 0;
+error:
+    self->object = self->reason = NULL;
+    return -1;
 }