gh-160525: Fix quadratic text concatenation in xml.dom.expatbuilder

Lib/test/test_minidom.py

@@ -6,6 +6,7 @@
 from test import support
 import unittest
 
+import xml.dom.expatbuilder
 import xml.dom.minidom
 
 from xml.dom.minidom import parse, Attr, Node, Document, Element, parseString
@@ -985,6 +986,130 @@ def testParseString(self):
         self.assertEqual(dom.documentElement.firstChild.data, "\xe9")
         dom.unlink()
 
+    def testParseTextInManyPieces(self):
+        # Expat delivers a run of character data in several callbacks
+        # when it is interrupted by markup that adds no node, such as an
+        # empty CDATA section or a reference to an external entity;
+        # the pieces must end up in a single Text node.
+        source = ("<!DOCTYPE doc [<!ENTITY ext SYSTEM 'http://xml.python.org/e'>"
+                  "<!ENTITY int 'e&ext;f<![CDATA[]]>g'>]>"
+                  "<doc>a<![CDATA[]]>b&ext;c<!--x-->d&int;&int;"
+                  "<![CDATA[h]]><![CDATA[]]>i<?pi x?>j</doc>")
+        for namespaces in True, False:
+            with self.subTest(namespaces=namespaces):
+                dom = xml.dom.expatbuilder.parseString(source, namespaces)
+                children = dom.documentElement.childNodes
+                self.assertEqual(
+                    [(c.nodeName, getattr(c, 'data', None)) for c in children],
+                    [('#text', 'abc'), ('#comment', 'x'),
+                     ('#text', 'defgefg'), ('#cdata-section', 'h'),
+                     ('#text', 'i'), ('pi', 'x'), ('#text', 'j')])
+                # A fragment can end in a run of character data.
+                fragment = xml.dom.expatbuilder.parseFragmentString(
+                    "<e/>a<![CDATA[]]>b&ext;c", dom.documentElement,
+                    namespaces)
+                self.assertEqual(
+                    [(c.nodeName, getattr(c, 'data', None))
+                     for c in fragment.childNodes],
+                    [('e', None), ('#text', 'abc')])
+                dom.unlink()
+
+    def testParseTextInManyPiecesNoQuadraticComplexity(self):
+        def work(n):
+            total_chars = 0
+
+            # Count the characters stored into text nodes as a proxy
+            # for the work done.
+            def set_data(self, data):
+                nonlocal total_chars
+                total_chars += len(data)
+                self._data = data
+
+            data = property(xml.dom.minidom.CharacterData._get_data, set_data)
+            with support.swap_attr(xml.dom.minidom.CharacterData, "data", data):
+                dom = parseString("<doc>" + "a<![CDATA[]]>" * n + "</doc>")
+            self.assertEqual(dom.documentElement.firstChild.data, "a" * n)
+            dom.unlink()
+            return total_chars
+
+        # Doubling N should not ~quadruple the work.
+        w1 = work(1024)
+        w2 = work(2048)
+        w3 = work(4096)
+
+        self.assertGreater(w1, 0)
+        r1 = w2 / w1
+        r2 = w3 / w2
+        self.assertLess(
+            max(r1, r2), 3.2,
+            msg=f"Possible quadratic behavior: work={w1,w2,w3} ratios={r1,r2}"
+        )
+
+    def testParseTextWithOverriddenHandlers(self):
+        # Character data must end up in the right place in the tree and
+        # be complete when a subclass of the builder replaces handlers
+        # without calling the base implementations.
+        class StartElement:
+            def start_element_handler(self, name, attributes):
+                node = self.document.createElement(name)
+                self.curNode.appendChild(node)
+                self.curNode = node
+
+        class EndElement:
+            def end_element_handler(self, name):
+                self.curNode = self.curNode.parentNode
+
+        class Comment:
+            def comment_handler(self, data):
+                self.curNode.appendChild(self.document.createComment(data))
+
+        class PI:
+            def pi_handler(self, target, data):
+                self.curNode.appendChild(
+                    self.document.createProcessingInstruction(target, data))
+
+        class RemovingEndElement:
+            def end_element_handler(self, name):
+                node = self.curNode
+                self.curNode = node.parentNode
+                if name == "d":
+                    self.curNode.removeChild(node)
+
+        source = ("<a>b<![CDATA[]]>c<d>e<![CDATA[]]>f</d>g<![CDATA[]]>h"
+                  "<!--i-->j<![CDATA[]]>k<?l m?>n<d/>o<![CDATA[]]>p</a>")
+        expected = source.replace("<![CDATA[]]>", "")
+        names = ["#text", "d", "#text", "#comment", "#text", "l", "#text",
+                 "d", "#text"]
+        ExpatBuilder = xml.dom.expatbuilder.ExpatBuilder
+        FragmentBuilder = xml.dom.expatbuilder.FragmentBuilder
+        for mixins in [(StartElement,), (EndElement,), (Comment,), (PI,),
+                       (StartElement, EndElement, Comment, PI)]:
+            with self.subTest(mixins=mixins):
+                class Builder(*mixins, ExpatBuilder):
+                    pass
+                dom = Builder().parseString(source)
+                root = dom.documentElement
+                self.assertEqual([c.nodeName for c in root.childNodes], names)
+                self.assertEqual(root.toxml(), expected)
+
+                class Builder(*mixins, FragmentBuilder):
+                    pass
+                fragment = Builder(dom).parseString("<d/>b<![CDATA[]]>c")
+                self.assertEqual(
+                    [(c.nodeName, getattr(c, 'data', None))
+                     for c in fragment.childNodes],
+                    [('d', None), ('#text', 'bc')])
+                dom.unlink()
+
+        class Builder(RemovingEndElement, ExpatBuilder):
+            pass
+        dom = Builder().parseString(source)
+        root = dom.documentElement
+        self.assertEqual([c.nodeName for c in root.childNodes],
+                         ["#text", "#comment", "#text", "l", "#text"])
+        self.assertEqual(root.toxml(), "<a>bcgh<!--i-->jk<?l m?>nop</a>")
+        dom.unlink()
+
     def testComment(self):
         dom = Document()
         comment = dom.createComment("comment")

Lib/xml/dom/expatbuilder.py

@@ -20,7 +20,12 @@
 #      speedup since pyexpat can break up character data into multiple
 #      callbacks even though we set the buffer_text attribute on the
 #      parser.  This also gives us the advantage that we don't need a
-#      separate normalization pass.
+#      separate normalization pass.  The pieces are collected in a list
+#      and joined once the run of character data ends (see
+#      ._flush_text()), since a run can arrive in a great many
+#      callbacks (pyexpat flushes its buffer before every other
+#      handler) and copying the accumulated text on each of them
+#      would be quadratic.
 #
 #   -  Determining that a node exists is done using an identity comparison
 #      with None rather than a truth test; this avoids searching for and
@@ -170,6 +175,10 @@ def reset(self):
         self.curNode = self.document
         self._elem_info = self.document._elem_info
         self._cdata = False
+        # Text node currently being extended and the pieces of its data
+        # which have not been joined yet; see _flush_text().
+        self._text_node = None
+        self._text_parts = None
 
     def install(self, parser):
         """Install the callbacks needed to build the DOM into the parser."""
@@ -208,6 +217,7 @@ def parseFile(self, file):
             parser.Parse(b"", True)
         except ParseEscape:
             pass
+        self._flush_text()
         doc = self.document
         self.reset()
         self._parser = None
@@ -221,6 +231,7 @@ def parseString(self, string):
             self._setup_subset(string)
         except ParseEscape:
             pass
+        self._flush_text()
         doc = self.document
         self.reset()
         self._parser = None
@@ -262,42 +273,62 @@ def end_doctype_decl_handler(self):
         if not (self._elem_info or self._filter):
             self._finish_end_element = id
 
+    def _flush_text(self):
+        # Join the pieces of the current run of character data into
+        # its Text node.  This must be called before anything else is
+        # added to the tree after the Text node, and before the tree is
+        # handed to a filter.  It is also called when parsing ends, as
+        # the handlers of a subclass might not have called it.
+        node = self._text_node
+        if node is not None:
+            node.data = ''.join(self._text_parts)
+            self._text_node = None
+            self._text_parts = None
+
     def pi_handler(self, target, data):
+        self._flush_text()
         node = self.document.createProcessingInstruction(target, data)
         _append_child(self.curNode, node)
         if self._filter and self._filter.acceptNode(node) == FILTER_REJECT:
             self.curNode.removeChild(node)
 
     def character_data_handler_cdata(self, data):
+        if not self._cdata:
+            self.character_data_handler(data)
+            return
+        # Flushing here rather than in start_cdata_section_handler()
+        # keeps an empty CDATA section, which adds no node, from ending
+        # the run of character data around it.
+        self._flush_text()
         childNodes = self.curNode.childNodes
-        if self._cdata:
-            if (  self._cdata_continue
-                  and childNodes[-1].nodeType == CDATA_SECTION_NODE):
-                childNodes[-1].appendData(data)
-                return
-            node = self.document.createCDATASection(data)
-            self._cdata_continue = True
-        elif childNodes and childNodes[-1].nodeType == TEXT_NODE:
-            node = childNodes[-1]
-            value = node.data + data
-            node.data = value
+        if (  self._cdata_continue
+              and childNodes[-1].nodeType == CDATA_SECTION_NODE):
+            childNodes[-1].appendData(data)
             return
-        else:
-            node = minidom.Text()
-            node.data = data
-            node.ownerDocument = self.document
+        node = self.document.createCDATASection(data)
+        self._cdata_continue = True
         _append_child(self.curNode, node)
 
     def character_data_handler(self, data):
         childNodes = self.curNode.childNodes
+        if self._text_node is not None:
+            if childNodes and childNodes[-1] is self._text_node:
+                self._text_parts.append(data)
+                return
+            # The Text node is no longer the last child of the current
+            # node, which happens when a handler overridden in a
+            # subclass changes the tree without calling _flush_text().
+            self._flush_text()
         if childNodes and childNodes[-1].nodeType == TEXT_NODE:
             node = childNodes[-1]
-            node.data = node.data + data
-            return
-        node = minidom.Text()
-        node.data = node.data + data
-        node.ownerDocument = self.document
-        _append_child(self.curNode, node)
+            self._text_parts = [node.data, data]
+        else:
+            node = minidom.Text()
+            node.data = data
+            node.ownerDocument = self.document
+            _append_child(self.curNode, node)
+            self._text_parts = [data]
+        self._text_node = node
 
     def entity_decl_handler(self, entityName, is_parameter_entity, value,
                             base, systemId, publicId, notationName):
@@ -324,6 +355,7 @@ def notation_decl_handler(self, notationName, base, systemId, publicId):
             del self.document.doctype.notations._seq[-1]
 
     def comment_handler(self, data):
+        self._flush_text()
         node = self.document.createComment(data)
         _append_child(self.curNode, node)
         if self._filter and self._filter.acceptNode(node) == FILTER_REJECT:
@@ -347,6 +379,7 @@ def first_element_handler(self, name, attributes):
         self.start_element_handler(name, attributes)
 
     def start_element_handler(self, name, attributes):
+        self._flush_text()
         node = self.document.createElement(name)
         _append_child(self.curNode, node)
         self.curNode = node
@@ -389,6 +422,7 @@ def _finish_start_element(self, node):
     # be changed to match.
     #
     def end_element_handler(self, name):
+        self._flush_text()
         curNode = self.curNode
         self.curNode = curNode.parentNode
         self._finish_end_element(curNode)
@@ -641,6 +675,7 @@ def parseString(self, string):
         except:
             self.reset()
             raise
+        self._flush_text()
         fragment = self.fragment
         self.reset()
 ##         self._parser = None
@@ -734,6 +769,7 @@ def start_namespace_decl_handler(self, prefix, uri):
         self._ns_ordered_prefixes.append((prefix, uri))
 
     def start_element_handler(self, name, attributes):
+        self._flush_text()
         if ' ' in name:
             uri, localname, prefix, qname = _parse_ns_name(self, name)
         else:
@@ -789,6 +825,7 @@ def start_element_handler(self, name, attributes):
         # it needs to be changed as well.
         #
         def end_element_handler(self, name):
+            self._flush_text()
             curNode = self.curNode
             if ' ' in name:
                 uri, localname, prefix, qname = _parse_ns_name(self, name)

Misc/NEWS.d/next/Security/2026-10-22-08-10-39.gh-issue-160525.q7TxLd.rst

@@ -0,0 +1,4 @@
+Fix quadratic complexity in :func:`xml.dom.minidom.parse` and
+:func:`xml.dom.minidom.parseString` when the text of an element is
+delivered by the parser in many pieces, for example when it is interrupted by
+empty CDATA sections or references to external entities.