Skip to content

Commit 0319494

Browse files
committed
gh-160525: Fix quadratic text concatenation in xml.dom.expatbuilder
Collect the pieces of a run of character data in a list and join them once, instead of copying the Text node's data on every Expat callback. ANT-2026-ZRQNVCB1 ANT-2026-QXYQ68DQ
1 parent 3b3c692 commit 0319494

3 files changed

Lines changed: 114 additions & 22 deletions

File tree

‎Lib/test/test_minidom.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from test import support
77
import unittest
88

9+
import xml.dom.expatbuilder
910
import xml.dom.minidom
1011

1112
from xml.dom.minidom import parse, Attr, Node, Document, Element, parseString
@@ -985,6 +986,65 @@ def testParseString(self):
985986
self.assertEqual(dom.documentElement.firstChild.data, "\xe9")
986987
dom.unlink()
987988

989+
def testParseTextInManyPieces(self):
990+
# Expat delivers a run of character data in several callbacks
991+
# when it is interrupted by markup that adds no node, such as an
992+
# empty CDATA section or a reference to an external entity;
993+
# the pieces must end up in a single Text node.
994+
source = ("<!DOCTYPE doc [<!ENTITY ext SYSTEM 'http://xml.python.org/e'>"
995+
"<!ENTITY int 'e&ext;f<![CDATA[]]>g'>]>"
996+
"<doc>a<![CDATA[]]>b&ext;c<!--x-->d&int;&int;"
997+
"<![CDATA[h]]><![CDATA[]]>i<?pi x?>j</doc>")
998+
for namespaces in True, False:
999+
with self.subTest(namespaces=namespaces):
1000+
dom = xml.dom.expatbuilder.parseString(source, namespaces)
1001+
children = dom.documentElement.childNodes
1002+
self.assertEqual(
1003+
[(c.nodeName, getattr(c, 'data', None)) for c in children],
1004+
[('#text', 'abc'), ('#comment', 'x'),
1005+
('#text', 'defgefg'), ('#cdata-section', 'h'),
1006+
('#text', 'i'), ('pi', 'x'), ('#text', 'j')])
1007+
# A fragment can end in a run of character data.
1008+
fragment = xml.dom.expatbuilder.parseFragmentString(
1009+
"<e/>a<![CDATA[]]>b&ext;c", dom.documentElement,
1010+
namespaces)
1011+
self.assertEqual(
1012+
[(c.nodeName, getattr(c, 'data', None))
1013+
for c in fragment.childNodes],
1014+
[('e', None), ('#text', 'abc')])
1015+
dom.unlink()
1016+
1017+
def testParseTextInManyPiecesNoQuadraticComplexity(self):
1018+
def work(n):
1019+
total_chars = 0
1020+
1021+
# Count the characters stored into text nodes as a proxy
1022+
# for the work done.
1023+
def set_data(self, data):
1024+
nonlocal total_chars
1025+
total_chars += len(data)
1026+
self._data = data
1027+
1028+
data = property(xml.dom.minidom.CharacterData._get_data, set_data)
1029+
with support.swap_attr(xml.dom.minidom.CharacterData, "data", data):
1030+
dom = parseString("<doc>" + "a<![CDATA[]]>" * n + "</doc>")
1031+
self.assertEqual(dom.documentElement.firstChild.data, "a" * n)
1032+
dom.unlink()
1033+
return total_chars
1034+
1035+
# Doubling N should not ~quadruple the work.
1036+
w1 = work(1024)
1037+
w2 = work(2048)
1038+
w3 = work(4096)
1039+
1040+
self.assertGreater(w1, 0)
1041+
r1 = w2 / w1
1042+
r2 = w3 / w2
1043+
self.assertLess(
1044+
max(r1, r2), 3.2,
1045+
msg=f"Possible quadratic behavior: work={w1,w2,w3} ratios={r1,r2}"
1046+
)
1047+
9881048
def testComment(self):
9891049
dom = Document()
9901050
comment = dom.createComment("comment")

‎Lib/xml/dom/expatbuilder.py‎

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,12 @@
2020
# speedup since pyexpat can break up character data into multiple
2121
# callbacks even though we set the buffer_text attribute on the
2222
# parser. This also gives us the advantage that we don't need a
23-
# separate normalization pass.
23+
# separate normalization pass. The pieces are collected in a list
24+
# and joined once the run of character data ends (see
25+
# ._flush_text()), since a run can arrive in a great many
26+
# callbacks (pyexpat flushes its buffer before every other
27+
# handler) and copying the accumulated text on each of them
28+
# would be quadratic.
2429
#
2530
# - Determining that a node exists is done using an identity comparison
2631
# with None rather than a truth test; this avoids searching for and
@@ -170,6 +175,10 @@ def reset(self):
170175
self.curNode = self.document
171176
self._elem_info = self.document._elem_info
172177
self._cdata = False
178+
# Text node currently being extended and the pieces of its data
179+
# which have not been joined yet; see _flush_text().
180+
self._text_node = None
181+
self._text_parts = None
173182

174183
def install(self, parser):
175184
"""Install the callbacks needed to build the DOM into the parser."""
@@ -262,42 +271,56 @@ def end_doctype_decl_handler(self):
262271
if not (self._elem_info or self._filter):
263272
self._finish_end_element = id
264273

274+
def _flush_text(self):
275+
# Join the pieces of the current run of character data into
276+
# its Text node. This must be called before anything else is
277+
# added to the tree after the Text node, and before the tree is
278+
# handed to a filter.
279+
node = self._text_node
280+
if node is not None:
281+
node.data = ''.join(self._text_parts)
282+
self._text_node = None
283+
self._text_parts = None
284+
265285
def pi_handler(self, target, data):
286+
self._flush_text()
266287
node = self.document.createProcessingInstruction(target, data)
267288
_append_child(self.curNode, node)
268289
if self._filter and self._filter.acceptNode(node) == FILTER_REJECT:
269290
self.curNode.removeChild(node)
270291

271292
def character_data_handler_cdata(self, data):
293+
if not self._cdata:
294+
self.character_data_handler(data)
295+
return
296+
# Flushing here rather than in start_cdata_section_handler()
297+
# keeps an empty CDATA section, which adds no node, from ending
298+
# the run of character data around it.
299+
self._flush_text()
272300
childNodes = self.curNode.childNodes
273-
if self._cdata:
274-
if ( self._cdata_continue
275-
and childNodes[-1].nodeType == CDATA_SECTION_NODE):
276-
childNodes[-1].appendData(data)
277-
return
278-
node = self.document.createCDATASection(data)
279-
self._cdata_continue = True
280-
elif childNodes and childNodes[-1].nodeType == TEXT_NODE:
281-
node = childNodes[-1]
282-
value = node.data + data
283-
node.data = value
301+
if ( self._cdata_continue
302+
and childNodes[-1].nodeType == CDATA_SECTION_NODE):
303+
childNodes[-1].appendData(data)
284304
return
285-
else:
286-
node = minidom.Text()
287-
node.data = data
288-
node.ownerDocument = self.document
305+
node = self.document.createCDATASection(data)
306+
self._cdata_continue = True
289307
_append_child(self.curNode, node)
290308

291309
def character_data_handler(self, data):
310+
if self._text_node is not None:
311+
self._text_parts.append(data)
312+
return
292313
childNodes = self.curNode.childNodes
293314
if childNodes and childNodes[-1].nodeType == TEXT_NODE:
294315
node = childNodes[-1]
295-
node.data = node.data + data
296-
return
297-
node = minidom.Text()
298-
node.data = node.data + data
299-
node.ownerDocument = self.document
300-
_append_child(self.curNode, node)
316+
self._text_parts = [node.data, data]
317+
else:
318+
node = minidom.Text()
319+
node.data = data
320+
node.ownerDocument = self.document
321+
_append_child(self.curNode, node)
322+
self._text_parts = [data]
323+
self._text_node = node
301324

302325
def entity_decl_handler(self, entityName, is_parameter_entity, value,
303326
base, systemId, publicId, notationName):
@@ -324,6 +347,7 @@ def notation_decl_handler(self, notationName, base, systemId, publicId):
324347
del self.document.doctype.notations._seq[-1]
325348

326349
def comment_handler(self, data):
350+
self._flush_text()
327351
node = self.document.createComment(data)
328352
_append_child(self.curNode, node)
329353
if self._filter and self._filter.acceptNode(node) == FILTER_REJECT:
@@ -347,6 +371,7 @@ def first_element_handler(self, name, attributes):
347371
self.start_element_handler(name, attributes)
348372

349373
def start_element_handler(self, name, attributes):
374+
self._flush_text()
350375
node = self.document.createElement(name)
351376
_append_child(self.curNode, node)
352377
self.curNode = node
@@ -389,6 +414,7 @@ def _finish_start_element(self, node):
389414
# be changed to match.
390415
#
391416
def end_element_handler(self, name):
417+
self._flush_text()
392418
curNode = self.curNode
393419
self.curNode = curNode.parentNode
394420
self._finish_end_element(curNode)
@@ -734,6 +760,7 @@ def start_namespace_decl_handler(self, prefix, uri):
734760
self._ns_ordered_prefixes.append((prefix, uri))
735761

736762
def start_element_handler(self, name, attributes):
763+
self._flush_text()
737764
if ' ' in name:
738765
uri, localname, prefix, qname = _parse_ns_name(self, name)
739766
else:
@@ -789,6 +816,7 @@ def start_element_handler(self, name, attributes):
789816
# it needs to be changed as well.
790817
#
791818
def end_element_handler(self, name):
819+
self._flush_text()
792820
curNode = self.curNode
793821
if ' ' in name:
794822
uri, localname, prefix, qname = _parse_ns_name(self, name)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix quadratic complexity in :func:`xml.dom.minidom.parse` and
2+
:func:`xml.dom.minidom.parseString` when the text of an element is
3+
delivered by the parser in many pieces, for example when it is interrupted by
4+
empty CDATA sections or references to external entities.

0 commit comments

Comments
 (0)