Skip to content

Commit 2ffab08

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

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
@@ -3477,6 +3477,37 @@ def test_find_xpath(self):
34773477
self.assertRaisesRegex(SyntaxError, 'XPath', e.find, './tag[last()-0]')
34783478
self.assertRaisesRegex(SyntaxError, 'XPath', e.find, './tag[last()+1]')
34793479

3480+
def test_find_xpath_index_no_quadratic_complexity(self):
3481+
class CountingElement(ET.Element):
3482+
findall_calls = 0
3483+
def findall(self, *args, **kwargs):
3484+
type(self).findall_calls += 1
3485+
return super().findall(*args, **kwargs)
3486+
3487+
def work(n, pattern):
3488+
root = CountingElement("root")
3489+
for _ in range(n):
3490+
ET.SubElement(root, "a")
3491+
CountingElement.findall_calls = 0
3492+
root.findall(pattern)
3493+
return CountingElement.findall_calls
3494+
3495+
for pattern in [".//a[1]", ".//a[last()]"]:
3496+
w1 = work(1024, pattern)
3497+
w2 = work(2048, pattern)
3498+
w3 = work(4096, pattern)
3499+
3500+
self.assertGreater(w1, 0)
3501+
r1 = w2 / w1
3502+
r2 = w3 / w2
3503+
# Doubling N must not ~double the parent.findall calls.
3504+
# Linear-in-N call counts indicate the cache is missing.
3505+
self.assertLess(
3506+
max(r1, r2), 1.5,
3507+
msg=f"Possible quadratic behavior on {pattern!r}: "
3508+
f"calls={w1, w2, w3} ratios={r1, r2}",
3509+
)
3510+
34803511
def test_findall(self):
34813512
e = ET.XML(SAMPLE_XML)
34823513
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)