From: Serhiy Storchaka Date: Sun, 2 Apr 2017 17:37:03 +0000 (+0300) Subject: bpo-27863: Fixed multiple crashes in ElementTree. (#765) (#903) (#963) X-Git-Tag: v2.7.14rc1~226 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9c2c42c221d7996070c0c0a2a114ab42fe3ddb9d;p=python bpo-27863: Fixed multiple crashes in ElementTree. (#765) (#903) (#963) (cherry picked from commit 576def096ec7b64814e038f03290031f172886c3) (cherry picked from commit a6b4e1902250d6f28ca6d083ce1c8d7e9b91974b) --- diff --git a/Lib/test/test_xml_etree.py b/Lib/test/test_xml_etree.py index ef6988564f..6f8c0e2beb 100644 --- a/Lib/test/test_xml_etree.py +++ b/Lib/test/test_xml_etree.py @@ -1594,6 +1594,83 @@ class BadElementTest(unittest.TestCase): with self.assertRaises(RuntimeError): repr(e) # Should not crash + def test_element_get_text(self): + # Issue #27863 + class X(str): + def __del__(self): + try: + elem.text + except NameError: + pass + + b = ET.TreeBuilder() + b.start('tag', {}) + b.data('ABCD') + b.data(X('EFGH')) + b.data('IJKL') + b.end('tag') + + elem = b.close() + self.assertEqual(elem.text, 'ABCDEFGHIJKL') + + def test_element_get_tail(self): + # Issue #27863 + class X(str): + def __del__(self): + try: + elem[0].tail + except NameError: + pass + + b = ET.TreeBuilder() + b.start('root', {}) + b.start('tag', {}) + b.end('tag') + b.data('ABCD') + b.data(X('EFGH')) + b.data('IJKL') + b.end('root') + + elem = b.close() + self.assertEqual(elem[0].tail, 'ABCDEFGHIJKL') + + def test_element_iter(self): + # Issue #27863 + e = ET.Element('tag') + e.extend([None]) # non-Element + + it = e.iter() + self.assertIs(next(it), e) + self.assertRaises((AttributeError, TypeError), list, it) + + def test_subscr(self): + # Issue #27863 + class X: + def __index__(self): + del e[:] + return 1 + + e = ET.Element('elem') + e.append(ET.Element('child')) + e[:X()] # shouldn't crash + + e.append(ET.Element('child')) + e[0:10:X()] # shouldn't crash + + def test_ass_subscr(self): + # Issue #27863 + class X: + def __index__(self): + e[:] = [] + return 1 + + e = ET.Element('elem') + for _ in range(10): + e.insert(0, ET.Element('child')) + + e[0:10:X()] = [] # shouldn't crash + + class MutatingElementPath(str): def __new__(cls, elem, *args): self = str.__new__(cls, *args) diff --git a/Misc/NEWS b/Misc/NEWS index 140335287c..5a0335b118 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -42,6 +42,9 @@ Extension Modules Library ------- +- bpo-27863: Fixed multiple crashes in ElementTree caused by race conditions + and wrong types. + - bpo-29942: Fix a crash in itertools.chain.from_iterable when encountering long runs of empty iterables. diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index 94dc5b7ca3..d7f75c902a 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -164,8 +164,7 @@ deepcopy(PyObject* object, PyObject* memo) LOCAL(PyObject*) list_join(PyObject* list) { - /* join list elements (destroying the list in the process) */ - + /* join list elements */ PyObject* joiner; PyObject* function; PyObject* args; @@ -173,12 +172,10 @@ list_join(PyObject* list) switch (PyList_GET_SIZE(list)) { case 0: - Py_DECREF(list); return PyString_FromString(""); case 1: result = PyList_GET_ITEM(list, 0); Py_INCREF(result); - Py_DECREF(list); return result; } @@ -196,9 +193,13 @@ list_join(PyObject* list) } args = PyTuple_New(1); - if (!args) + if (!args) { + Py_DECREF(function); + Py_DECREF(joiner); return NULL; + } + Py_INCREF(list); PyTuple_SET_ITEM(args, 0, list); result = PyObject_CallObject(function, args); @@ -435,15 +436,17 @@ element_get_text(ElementObject* self) { /* return borrowed reference to text attribute */ - PyObject* res = self->text; + PyObject *res = self->text; if (JOIN_GET(res)) { res = JOIN_OBJ(res); if (PyList_CheckExact(res)) { - res = list_join(res); - if (!res) + PyObject *tmp = list_join(res); + if (!tmp) return NULL; - self->text = res; + self->text = tmp; + Py_DECREF(res); + res = tmp; } } @@ -455,15 +458,17 @@ element_get_tail(ElementObject* self) { /* return borrowed reference to text attribute */ - PyObject* res = self->tail; + PyObject *res = self->tail; if (JOIN_GET(res)) { res = JOIN_OBJ(res); if (PyList_CheckExact(res)) { - res = list_join(res); - if (!res) + PyObject *tmp = list_join(res); + if (!tmp) return NULL; - self->tail = res; + self->tail = tmp; + Py_DECREF(res); + res = tmp; } } @@ -1730,6 +1735,37 @@ treebuilder_dealloc(TreeBuilderObject* self) PyObject_Del(self); } +/* -------------------------------------------------------------------- */ +/* helpers for handling of arbitrary element-like objects */ + +static void +treebuilder_set_element_text_or_tail(PyObject **data, PyObject **dest) +{ + PyObject *tmp = JOIN_OBJ(*dest); + *dest = JOIN_SET(*data, PyList_CheckExact(*data)); + *data = NULL; + Py_DECREF(tmp); +} + +LOCAL(void) +treebuilder_flush_data(TreeBuilderObject* self) +{ + ElementObject *element = self->last; + + if (self->data) { + if (self->this == element) { + treebuilder_set_element_text_or_tail( + &self->data, + &element->text); + } + else { + treebuilder_set_element_text_or_tail( + &self->data, + &element->tail); + } + } +} + LOCAL(int) treebuilder_append_event(TreeBuilderObject *self, PyObject *action, PyObject *node) @@ -1764,20 +1800,7 @@ treebuilder_handle_start(TreeBuilderObject* self, PyObject* tag, PyObject* node; PyObject* this; - if (self->data) { - if (self->this == self->last) { - Py_DECREF(JOIN_OBJ(self->last->text)); - self->last->text = JOIN_SET( - self->data, PyList_CheckExact(self->data) - ); - } else { - Py_DECREF(JOIN_OBJ(self->last->tail)); - self->last->tail = JOIN_SET( - self->data, PyList_CheckExact(self->data) - ); - } - self->data = NULL; - } + treebuilder_flush_data(self); node = element_new(tag, attrib); if (!node) @@ -1867,20 +1890,7 @@ treebuilder_handle_end(TreeBuilderObject* self, PyObject* tag) { ElementObject *item; - if (self->data) { - if (self->this == self->last) { - Py_DECREF(JOIN_OBJ(self->last->text)); - self->last->text = JOIN_SET( - self->data, PyList_CheckExact(self->data) - ); - } else { - Py_DECREF(JOIN_OBJ(self->last->tail)); - self->last->tail = JOIN_SET( - self->data, PyList_CheckExact(self->data) - ); - } - self->data = NULL; - } + treebuilder_flush_data(self); if (self->index == 0) { PyErr_SetString(