Skip to content

Commit 99347be

Browse files
committed
gh-160525: Keep expatbuilder text correct when handlers are overridden
A subclass of ExpatBuilder can replace the element, comment or processing instruction handlers without calling the base implementations, which are what end the pending run of character data. Check that the Text node being extended is still the last child of the current node before adding to it, and join any pending pieces when parsing ends, so that such subclasses keep building the same tree.
1 parent 0319494 commit 99347be

2 files changed

Lines changed: 78 additions & 4 deletions

File tree

‎Lib/test/test_minidom.py‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,71 @@ def set_data(self, data):
10451045
msg=f"Possible quadratic behavior: work={w1,w2,w3} ratios={r1,r2}"
10461046
)
10471047

1048+
def testParseTextWithOverriddenHandlers(self):
1049+
# Character data must end up in the right place in the tree and
1050+
# be complete when a subclass of the builder replaces handlers
1051+
# without calling the base implementations.
1052+
class StartElement:
1053+
def start_element_handler(self, name, attributes):
1054+
node = self.document.createElement(name)
1055+
self.curNode.appendChild(node)
1056+
self.curNode = node
1057+
1058+
class EndElement:
1059+
def end_element_handler(self, name):
1060+
self.curNode = self.curNode.parentNode
1061+
1062+
class Comment:
1063+
def comment_handler(self, data):
1064+
self.curNode.appendChild(self.document.createComment(data))
1065+
1066+
class PI:
1067+
def pi_handler(self, target, data):
1068+
self.curNode.appendChild(
1069+
self.document.createProcessingInstruction(target, data))
1070+
1071+
class RemovingEndElement:
1072+
def end_element_handler(self, name):
1073+
node = self.curNode
1074+
self.curNode = node.parentNode
1075+
if name == "d":
1076+
self.curNode.removeChild(node)
1077+
1078+
source = ("<a>b<![CDATA[]]>c<d>e<![CDATA[]]>f</d>g<![CDATA[]]>h"
1079+
"<!--i-->j<![CDATA[]]>k<?l m?>n<d/>o<![CDATA[]]>p</a>")
1080+
expected = source.replace("<![CDATA[]]>", "")
1081+
names = ["#text", "d", "#text", "#comment", "#text", "l", "#text",
1082+
"d", "#text"]
1083+
ExpatBuilder = xml.dom.expatbuilder.ExpatBuilder
1084+
FragmentBuilder = xml.dom.expatbuilder.FragmentBuilder
1085+
for mixins in [(StartElement,), (EndElement,), (Comment,), (PI,),
1086+
(StartElement, EndElement, Comment, PI)]:
1087+
with self.subTest(mixins=mixins):
1088+
class Builder(*mixins, ExpatBuilder):
1089+
pass
1090+
dom = Builder().parseString(source)
1091+
root = dom.documentElement
1092+
self.assertEqual([c.nodeName for c in root.childNodes], names)
1093+
self.assertEqual(root.toxml(), expected)
1094+
1095+
class Builder(*mixins, FragmentBuilder):
1096+
pass
1097+
fragment = Builder(dom).parseString("<d/>b<![CDATA[]]>c")
1098+
self.assertEqual(
1099+
[(c.nodeName, getattr(c, 'data', None))
1100+
for c in fragment.childNodes],
1101+
[('d', None), ('#text', 'bc')])
1102+
dom.unlink()
1103+
1104+
class Builder(RemovingEndElement, ExpatBuilder):
1105+
pass
1106+
dom = Builder().parseString(source)
1107+
root = dom.documentElement
1108+
self.assertEqual([c.nodeName for c in root.childNodes],
1109+
["#text", "#comment", "#text", "l", "#text"])
1110+
self.assertEqual(root.toxml(), "<a>bcgh<!--i-->jk<?l m?>nop</a>")
1111+
dom.unlink()
1112+
10481113
def testComment(self):
10491114
dom = Document()
10501115
comment = dom.createComment("comment")

‎Lib/xml/dom/expatbuilder.py‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ def parseFile(self, file):
217217
parser.Parse(b"", True)
218218
except ParseEscape:
219219
pass
220+
self._flush_text()
220221
doc = self.document
221222
self.reset()
222223
self._parser = None
@@ -230,6 +231,7 @@ def parseString(self, string):
230231
self._setup_subset(string)
231232
except ParseEscape:
232233
pass
234+
self._flush_text()
233235
doc = self.document
234236
self.reset()
235237
self._parser = None
@@ -275,7 +277,8 @@ def _flush_text(self):
275277
# Join the pieces of the current run of character data into
276278
# its Text node. This must be called before anything else is
277279
# added to the tree after the Text node, and before the tree is
278-
# handed to a filter.
280+
# handed to a filter. It is also called when parsing ends, as
281+
# the handlers of a subclass might not have called it.
279282
node = self._text_node
280283
if node is not None:
281284
node.data = ''.join(self._text_parts)
@@ -307,10 +310,15 @@ def character_data_handler_cdata(self, data):
307310
_append_child(self.curNode, node)
308311

309312
def character_data_handler(self, data):
310-
if self._text_node is not None:
311-
self._text_parts.append(data)
312-
return
313313
childNodes = self.curNode.childNodes
314+
if self._text_node is not None:
315+
if childNodes and childNodes[-1] is self._text_node:
316+
self._text_parts.append(data)
317+
return
318+
# The Text node is no longer the last child of the current
319+
# node, which happens when a handler overridden in a
320+
# subclass changes the tree without calling _flush_text().
321+
self._flush_text()
314322
if childNodes and childNodes[-1].nodeType == TEXT_NODE:
315323
node = childNodes[-1]
316324
self._text_parts = [node.data, data]
@@ -667,6 +675,7 @@ def parseString(self, string):
667675
except:
668676
self.reset()
669677
raise
678+
self._flush_text()
670679
fragment = self.fragment
671680
self.reset()
672681
## self._parser = None

0 commit comments

Comments
 (0)