Skip to content

Commit 145291d

Browse files
committed
gh-135075: Make PyObject_SetAttr() fail with NULL value and exception
Make PyObject_SetAttr() and PyObject_SetAttrString() fail if called with NULL value and an exception set.
1 parent 9c0cb5b commit 145291d

File tree

5 files changed

+87
-9
lines changed

5 files changed

+87
-9
lines changed

Doc/c-api/object.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ Object Protocol
197197
in favour of using :c:func:`PyObject_DelAttr`, but there are currently no
198198
plans to remove it.
199199
200+
The function must not be called with ``NULL`` *v* and an an exception set.
201+
200202
201203
.. c:function:: int PyObject_SetAttrString(PyObject *o, const char *attr_name, PyObject *v)
202204
@@ -207,6 +209,8 @@ Object Protocol
207209
If *v* is ``NULL``, the attribute is deleted, but this feature is
208210
deprecated in favour of using :c:func:`PyObject_DelAttrString`.
209211
212+
The function must not be called with ``NULL`` *v* and an an exception set.
213+
210214
The number of different attribute names passed to this function
211215
should be kept small, usually by using a statically allocated string
212216
as *attr_name*.

Lib/test/test_capi/test_object.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,5 +247,20 @@ def func(x):
247247

248248
func(object())
249249

250+
def test_object_setattr_null_exc(self):
251+
class Obj:
252+
pass
253+
obj = Obj()
254+
255+
obj.attr = 123
256+
with self.assertRaises(SystemError):
257+
_testcapi.object_setattr_null_exc(obj, 'attr')
258+
self.assertTrue(hasattr(obj, 'attr'))
259+
260+
with self.assertRaises(SystemError):
261+
_testcapi.object_setattrstring_null_exc(obj, 'attr')
262+
self.assertTrue(hasattr(obj, 'attr'))
263+
264+
250265
if __name__ == "__main__":
251266
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Make :c:func:`PyObject_SetAttr` and :c:func:`PyObject_SetAttrString` fail if
2+
called with ``NULL`` value and an exception set. Patch by Victor Stinner.

Modules/_testcapi/object.c

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,41 @@ is_uniquely_referenced(PyObject *self, PyObject *op)
485485
}
486486

487487

488+
static PyObject *
489+
object_setattr_null_exc(PyObject *self, PyObject *args)
490+
{
491+
PyObject *obj, *name;
492+
if (!PyArg_ParseTuple(args, "OO", &obj, &name)) {
493+
return NULL;
494+
}
495+
496+
PyErr_SetString(PyExc_ValueError, "error");
497+
if (PyObject_SetAttr(obj, name, NULL) < 0) {
498+
return NULL;
499+
}
500+
assert(PyErr_Occurred());
501+
return NULL;
502+
}
503+
504+
505+
static PyObject *
506+
object_setattrstring_null_exc(PyObject *self, PyObject *args)
507+
{
508+
PyObject *obj;
509+
const char *name;
510+
if (!PyArg_ParseTuple(args, "Os", &obj, &name)) {
511+
return NULL;
512+
}
513+
514+
PyErr_SetString(PyExc_ValueError, "error");
515+
if (PyObject_SetAttrString(obj, name, NULL) < 0) {
516+
return NULL;
517+
}
518+
assert(PyErr_Occurred());
519+
return NULL;
520+
}
521+
522+
488523
static PyMethodDef test_methods[] = {
489524
{"call_pyobject_print", call_pyobject_print, METH_VARARGS},
490525
{"pyobject_print_null", pyobject_print_null, METH_VARARGS},
@@ -511,6 +546,8 @@ static PyMethodDef test_methods[] = {
511546
{"test_py_is_funcs", test_py_is_funcs, METH_NOARGS},
512547
{"clear_managed_dict", clear_managed_dict, METH_O, NULL},
513548
{"is_uniquely_referenced", is_uniquely_referenced, METH_O},
549+
{"object_setattr_null_exc", object_setattr_null_exc, METH_VARARGS},
550+
{"object_setattrstring_null_exc", object_setattrstring_null_exc, METH_VARARGS},
514551
{NULL},
515552
};
516553

Objects/object.c

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,16 +1213,27 @@ PyObject_HasAttrString(PyObject *obj, const char *name)
12131213
int
12141214
PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w)
12151215
{
1216-
PyObject *s;
1217-
int res;
1216+
if (Py_TYPE(v)->tp_setattr != NULL) {
1217+
PyThreadState *tstate = _PyThreadState_GET();
1218+
if (w == NULL && _PyErr_Occurred(tstate)) {
1219+
PyObject *exc = _PyErr_GetRaisedException(tstate);
1220+
_PyErr_SetString(tstate, PyExc_SystemError,
1221+
"PyObject_SetAttrString() must not be called with NULL value "
1222+
"and an exception set");
1223+
_PyErr_ChainExceptions1Tstate(tstate, exc);
1224+
return -1;
1225+
}
12181226

1219-
if (Py_TYPE(v)->tp_setattr != NULL)
12201227
return (*Py_TYPE(v)->tp_setattr)(v, (char*)name, w);
1221-
s = PyUnicode_InternFromString(name);
1222-
if (s == NULL)
1228+
}
1229+
1230+
PyObject *s = PyUnicode_InternFromString(name);
1231+
if (s == NULL) {
12231232
return -1;
1224-
res = PyObject_SetAttr(v, s, w);
1225-
Py_XDECREF(s);
1233+
}
1234+
1235+
int res = PyObject_SetAttr(v, s, w);
1236+
Py_DECREF(s);
12261237
return res;
12271238
}
12281239

@@ -1440,6 +1451,16 @@ PyObject_HasAttr(PyObject *obj, PyObject *name)
14401451
int
14411452
PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value)
14421453
{
1454+
PyThreadState *tstate = _PyThreadState_GET();
1455+
if (value == NULL && _PyErr_Occurred(tstate)) {
1456+
PyObject *exc = _PyErr_GetRaisedException(tstate);
1457+
_PyErr_SetString(tstate, PyExc_SystemError,
1458+
"PyObject_SetAttr() must not be called with NULL value "
1459+
"and an exception set");
1460+
_PyErr_ChainExceptions1Tstate(tstate, exc);
1461+
return -1;
1462+
}
1463+
14431464
PyTypeObject *tp = Py_TYPE(v);
14441465
int err;
14451466

@@ -1451,8 +1472,7 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value)
14511472
}
14521473
Py_INCREF(name);
14531474

1454-
PyInterpreterState *interp = _PyInterpreterState_GET();
1455-
_PyUnicode_InternMortal(interp, &name);
1475+
_PyUnicode_InternMortal(tstate->interp, &name);
14561476
if (tp->tp_setattro != NULL) {
14571477
err = (*tp->tp_setattro)(v, name, value);
14581478
Py_DECREF(name);

0 commit comments

Comments
 (0)