Skip to content

bpo-25794: Fix type.__setattr__() for non-interned attribute names. #1652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions Lib/test/test_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
3 changes: 3 additions & 0 deletions Misc/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
39 changes: 33 additions & 6 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
PyUnicode_InternInPlace(&name);
if (!PyUnicode_CHECK_INTERNED(name)) {
PyErr_SetString(PyExc_MemoryError,
"Out of memory interning an attribute 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));
}
Py_DECREF(name);
return res;
}

Expand Down Expand Up @@ -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;
Expand All @@ -7090,6 +7115,9 @@ update_slot(PyTypeObject *type, PyObject *name)
slotdef **pp;
int offset;

assert(PyUnicode_CheckExact(name));
assert(PyUnicode_CHECK_INTERNED(name));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can now remove the following comment in line 7131:

/* XXX assume name is interned! */


/* Clear the VALID_VERSION flag of 'type' and all its
subclasses. This could possibly be unified with the
update_subclasses() recursion below, but carefully:
Expand All @@ -7100,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;
}
Expand Down