Skip to content

Commit 0dfaf7c

Browse files
encukouDarkaMaulpicnixz
authored andcommitted
gh-152674: Avoid quadratic behavior in xml.etree.ElementPath index predicates (GH-152676)
(cherry picked from commit 2ffab08) Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: Alexis <alexis.challande@trailofbits.com> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent 2d0da0b commit 0dfaf7c

3 files changed

Lines changed: 49 additions & 5 deletions

File tree

Lib/test/test_xml_etree.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3352,6 +3352,37 @@ def test_find_xpath(self):
33523352
self.assertRaisesRegex(SyntaxError, 'XPath', e.find, './tag[last()-0]')
33533353
self.assertRaisesRegex(SyntaxError, 'XPath', e.find, './tag[last()+1]')
33543354

3355+
def test_find_xpath_index_no_quadratic_complexity(self):
3356+
class CountingElement(ET.Element):
3357+
findall_calls = 0
3358+
def findall(self, *args, **kwargs):
3359+
type(self).findall_calls += 1
3360+
return super().findall(*args, **kwargs)
3361+
3362+
def work(n, pattern):
3363+
root = CountingElement("root")
3364+
for _ in range(n):
3365+
ET.SubElement(root, "a")
3366+
CountingElement.findall_calls = 0
3367+
root.findall(pattern)
3368+
return CountingElement.findall_calls
3369+
3370+
for pattern in [".//a[1]", ".//a[last()]"]:
3371+
w1 = work(1024, pattern)
3372+
w2 = work(2048, pattern)
3373+
w3 = work(4096, pattern)
3374+
3375+
self.assertGreater(w1, 0)
3376+
r1 = w2 / w1
3377+
r2 = w3 / w2
3378+
# Doubling N must not ~double the parent.findall calls.
3379+
# Linear-in-N call counts indicate the cache is missing.
3380+
self.assertLess(
3381+
max(r1, r2), 1.5,
3382+
msg=f"Possible quadratic behavior on {pattern!r}: "
3383+
f"calls={w1, w2, w3} ratios={r1, r2}",
3384+
)
3385+
33553386
def test_findall(self):
33563387
e = ET.XML(SAMPLE_XML)
33573388
e[2] = ET.XML(SAMPLE_SECTION)

Lib/xml/etree/ElementPath.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -324,15 +324,22 @@ def select_negated(context, result):
324324
index = -1
325325
def select(context, result):
326326
parent_map = get_parent_map(context)
327+
cache = {}
327328
for elem in result:
328329
try:
329330
parent = parent_map[elem]
331+
except KeyError:
332+
continue
333+
key = (parent, elem.tag)
334+
if key not in cache:
330335
# FIXME: what if the selector is "*" ?
331-
elems = list(parent.findall(elem.tag))
332-
if elems[index] is elem:
333-
yield elem
334-
except (IndexError, KeyError):
335-
pass
336+
elems = parent.findall(elem.tag)
337+
try:
338+
cache[key] = elems[index]
339+
except IndexError:
340+
cache[key] = None
341+
if cache[key] is elem:
342+
yield elem
336343
return select
337344
raise SyntaxError("invalid predicate")
338345

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
The :class:`xml.etree.ElementTree.Element` methods
2+
:meth:`~xml.etree.ElementTree.Element.findall`,
3+
:meth:`~xml.etree.ElementTree.Element.iterfind` and
4+
:meth:`~xml.etree.ElementTree.Element.find` avoid quadratic behavior when
5+
using XPath index predicates (``[1]``, ``[last()]``, ``[last()-N]``) on XML
6+
documents with many same-tag siblings.

0 commit comments

Comments
 (0)