Subclasses of string can no longer be interned. The semantics of
interning were not clear here -- a subclass could be mutable, for
example -- and had bugs. Explicitly interning a subclass of string
via intern() will raise a TypeError. Internal operations that attempt
to intern a string subclass will have no effect.
Added a few tests to test_builtin that includes the old buggy code and
verifies that calls like PyObject_SetAttr() don't fail. Perhaps these
tests should have gone in test_string.
diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py
index bc5afdc..8e3a925 100644
--- a/Lib/test/test_builtin.py
+++ b/Lib/test/test_builtin.py
@@ -608,6 +608,23 @@
s2 = s.swapcase().swapcase()
self.assert_(intern(s2) is s)
+ # Subclasses of string can't be interned, because they
+ # provide too much opportunity for insane things to happen.
+ # We don't want them in the interned dict and if they aren't
+ # actually interned, we don't want to create the appearance
+ # that they are by allowing intern() to succeeed.
+ class S(str):
+ def __hash__(self):
+ return 123
+
+ self.assertRaises(TypeError, intern, S("abc"))
+
+ # It's still safe to pass these strings to routines that
+ # call intern internally, e.g. PyObject_SetAttr().
+ s = S("abc")
+ setattr(s, s, s)
+ self.assertEqual(getattr(s, s), s)
+
def test_iter(self):
self.assertRaises(TypeError, iter)
self.assertRaises(TypeError, iter, 42, 42)
diff --git a/Misc/NEWS b/Misc/NEWS
index 9f80274..7550076 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,12 @@
Core and builtins
-----------------
+Subclasses of string can no longer be interned. The semantics of
+interning were not clear here -- a subclass could be mutable, for
+example -- and had bugs. Explicitly interning a subclass of string
+via intern() will raise a TypeError. Internal operations that attempt
+to intern a string subclass will have no effect.
+
Extension modules
-----------------
diff --git a/Objects/stringobject.c b/Objects/stringobject.c
index 29562a9..f29929b 100644
--- a/Objects/stringobject.c
+++ b/Objects/stringobject.c
@@ -4313,6 +4313,10 @@
PyObject *t;
if (s == NULL || !PyString_Check(s))
Py_FatalError("PyString_InternInPlace: strings only please!");
+ /* If it's a string subclass, we don't really know what putting
+ it in the interned dict might do. */
+ if (!PyString_CheckExact(s))
+ return;
if (PyString_CHECK_INTERNED(s))
return;
if (interned == NULL) {
@@ -4322,36 +4326,22 @@
return;
}
}
- if ((t = PyDict_GetItem(interned, (PyObject *)s)) != NULL) {
+ t = PyDict_GetItem(interned, (PyObject *)s);
+ if (t) {
Py_INCREF(t);
Py_DECREF(*p);
*p = t;
return;
}
- /* Ensure that only true string objects appear in the intern dict */
- if (!PyString_CheckExact(s)) {
- t = PyString_FromStringAndSize(PyString_AS_STRING(s),
- PyString_GET_SIZE(s));
- if (t == NULL) {
- PyErr_Clear();
- return;
- }
- } else {
- t = (PyObject*) s;
- Py_INCREF(t);
- }
- if (PyDict_SetItem(interned, t, t) == 0) {
- /* The two references in interned are not counted by
- refcnt. The string deallocator will take care of this */
- ((PyObject *)t)->ob_refcnt-=2;
- PyString_CHECK_INTERNED(t) = SSTATE_INTERNED_MORTAL;
- Py_DECREF(*p);
- *p = t;
+ if (PyDict_SetItem(interned, s, s) < 0) {
+ PyErr_Clear();
return;
}
- Py_DECREF(t);
- PyErr_Clear();
+ /* The two references in interned are not counted by refcnt.
+ The string deallocator will take care of this */
+ (*p)->ob_refcnt -= 2;
+ PyString_CHECK_INTERNED(s) = SSTATE_INTERNED_MORTAL;
}
void
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index 8ff733c..04fcf59 100644
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -1035,6 +1035,11 @@
PyObject *s;
if (!PyArg_ParseTuple(args, "S:intern", &s))
return NULL;
+ if (!PyString_CheckExact(s)) {
+ PyErr_SetString(PyExc_TypeError,
+ "can't intern subclass of string");
+ return NULL;
+ }
Py_INCREF(s);
PyString_InternInPlace(&s);
return s;