Skip to content

Commit 7777e4e

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 d2b2f5e commit 7777e4e

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

2912+
def test_find_xpath_index_no_quadratic_complexity(self):
2913+
class CountingElement(ET.Element):
2914+
findall_calls = 0
2915+
def findall(self, *args, **kwargs):
2916+
type(self).findall_calls += 1
2917+
return super().findall(*args, **kwargs)
2918+
2919+
def work(n, pattern):
2920+
root = CountingElement("root")
2921+
for _ in range(n):
2922+
ET.SubElement(root, "a")
2923+
CountingElement.findall_calls = 0
2924+
root.findall(pattern)
2925+
return CountingElement.findall_calls
2926+
2927+
for pattern in [".//a[1]", ".//a[last()]"]:
2928+
w1 = work(1024, pattern)
2929+
w2 = work(2048, pattern)
2930+
w3 = work(4096, pattern)
2931+
2932+
self.assertGreater(w1, 0)
2933+
r1 = w2 / w1
2934+
r2 = w3 / w2
2935+
# Doubling N must not ~double the parent.findall calls.
2936+
# Linear-in-N call counts indicate the cache is missing.
2937+
self.assertLess(
2938+
max(r1, r2), 1.5,
2939+
msg=f"Possible quadratic behavior on {pattern!r}: "
2940+
f"calls={w1, w2, w3} ratios={r1, r2}",
2941+
)
2942+
29122943
def test_findall(self):
29132944
e = ET.XML(SAMPLE_XML)
29142945
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)