From 1e2a2d7df27a0d90ad4c3d39da2b6ea784b45b82 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 11 May 2024 19:44:02 +0200
Subject: [PATCH] Fix crash in ParentNode::append() when dealing with a
fragment containing text nodes
Credits for test: https://github.com/PhpGt/Dom/pull/454.
Closes GH-14206.
---
NEWS | 2 ++
ext/dom/parentnode.c | 22 +++++++++++++++----
...entNode_append_fragment_text_coalesce.phpt | 21 ++++++++++++++++++
3 files changed, 41 insertions(+), 4 deletions(-)
create mode 100644 ext/dom/tests/ParentNode_append_fragment_text_coalesce.phpt
diff --git a/NEWS b/NEWS
index b4eb80e3398..ac8ee18065b 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,8 @@ PHP NEWS
. Fix references not handled correctly in C14N. (nielsdos)
. Fix crash when calling childNodes next() when iterator is exhausted.
(nielsdos)
+ . Fix crash in ParentNode::append() when dealing with a fragment
+ containing text nodes. (nielsdos)
- Hash:
. ext/hash: Swap the checking order of `__has_builtin` and `__GNUC__`
diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c
index d6b0705545a..c30db6fcd74 100644
--- a/ext/dom/parentnode.c
+++ b/ext/dom/parentnode.c
@@ -150,6 +150,22 @@ static xmlDocPtr dom_doc_from_context_node(xmlNodePtr contextNode)
}
}
+/* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild):
+ * "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)".
+ * So we must use a custom way of adding that does not merge. */
+static void dom_add_child_without_merging(xmlNodePtr parent, xmlNodePtr child)
+{
+ if (parent->children == NULL) {
+ parent->children = child;
+ } else {
+ xmlNodePtr last = parent->last;
+ last->next = child;
+ child->prev = last;
+ }
+ parent->last = child;
+ child->parent = parent;
+}
+
xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc)
{
int i;
@@ -183,7 +199,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
* So we must take a copy if this situation arises to prevent a use-after-free. */
bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE;
if (will_free) {
- newNode = xmlCopyNode(newNode, 1);
+ newNode = xmlCopyNode(newNode, 0);
}
if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
@@ -192,9 +208,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
while (newNode) {
xmlNodePtr next = newNode->next;
xmlUnlinkNode(newNode);
- if (!xmlAddChild(fragment, newNode)) {
- goto err;
- }
+ dom_add_child_without_merging(fragment, newNode);
newNode = next;
}
} else if (!xmlAddChild(fragment, newNode)) {
diff --git a/ext/dom/tests/ParentNode_append_fragment_text_coalesce.phpt b/ext/dom/tests/ParentNode_append_fragment_text_coalesce.phpt
new file mode 100644
index 00000000000..601819d6117
--- /dev/null
+++ b/ext/dom/tests/ParentNode_append_fragment_text_coalesce.phpt
@@ -0,0 +1,21 @@
+--TEST--
+Text coalesce bug when appending fragment with text nodes
+--EXTENSIONS--
+dom
+--FILE--
+loadXML('');
+
+$sut = $document->createDocumentFragment();
+for($i = 0; $i < 10; $i++) {
+ $textNode = $document->createTextNode("Node$i");
+ $sut->append($textNode);
+}
+
+$document->documentElement->append($sut);
+echo $document->saveXML();
+?>
+--EXPECT--
+
+Node0Node1Node2Node3Node4Node5Node6Node7Node8Node9