gh-107915: Handle errors in C API functions PyErr_Set*() and PyErr_Format() (GH-107918)

Such C API functions as PyErr_SetString(), PyErr_Format(),
PyErr_SetFromErrnoWithFilename() and many others no longer crash or
ignore errors if it failed to format the error message or decode the
filename. Instead, they keep a corresponding error.

Lib/test/test_capi/test_exceptions.py

@@ -1,9 +1,12 @@
+import errno
+import os
 import re
 import sys
 import unittest
 
 from test import support
 from test.support import import_helper
+from test.support.os_helper import TESTFN, TESTFN_UNDECODABLE
 from test.support.script_helper import assert_python_failure
 from test.support.testcase import ExceptionIsLikeMixin
 
@@ -12,6 +15,8 @@ from .test_misc import decode_stderr
 # Skip this test if the _testcapi module isn't available.
 _testcapi = import_helper.import_module('_testcapi')
 
+NULL = None
+
 class Test_Exceptions(unittest.TestCase):
 
     def test_exception(self):
@@ -189,6 +194,82 @@ class Test_ErrSetAndRestore(unittest.TestCase):
         self.assertEqual(exc.__notes__[0],
                          'Normalization failed: type=Broken args=<unknown>')
 
+    def test_set_string(self):
+        """Test PyErr_SetString()"""
+        setstring = _testcapi.err_setstring
+        with self.assertRaises(ZeroDivisionError) as e:
+            setstring(ZeroDivisionError, b'error')
+        self.assertEqual(e.exception.args, ('error',))
+        with self.assertRaises(ZeroDivisionError) as e:
+            setstring(ZeroDivisionError, 'помилка'.encode())
+        self.assertEqual(e.exception.args, ('помилка',))
+
+        with self.assertRaises(UnicodeDecodeError):
+            setstring(ZeroDivisionError, b'\xff')
+        self.assertRaises(SystemError, setstring, list, b'error')
+        # CRASHES setstring(ZeroDivisionError, NULL)
+        # CRASHES setstring(NULL, b'error')
+
+    def test_format(self):
+        """Test PyErr_Format()"""
+        import_helper.import_module('ctypes')
+        from ctypes import pythonapi, py_object, c_char_p, c_int
+        name = "PyErr_Format"
+        PyErr_Format = getattr(pythonapi, name)
+        PyErr_Format.argtypes = (py_object, c_char_p,)
+        PyErr_Format.restype = py_object
+        with self.assertRaises(ZeroDivisionError) as e:
+            PyErr_Format(ZeroDivisionError, b'%s %d', b'error', c_int(42))
+        self.assertEqual(e.exception.args, ('error 42',))
+        with self.assertRaises(ZeroDivisionError) as e:
+            PyErr_Format(ZeroDivisionError, b'%s', 'помилка'.encode())
+        self.assertEqual(e.exception.args, ('помилка',))
+
+        with self.assertRaisesRegex(OverflowError, 'not in range'):
+            PyErr_Format(ZeroDivisionError, b'%c', c_int(-1))
+        with self.assertRaisesRegex(ValueError, 'format string'):
+            PyErr_Format(ZeroDivisionError, b'\xff')
+        self.assertRaises(SystemError, PyErr_Format, list, b'error')
+        # CRASHES PyErr_Format(ZeroDivisionError, NULL)
+        # CRASHES PyErr_Format(py_object(), b'error')
+
+    def test_setfromerrnowithfilename(self):
+        """Test PyErr_SetFromErrnoWithFilename()"""
+        setfromerrnowithfilename = _testcapi.err_setfromerrnowithfilename
+        ENOENT = errno.ENOENT
+        with self.assertRaises(FileNotFoundError) as e:
+            setfromerrnowithfilename(ENOENT, OSError, b'file')
+        self.assertEqual(e.exception.args,
+                         (ENOENT, 'No such file or directory'))
+        self.assertEqual(e.exception.errno, ENOENT)
+        self.assertEqual(e.exception.filename, 'file')
+
+        with self.assertRaises(FileNotFoundError) as e:
+            setfromerrnowithfilename(ENOENT, OSError, os.fsencode(TESTFN))
+        self.assertEqual(e.exception.filename, TESTFN)
+
+        if TESTFN_UNDECODABLE:
+            with self.assertRaises(FileNotFoundError) as e:
+                setfromerrnowithfilename(ENOENT, OSError, TESTFN_UNDECODABLE)
+            self.assertEqual(e.exception.filename,
+                             os.fsdecode(TESTFN_UNDECODABLE))
+
+        with self.assertRaises(FileNotFoundError) as e:
+            setfromerrnowithfilename(ENOENT, OSError, NULL)
+        self.assertIsNone(e.exception.filename)
+
+        with self.assertRaises(OSError) as e:
+            setfromerrnowithfilename(0, OSError, b'file')
+        self.assertEqual(e.exception.args, (0, 'Error'))
+        self.assertEqual(e.exception.errno, 0)
+        self.assertEqual(e.exception.filename, 'file')
+
+        with self.assertRaises(ZeroDivisionError) as e:
+            setfromerrnowithfilename(ENOENT, ZeroDivisionError, b'file')
+        self.assertEqual(e.exception.args,
+                         (ENOENT, 'No such file or directory', 'file'))
+        # CRASHES setfromerrnowithfilename(ENOENT, NULL, b'error')
+
 
 class Test_PyUnstable_Exc_PrepReraiseStar(ExceptionIsLikeMixin, unittest.TestCase):
 

Misc/NEWS.d/next/C API/2023-08-13-12-33-00.gh-issue-107915.jQ0wOi.rst

@@ -0,0 +1,4 @@
+Such C API functions as ``PyErr_SetString()``, ``PyErr_Format()``,
+``PyErr_SetFromErrnoWithFilename()`` and many others no longer crash or
+ignore errors if it failed to format the error message or decode the
+filename. Instead, they keep a corresponding error.

Modules/_testcapi/clinic/exceptions.c.h

@@ -215,6 +215,68 @@ exit:
     return return_value;
 }
 
+PyDoc_STRVAR(_testcapi_err_setstring__doc__,
+"err_setstring($module, exc, value, /)\n"
+"--\n"
+"\n");
+
+#define _TESTCAPI_ERR_SETSTRING_METHODDEF    \
+    {"err_setstring", _PyCFunction_CAST(_testcapi_err_setstring), METH_FASTCALL, _testcapi_err_setstring__doc__},
+
+static PyObject *
+_testcapi_err_setstring_impl(PyObject *module, PyObject *exc,
+                             const char *value, Py_ssize_t value_length);
+
+static PyObject *
+_testcapi_err_setstring(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
+{
+    PyObject *return_value = NULL;
+    PyObject *exc;
+    const char *value;
+    Py_ssize_t value_length;
+
+    if (!_PyArg_ParseStack(args, nargs, "Oz#:err_setstring",
+        &exc, &value, &value_length)) {
+        goto exit;
+    }
+    return_value = _testcapi_err_setstring_impl(module, exc, value, value_length);
+
+exit:
+    return return_value;
+}
+
+PyDoc_STRVAR(_testcapi_err_setfromerrnowithfilename__doc__,
+"err_setfromerrnowithfilename($module, error, exc, value, /)\n"
+"--\n"
+"\n");
+
+#define _TESTCAPI_ERR_SETFROMERRNOWITHFILENAME_METHODDEF    \
+    {"err_setfromerrnowithfilename", _PyCFunction_CAST(_testcapi_err_setfromerrnowithfilename), METH_FASTCALL, _testcapi_err_setfromerrnowithfilename__doc__},
+
+static PyObject *
+_testcapi_err_setfromerrnowithfilename_impl(PyObject *module, int error,
+                                            PyObject *exc, const char *value,
+                                            Py_ssize_t value_length);
+
+static PyObject *
+_testcapi_err_setfromerrnowithfilename(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
+{
+    PyObject *return_value = NULL;
+    int error;
+    PyObject *exc;
+    const char *value;
+    Py_ssize_t value_length;
+
+    if (!_PyArg_ParseStack(args, nargs, "iOz#:err_setfromerrnowithfilename",
+        &error, &exc, &value, &value_length)) {
+        goto exit;
+    }
+    return_value = _testcapi_err_setfromerrnowithfilename_impl(module, error, exc, value, value_length);
+
+exit:
+    return return_value;
+}
+
 PyDoc_STRVAR(_testcapi_raise_exception__doc__,
 "raise_exception($module, exception, num_args, /)\n"
 "--\n"
@@ -426,4 +488,4 @@ _testcapi_unstable_exc_prep_reraise_star(PyObject *module, PyObject *const *args
 exit:
     return return_value;
 }
-/*[clinic end generated code: output=fd6aef54f195c77b input=a9049054013a1b77]*/
+/*[clinic end generated code: output=d574342d716e98b5 input=a9049054013a1b77]*/

Modules/_testcapi/exceptions.c

@@ -1,6 +1,8 @@
 #include "parts.h"
 #include "clinic/exceptions.c.h"
 
+#define NULLABLE(x) do { if (x == Py_None) x = NULL; } while (0);
+
 /*[clinic input]
 module _testcapi
 [clinic start generated code]*/
@@ -129,6 +131,43 @@ _testcapi_exc_set_object_fetch_impl(PyObject *module, PyObject *exc,
     return value;
 }
 
+/*[clinic input]
+_testcapi.err_setstring
+    exc: object
+    value: str(zeroes=True, accept={robuffer, str, NoneType})
+    /
+[clinic start generated code]*/
+
+static PyObject *
+_testcapi_err_setstring_impl(PyObject *module, PyObject *exc,
+                             const char *value, Py_ssize_t value_length)
+/*[clinic end generated code: output=fba8705e5703dd3f input=e8a95fad66d9004b]*/
+{
+    NULLABLE(exc);
+    PyErr_SetString(exc, value);
+    return NULL;
+}
+
+/*[clinic input]
+_testcapi.err_setfromerrnowithfilename
+    error: int
+    exc: object
+    value: str(zeroes=True, accept={robuffer, str, NoneType})
+    /
+[clinic start generated code]*/
+
+static PyObject *
+_testcapi_err_setfromerrnowithfilename_impl(PyObject *module, int error,
+                                            PyObject *exc, const char *value,
+                                            Py_ssize_t value_length)
+/*[clinic end generated code: output=d02df5749a01850e input=ff7c384234bf097f]*/
+{
+    NULLABLE(exc);
+    errno = error;
+    PyErr_SetFromErrnoWithFilename(exc, value);
+    return NULL;
+}
+
 /*[clinic input]
 _testcapi.raise_exception
     exception as exc: object
@@ -338,6 +377,8 @@ static PyMethodDef test_methods[] = {
     _TESTCAPI_MAKE_EXCEPTION_WITH_DOC_METHODDEF
     _TESTCAPI_EXC_SET_OBJECT_METHODDEF
     _TESTCAPI_EXC_SET_OBJECT_FETCH_METHODDEF
+    _TESTCAPI_ERR_SETSTRING_METHODDEF
+    _TESTCAPI_ERR_SETFROMERRNOWITHFILENAME_METHODDEF
     _TESTCAPI_RAISE_EXCEPTION_METHODDEF
     _TESTCAPI_RAISE_MEMORYERROR_METHODDEF
     _TESTCAPI_SET_EXC_INFO_METHODDEF

Python/errors.c

@@ -292,8 +292,10 @@ _PyErr_SetString(PyThreadState *tstate, PyObject *exception,
                  const char *string)
 {
     PyObject *value = PyUnicode_FromString(string);
-    _PyErr_SetObject(tstate, exception, value);
-    Py_XDECREF(value);
+    if (value != NULL) {
+        _PyErr_SetObject(tstate, exception, value);
+        Py_DECREF(value);
+    }
 }
 
 void
@@ -891,7 +893,13 @@ PyErr_SetFromErrnoWithFilenameObjects(PyObject *exc, PyObject *filenameObject, P
 PyObject *
 PyErr_SetFromErrnoWithFilename(PyObject *exc, const char *filename)
 {
-    PyObject *name = filename ? PyUnicode_DecodeFSDefault(filename) : NULL;
+    PyObject *name = NULL;
+    if (filename) {
+        name = PyUnicode_DecodeFSDefault(filename);
+        if (name == NULL) {
+            return NULL;
+        }
+    }
     PyObject *result = PyErr_SetFromErrnoWithFilenameObjects(exc, name, NULL);
     Py_XDECREF(name);
     return result;
@@ -988,7 +996,13 @@ PyObject *PyErr_SetExcFromWindowsErrWithFilename(
     int ierr,
     const char *filename)
 {
-    PyObject *name = filename ? PyUnicode_DecodeFSDefault(filename) : NULL;
+    PyObject *name = NULL;
+    if (filename) {
+        name = PyUnicode_DecodeFSDefault(filename);
+        if (name == NULL) {
+            return NULL;
+        }
+    }
     PyObject *ret = PyErr_SetExcFromWindowsErrWithFilenameObjects(exc,
                                                                  ierr,
                                                                  name,
@@ -1012,7 +1026,13 @@ PyObject *PyErr_SetFromWindowsErrWithFilename(
     int ierr,
     const char *filename)
 {
-    PyObject *name = filename ? PyUnicode_DecodeFSDefault(filename) : NULL;
+    PyObject *name = NULL;
+    if (filename) {
+        name = PyUnicode_DecodeFSDefault(filename);
+        if (name == NULL) {
+            return NULL;
+        }
+    }
     PyObject *result = PyErr_SetExcFromWindowsErrWithFilenameObjects(
                                                   PyExc_OSError,
                                                   ierr, name, NULL);
@@ -1137,9 +1157,10 @@ _PyErr_FormatV(PyThreadState *tstate, PyObject *exception,
     _PyErr_Clear(tstate);
 
     string = PyUnicode_FromFormatV(format, vargs);
-
-    _PyErr_SetObject(tstate, exception, string);
-    Py_XDECREF(string);
+    if (string != NULL) {
+        _PyErr_SetObject(tstate, exception, string);
+        Py_DECREF(string);
+    }
     return NULL;
 }