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