Skip to content

Commit 1e2a2d7

Browse files
committed
Fix crash in ParentNode::append() when dealing with a fragment containing text nodes
Credits for test: phpgt/Dom#454. Closes GH-14206.
1 parent 1890d47 commit 1e2a2d7

File tree

3 files changed

+41
-4
lines changed

3 files changed

+41
-4
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ PHP NEWS
2020
. Fix references not handled correctly in C14N. (nielsdos)
2121
. Fix crash when calling childNodes next() when iterator is exhausted.
2222
(nielsdos)
23+
. Fix crash in ParentNode::append() when dealing with a fragment
24+
containing text nodes. (nielsdos)
2325

2426
- Hash:
2527
. ext/hash: Swap the checking order of `__has_builtin` and `__GNUC__`

ext/dom/parentnode.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,22 @@ static xmlDocPtr dom_doc_from_context_node(xmlNodePtr contextNode)
150150
}
151151
}
152152

153+
/* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild):
154+
* "Add a new node to @parent, at the end of the child (or property) list merging adjacent TEXT nodes (in which case @cur is freed)".
155+
* So we must use a custom way of adding that does not merge. */
156+
static void dom_add_child_without_merging(xmlNodePtr parent, xmlNodePtr child)
157+
{
158+
if (parent->children == NULL) {
159+
parent->children = child;
160+
} else {
161+
xmlNodePtr last = parent->last;
162+
last->next = child;
163+
child->prev = last;
164+
}
165+
parent->last = child;
166+
child->parent = parent;
167+
}
168+
153169
xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc)
154170
{
155171
int i;
@@ -183,7 +199,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
183199
* So we must take a copy if this situation arises to prevent a use-after-free. */
184200
bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE;
185201
if (will_free) {
186-
newNode = xmlCopyNode(newNode, 1);
202+
newNode = xmlCopyNode(newNode, 0);
187203
}
188204

189205
if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
@@ -192,9 +208,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
192208
while (newNode) {
193209
xmlNodePtr next = newNode->next;
194210
xmlUnlinkNode(newNode);
195-
if (!xmlAddChild(fragment, newNode)) {
196-
goto err;
197-
}
211+
dom_add_child_without_merging(fragment, newNode);
198212
newNode = next;
199213
}
200214
} else if (!xmlAddChild(fragment, newNode)) {
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
Text coalesce bug when appending fragment with text nodes
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$document = new DOMDocument();
8+
$document->loadXML('<root/>');
9+
10+
$sut = $document->createDocumentFragment();
11+
for($i = 0; $i < 10; $i++) {
12+
$textNode = $document->createTextNode("Node$i");
13+
$sut->append($textNode);
14+
}
15+
16+
$document->documentElement->append($sut);
17+
echo $document->saveXML();
18+
?>
19+
--EXPECT--
20+
<?xml version="1.0"?>
21+
<root>Node0Node1Node2Node3Node4Node5Node6Node7Node8Node9</root>

0 commit comments

Comments
 (0)