diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index 0fd159f1eb87f8..21fa1491b33d86 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -197,6 +197,13 @@ Object Protocol in favour of using :c:func:`PyObject_DelAttr`, but there are currently no plans to remove it. + The function must not be called with ``NULL`` *v* and an an exception set. + This case can arise from forgetting ``NULL`` checks and would delete the + attribute. + + .. versionchanged:: next + Must not be called with NULL value if an exception is set. + .. c:function:: int PyObject_SetAttrString(PyObject *o, const char *attr_name, PyObject *v) @@ -207,6 +214,10 @@ Object Protocol If *v* is ``NULL``, the attribute is deleted, but this feature is deprecated in favour of using :c:func:`PyObject_DelAttrString`. + The function must not be called with ``NULL`` *v* and an an exception set. + This case can arise from forgetting ``NULL`` checks and would delete the + attribute. + The number of different attribute names passed to this function should be kept small, usually by using a statically allocated string as *attr_name*. @@ -215,6 +226,10 @@ Object Protocol For more details, see :c:func:`PyUnicode_InternFromString`, which may be used internally to create a key object. + .. versionchanged:: next + Must not be called with NULL value if an exception is set. + + .. c:function:: int PyObject_GenericSetAttr(PyObject *o, PyObject *name, PyObject *value) Generic attribute setter and deleter function that is meant diff --git a/Lib/test/test_capi/test_abstract.py b/Lib/test/test_capi/test_abstract.py index 7d548ae87c0fa6..3a2ed9f5db82f0 100644 --- a/Lib/test/test_capi/test_abstract.py +++ b/Lib/test/test_capi/test_abstract.py @@ -1077,6 +1077,31 @@ def test_iter_nextitem(self): with self.assertRaisesRegex(TypeError, regex): PyIter_NextItem(10) + def test_object_setattr_null_exc(self): + class Obj: + pass + obj = Obj() + obj.attr = 123 + + exc = ValueError("error") + with self.assertRaises(SystemError) as cm: + _testcapi.object_setattr_null_exc(obj, 'attr', exc) + self.assertIs(cm.exception.__context__, exc) + self.assertIsNone(cm.exception.__cause__) + self.assertHasAttr(obj, 'attr') + + with self.assertRaises(SystemError) as cm: + _testcapi.object_setattrstring_null_exc(obj, 'attr', exc) + self.assertIs(cm.exception.__context__, exc) + self.assertIsNone(cm.exception.__cause__) + self.assertHasAttr(obj, 'attr') + + with self.assertRaises(SystemError) as cm: + # undecodable name + _testcapi.object_setattrstring_null_exc(obj, b'\xff', exc) + self.assertIs(cm.exception.__context__, exc) + self.assertIsNone(cm.exception.__cause__) + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_capi/test_object.py b/Lib/test/test_capi/test_object.py index d4056727d07fbf..1be5c08bdda3d5 100644 --- a/Lib/test/test_capi/test_object.py +++ b/Lib/test/test_capi/test_object.py @@ -247,5 +247,6 @@ def func(x): func(object()) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/C_API/2025-07-01-16-22-39.gh-issue-135075.angu3J.rst b/Misc/NEWS.d/next/C_API/2025-07-01-16-22-39.gh-issue-135075.angu3J.rst new file mode 100644 index 00000000000000..88e0fa65f45dd0 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2025-07-01-16-22-39.gh-issue-135075.angu3J.rst @@ -0,0 +1,2 @@ +Make :c:func:`PyObject_SetAttr` and :c:func:`PyObject_SetAttrString` fail if +called with ``NULL`` value and an exception set. Patch by Victor Stinner. diff --git a/Modules/_testcapi/abstract.c b/Modules/_testcapi/abstract.c index d4045afd515909..c1f769456accef 100644 --- a/Modules/_testcapi/abstract.c +++ b/Modules/_testcapi/abstract.c @@ -178,6 +178,42 @@ sequence_fast_get_item(PyObject *self, PyObject *args) } +static PyObject * +object_setattr_null_exc(PyObject *self, PyObject *args) +{ + PyObject *obj, *name, *exc; + if (!PyArg_ParseTuple(args, "OOO", &obj, &name, &exc)) { + return NULL; + } + + PyErr_SetObject((PyObject*)Py_TYPE(exc), exc); + if (PyObject_SetAttr(obj, name, NULL) < 0) { + return NULL; + } + assert(PyErr_Occurred()); + return NULL; +} + + +static PyObject * +object_setattrstring_null_exc(PyObject *self, PyObject *args) +{ + PyObject *obj, *exc; + const char *name; + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "Oz#O", &obj, &name, &size, &exc)) { + return NULL; + } + + PyErr_SetObject((PyObject*)Py_TYPE(exc), exc); + if (PyObject_SetAttrString(obj, name, NULL) < 0) { + return NULL; + } + assert(PyErr_Occurred()); + return NULL; +} + + static PyMethodDef test_methods[] = { {"object_getoptionalattr", object_getoptionalattr, METH_VARARGS}, {"object_getoptionalattrstring", object_getoptionalattrstring, METH_VARARGS}, @@ -191,6 +227,8 @@ static PyMethodDef test_methods[] = { {"sequence_fast_get_size", sequence_fast_get_size, METH_O}, {"sequence_fast_get_item", sequence_fast_get_item, METH_VARARGS}, + {"object_setattr_null_exc", object_setattr_null_exc, METH_VARARGS}, + {"object_setattrstring_null_exc", object_setattrstring_null_exc, METH_VARARGS}, {NULL}, }; diff --git a/Objects/object.c b/Objects/object.c index 1223983753ac46..3ed7d55593dffa 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1213,16 +1213,27 @@ PyObject_HasAttrString(PyObject *obj, const char *name) int PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w) { - PyObject *s; - int res; + PyThreadState *tstate = _PyThreadState_GET(); + if (w == NULL && _PyErr_Occurred(tstate)) { + PyObject *exc = _PyErr_GetRaisedException(tstate); + _PyErr_SetString(tstate, PyExc_SystemError, + "PyObject_SetAttrString() must not be called with NULL value " + "and an exception set"); + _PyErr_ChainExceptions1Tstate(tstate, exc); + return -1; + } - if (Py_TYPE(v)->tp_setattr != NULL) + if (Py_TYPE(v)->tp_setattr != NULL) { return (*Py_TYPE(v)->tp_setattr)(v, (char*)name, w); - s = PyUnicode_InternFromString(name); - if (s == NULL) + } + + PyObject *s = PyUnicode_InternFromString(name); + if (s == NULL) { return -1; - res = PyObject_SetAttr(v, s, w); - Py_XDECREF(s); + } + + int res = PyObject_SetAttr(v, s, w); + Py_DECREF(s); return res; } @@ -1440,6 +1451,16 @@ PyObject_HasAttr(PyObject *obj, PyObject *name) int PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) { + PyThreadState *tstate = _PyThreadState_GET(); + if (value == NULL && _PyErr_Occurred(tstate)) { + PyObject *exc = _PyErr_GetRaisedException(tstate); + _PyErr_SetString(tstate, PyExc_SystemError, + "PyObject_SetAttr() must not be called with NULL value " + "and an exception set"); + _PyErr_ChainExceptions1Tstate(tstate, exc); + return -1; + } + PyTypeObject *tp = Py_TYPE(v); int err; @@ -1451,8 +1472,7 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) } Py_INCREF(name); - PyInterpreterState *interp = _PyInterpreterState_GET(); - _PyUnicode_InternMortal(interp, &name); + _PyUnicode_InternMortal(tstate->interp, &name); if (tp->tp_setattro != NULL) { err = (*tp->tp_setattro)(v, name, value); Py_DECREF(name);