Skip to content

[2.7] bpo-31455: avoid calling "PyObject_GetAttrString()" with a live exception set #3992

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 1 commit into from
Mar 24, 2018
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
33 changes: 33 additions & 0 deletions Lib/test/test_xml_etree.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,13 @@ def wrapper(*args):
return test(*args)
return wrapper

def cet_only(test):
def wrapper(*args):
if ET is pyET:
raise unittest.SkipTest('only for the C version')
return test(*args)
return wrapper

# --------------------------------------------------------------------
# element tree tests

Expand Down Expand Up @@ -2229,6 +2236,32 @@ def close(self):
('html', '-//W3C//DTD XHTML 1.0 Transitional//EN',
'http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd'))

@cet_only # PyET does not look up the attributes in XMLParser().__init__()
def test_builder_lookup_errors(self):
class RaisingBuilder(object):
def __init__(self, raise_in=None, what=ValueError):
self.raise_in = raise_in
self.what = what

def __getattr__(self, name):
if name == self.raise_in:
raise self.what(self.raise_in)
def handle(*args):
pass
return handle

ET.XMLParser(target=RaisingBuilder())
# cET also checks for 'close' and 'doctype', PyET does it only at need
for event in ('start', 'data', 'end', 'comment', 'pi'):
with self.assertRaises(ValueError):
ET.XMLParser(target=RaisingBuilder(event))

ET.XMLParser(target=RaisingBuilder(what=AttributeError))
for event in ('start', 'data', 'end', 'comment', 'pi'):
parser = ET.XMLParser(target=RaisingBuilder(event, what=AttributeError))
parser.feed(self.sample1)
self.assertIsNone(parser.close())


class XMLParserTest(unittest.TestCase):
sample1 = b'<file><line>22</line></file>'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The C accelerator module of ElementTree ignored exceptions raised when
looking up TreeBuilder target methods in XMLParser().
35 changes: 33 additions & 2 deletions Modules/_elementtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -2510,6 +2510,18 @@ expat_unknown_encoding_handler(XMLParserObject *self, const XML_Char *name,
/* -------------------------------------------------------------------- */
/* constructor and destructor */

static int
ignore_attribute_error(PyObject *value)
{
if (value == NULL) {
if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
return -1;
}
PyErr_Clear();
}
return 0;
}

static PyObject*
xmlparser(PyObject* self_, PyObject* args, PyObject* kw)
{
Expand Down Expand Up @@ -2578,14 +2590,33 @@ xmlparser(PyObject* self_, PyObject* args, PyObject* kw)
self->target = target;

self->handle_xml = PyObject_GetAttrString(target, "xml");
if (ignore_attribute_error(self->handle_xml)) {
return NULL;
}
self->handle_start = PyObject_GetAttrString(target, "start");
if (ignore_attribute_error(self->handle_start)) {
return NULL;
}
self->handle_data = PyObject_GetAttrString(target, "data");
if (ignore_attribute_error(self->handle_data)) {
return NULL;
}
self->handle_end = PyObject_GetAttrString(target, "end");
if (ignore_attribute_error(self->handle_end)) {
return NULL;
}
self->handle_comment = PyObject_GetAttrString(target, "comment");
if (ignore_attribute_error(self->handle_comment)) {
return NULL;
}
self->handle_pi = PyObject_GetAttrString(target, "pi");
if (ignore_attribute_error(self->handle_pi)) {
return NULL;
}
self->handle_close = PyObject_GetAttrString(target, "close");

PyErr_Clear();
if (ignore_attribute_error(self->handle_close)) {
return NULL;
}

/* configure parser */
EXPAT(SetUserData)(self->parser, self);
Expand Down