diff --git a/Lib/test/test_xml_etree_c.py b/Lib/test/test_xml_etree_c.py index e122b3db243..71a755c0a64 100644 --- a/Lib/test/test_xml_etree_c.py +++ b/Lib/test/test_xml_etree_c.py @@ -53,6 +53,31 @@ class MiscTests(unittest.TestCase): del element.attrib self.assertEqual(element.attrib, {'A': 'B', 'C': 'D'}) + def test_bpo_31728(self): + # A crash shouldn't happen in case garbage collection triggers a call + # to clear() or a reading of text or tail, while a setter or clear() + # is already running. + elem = cET.Element('elem') + class X: + def __del__(self): + elem.text + elem.tail + elem.clear() + + elem.text = X() + elem.clear() # shouldn't crash + + elem.tail = X() + elem.clear() # shouldn't crash + + elem.text = X() + elem.text = X() # shouldn't crash + elem.clear() + + elem.tail = X() + elem.tail = X() # shouldn't crash + elem.clear() + def test_main(): from test import test_xml_etree, test_xml_etree_c diff --git a/Misc/NEWS.d/next/Library/2017-10-11-13-05-19.bpo-31728.XrVMME.rst b/Misc/NEWS.d/next/Library/2017-10-11-13-05-19.bpo-31728.XrVMME.rst new file mode 100644 index 00000000000..b317d9f210b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-10-11-13-05-19.bpo-31728.XrVMME.rst @@ -0,0 +1,2 @@ +Prevent crashes in `_elementtree` due to unsafe cleanup of `Element.text` +and `Element.tail`. Patch by Oren Milman. diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index b52402f3811..574559c6313 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -131,6 +131,15 @@ static PyObject* elementpath_obj; /* helpers */ +/* Py_SETREF for a PyObject* that uses a join flag. */ +Py_LOCAL_INLINE(void) +_set_joined_ptr(PyObject **p, PyObject *new_joined_ptr) +{ + PyObject *tmp = JOIN_OBJ(*p); + *p = new_joined_ptr; + Py_DECREF(tmp); +} + LOCAL(PyObject*) deepcopy(PyObject* object, PyObject* memo) { @@ -585,12 +594,10 @@ element_clear(ElementObject* self, PyObject* args) } Py_INCREF(Py_None); - Py_DECREF(JOIN_OBJ(self->text)); - self->text = Py_None; + _set_joined_ptr(&self->text, Py_None); Py_INCREF(Py_None); - Py_DECREF(JOIN_OBJ(self->tail)); - self->tail = Py_None; + _set_joined_ptr(&self->tail, Py_None); Py_RETURN_NONE; } @@ -610,13 +617,11 @@ element_copy(ElementObject* self, PyObject* args) if (!element) return NULL; - Py_DECREF(JOIN_OBJ(element->text)); - element->text = self->text; - Py_INCREF(JOIN_OBJ(element->text)); + Py_INCREF(JOIN_OBJ(self->text)); + _set_joined_ptr(&element->text, self->text); - Py_DECREF(JOIN_OBJ(element->tail)); - element->tail = self->tail; - Py_INCREF(JOIN_OBJ(element->tail)); + Py_INCREF(JOIN_OBJ(self->tail)); + _set_joined_ptr(&element->tail, self->tail); if (self->extra) { @@ -678,14 +683,12 @@ element_deepcopy(ElementObject* self, PyObject* args) text = deepcopy(JOIN_OBJ(self->text), memo); if (!text) goto error; - Py_DECREF(element->text); - element->text = JOIN_SET(text, JOIN_GET(self->text)); + _set_joined_ptr(&element->text, JOIN_SET(text, JOIN_GET(self->text))); tail = deepcopy(JOIN_OBJ(self->tail), memo); if (!tail) goto error; - Py_DECREF(element->tail); - element->tail = JOIN_SET(tail, JOIN_GET(self->tail)); + _set_joined_ptr(&element->tail, JOIN_SET(tail, JOIN_GET(self->tail))); if (self->extra) { @@ -1624,13 +1627,11 @@ element_setattr(ElementObject* self, const char* name, PyObject* value) Py_INCREF(value); Py_SETREF(self->tag, value); } else if (strcmp(name, "text") == 0) { - Py_DECREF(JOIN_OBJ(self->text)); - self->text = value; - Py_INCREF(self->text); + Py_INCREF(value); + _set_joined_ptr(&self->text, value); } else if (strcmp(name, "tail") == 0) { - Py_DECREF(JOIN_OBJ(self->tail)); - self->tail = value; - Py_INCREF(self->tail); + Py_INCREF(value); + _set_joined_ptr(&self->tail, value); } else if (strcmp(name, "attrib") == 0) { if (!self->extra) element_new_extra(self, NULL);