From 633ea217a85f6b6ba5bdbc73094254d5811b3485 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 19 Aug 2023 14:51:03 +0300 Subject: 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 | 81 ++++++++++++++++++++++ .../2023-08-13-12-33-00.gh-issue-107915.jQ0wOi.rst | 4 ++ Modules/_testcapi/clinic/exceptions.c.h | 64 ++++++++++++++++- Modules/_testcapi/exceptions.c | 41 +++++++++++ Python/errors.c | 37 +++++++--- 5 files changed, 218 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-08-13-12-33-00.gh-issue-107915.jQ0wOi.rst diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 118b575..b96cc79 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/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=') + 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): diff --git a/Misc/NEWS.d/next/C API/2023-08-13-12-33-00.gh-issue-107915.jQ0wOi.rst b/Misc/NEWS.d/next/C API/2023-08-13-12-33-00.gh-issue-107915.jQ0wOi.rst new file mode 100644 index 0000000..58ee3f1 --- /dev/null +++ b/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. diff --git a/Modules/_testcapi/clinic/exceptions.c.h b/Modules/_testcapi/clinic/exceptions.c.h index 01730ff..16954a5 100644 --- a/Modules/_testcapi/clinic/exceptions.c.h +++ b/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]*/ diff --git a/Modules/_testcapi/exceptions.c b/Modules/_testcapi/exceptions.c index a627bf1..d7edf8c 100644 --- a/Modules/_testcapi/exceptions.c +++ b/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]*/ @@ -130,6 +132,43 @@ _testcapi_exc_set_object_fetch_impl(PyObject *module, PyObject *exc, } /*[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 num_args: int @@ -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 diff --git a/Python/errors.c b/Python/errors.c index 916958c..d9086c5 100644 --- a/Python/errors.c +++ b/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; } -- cgit v0.12