From e60f4a1de016b35eeb7e34d655cbe3e50362583f Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 18 May 2017 20:46:06 +0300 Subject: [PATCH 1/2] bpo-25794: Fix type.__setattr__() for non-interned attribute names. Based on patch by Eryk Sun. --- Lib/test/test_class.py | 27 +++++++++++++++++++++++++++ Misc/NEWS | 3 +++ Objects/typeobject.c | 40 ++++++++++++++++++++++++++++++++++------ 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_class.py b/Lib/test/test_class.py index 4d554a397b4a51..ecc01f277954d5 100644 --- a/Lib/test/test_class.py +++ b/Lib/test/test_class.py @@ -568,5 +568,32 @@ class B(A): a = A(hash(A.f)^(-1)) hash(a.f) + def testSetattrWrapperNameIntern(self): + # Issue #25794: __setattr__ should intern the attribute name + class A: + pass + + def add(self, other): + return 'summa' + + name = str(b'__add__', 'ascii') # shouldn't be optimized + self.assertIsNot(name, '__add__') # not interned + type.__setattr__(A, name, add) + self.assertEqual(A() + 1, 'summa') + + name2 = str(b'__add__', 'ascii') + self.assertIsNot(name2, '__add__') + self.assertIsNot(name2, name) + type.__delattr__(A, name2) + with self.assertRaises(TypeError): + A() + 1 + + def testSetattrNonStringName(self): + class A: + pass + + with self.assertRaises(TypeError): + type.__setattr__(A, b'x', None) + if __name__ == '__main__': unittest.main() diff --git a/Misc/NEWS b/Misc/NEWS index c6aed7f48c8dcb..9aa982ef62c41b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ What's New in Python 3.7.0 alpha 1? Core and Builtins ----------------- +- bpo-25794: Fixed type.__setattr__() and type.__delattr__() for + non-interned attribute names. Based on patch by Eryk Sun. + - bpo-30039: If a KeyboardInterrupt happens when the interpreter is in the middle of resuming a chain of nested 'yield from' or 'await' calls, it's now correctly delivered to the innermost frame. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 121d66d8bcabcc..1667d0eb960672 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3130,7 +3130,7 @@ type_getattro(PyTypeObject *type, PyObject *name) static int type_setattro(PyTypeObject *type, PyObject *name, PyObject *value) { - int res; + int res, copied = 0; if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) { PyErr_Format( PyExc_TypeError, @@ -3138,10 +3138,35 @@ type_setattro(PyTypeObject *type, PyObject *name, PyObject *value) type->tp_name); return -1; } - if (_PyObject_GenericSetAttrWithDict((PyObject *)type, name, value, NULL) < 0) - return -1; - res = update_slot(type, name); - assert(_PyType_CheckConsistency(type)); + if (PyUnicode_Check(name)) { + if (PyUnicode_CheckExact(name)) { + if (PyUnicode_READY(name) == -1) + return -1; + Py_INCREF(name); + } + else { + name = _PyUnicode_Copy(name); + if (name == NULL) + return -1; + } + copied = 1; + assert(PyUnicode_CheckExact(name)); + PyUnicode_InternInPlace(&name); + if (!PyUnicode_CHECK_INTERNED(name)) { + PyErr_SetString(PyExc_MemoryError, + "Out of memory interning an attribute name"); + if (copied) + Py_DECREF(name); + return -1; + } + } + res = _PyObject_GenericSetAttrWithDict((PyObject *)type, name, value, NULL); + if (res == 0) { + res = update_slot(type, name); + assert(_PyType_CheckConsistency(type)); + } + if (copied) + Py_DECREF(name); return res; } @@ -7065,7 +7090,7 @@ init_slotdefs(void) /* Slots must be ordered by their offset in the PyHeapTypeObject. */ assert(!p[1].name || p->offset <= p[1].offset); p->name_strobj = PyUnicode_InternFromString(p->name); - if (!p->name_strobj) + if (!p->name_strobj || !PyUnicode_CHECK_INTERNED(p->name_strobj)) Py_FatalError("Out of memory interning slotdef names"); } slotdefs_initialized = 1; @@ -7090,6 +7115,9 @@ update_slot(PyTypeObject *type, PyObject *name) slotdef **pp; int offset; + assert(PyUnicode_CheckExact(name)); + assert(PyUnicode_CHECK_INTERNED(name)); + /* Clear the VALID_VERSION flag of 'type' and all its subclasses. This could possibly be unified with the update_subclasses() recursion below, but carefully: From 10386288b5794a70dac216b68ee29a0c1d666649 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 19 May 2017 09:27:04 +0300 Subject: [PATCH 2/2] Clean up. --- Objects/typeobject.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 1667d0eb960672..224fe6199fca0d 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3130,7 +3130,7 @@ type_getattro(PyTypeObject *type, PyObject *name) static int type_setattro(PyTypeObject *type, PyObject *name, PyObject *value) { - int res, copied = 0; + int res; if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) { PyErr_Format( PyExc_TypeError, @@ -3149,24 +3149,24 @@ type_setattro(PyTypeObject *type, PyObject *name, PyObject *value) if (name == NULL) return -1; } - copied = 1; - assert(PyUnicode_CheckExact(name)); PyUnicode_InternInPlace(&name); if (!PyUnicode_CHECK_INTERNED(name)) { PyErr_SetString(PyExc_MemoryError, "Out of memory interning an attribute name"); - if (copied) - Py_DECREF(name); + Py_DECREF(name); return -1; } } + else { + /* Will fail in _PyObject_GenericSetAttrWithDict. */ + Py_INCREF(name); + } res = _PyObject_GenericSetAttrWithDict((PyObject *)type, name, value, NULL); if (res == 0) { res = update_slot(type, name); assert(_PyType_CheckConsistency(type)); } - if (copied) - Py_DECREF(name); + Py_DECREF(name); return res; } @@ -7128,7 +7128,6 @@ update_slot(PyTypeObject *type, PyObject *name) init_slotdefs(); pp = ptrs; for (p = slotdefs; p->name; p++) { - /* XXX assume name is interned! */ if (p->name_strobj == name) *pp++ = p; }