diff --git a/Lib/test/test_minidom.py b/Lib/test/test_minidom.py index 446bbe096bd19d..7ddd83e336b8e3 100644 --- a/Lib/test/test_minidom.py +++ b/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 = ("" + "g'>]>" + "ab&ext;cd∫∫" + "ij") + 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( + "ab&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("" + "a" * n + "") + 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 = ("bcefgh" + "jknop") + expected = source.replace("", "") + 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("bc") + 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(), "bcghjknop") + dom.unlink() + def testComment(self): dom = Document() comment = dom.createComment("comment") diff --git a/Lib/xml/dom/expatbuilder.py b/Lib/xml/dom/expatbuilder.py index e3917c1cc68288..a3ccd2866c1124 100644 --- a/Lib/xml/dom/expatbuilder.py +++ b/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) diff --git a/Misc/NEWS.d/next/Security/2026-10-22-08-10-39.gh-issue-160525.q7TxLd.rst b/Misc/NEWS.d/next/Security/2026-10-22-08-10-39.gh-issue-160525.q7TxLd.rst new file mode 100644 index 00000000000000..ff09ffabc743a2 --- /dev/null +++ b/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.