Handle fragments consisting out of multiple children without a single root correctly

ndossche · ndossche · commit 5c26258eeb27 · 2023-07-13T16:09:04.000+02:00
Closes GH-11698.
diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c
@@ -183,15 +183,7 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
 					goto err;
 				}
 
-				if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
-					/* Unpack document fragment nodes, the behaviour differs for different libxml2 versions. */
-					newNode = newNode->children;
-					if (UNEXPECTED(newNode == NULL)) {
-						/* No nodes to add, nothing to do here */
-						continue;
-					}
-					xmlUnlinkNode(newNode);
-				} else if (newNode->parent != NULL) {
+				if (newNode->parent != NULL) {
 					xmlUnlinkNode(newNode);
 				}
 
@@ -210,14 +202,23 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
 					newNode = xmlCopyNode(newNode, 1);
 				}
 
-				if (!xmlAddChild(fragment, newNode)) {
+				if (newNode->type == XML_DOCUMENT_FRAG_NODE) {
+					/* Unpack document fragment nodes, the behaviour differs for different libxml2 versions. */
+					newNode = newNode->children;
+					while (newNode) {
+						xmlNodePtr next = newNode->next;
+						xmlUnlinkNode(newNode);
+						if (!xmlAddChild(fragment, newNode)) {
+							goto hierarchy_request_err;
+						}
+						newNode = next;
+					}
+				} else if (!xmlAddChild(fragment, newNode)) {
 					if (will_free) {
 						xmlFreeNode(newNode);
 					}
 					goto hierarchy_request_err;
 				}
-
-				continue;
 			} else {
 				zend_argument_type_error(i + 1, "must be of type DOMNode|string, %s given", zend_zval_type_name(&nodes[i]));
 				goto err;
@@ -363,14 +364,13 @@ static void dom_pre_insert(xmlNodePtr insertion_point, xmlNodePtr parentNode, xm
 		/* Place it as last node */
 		if (parentNode->children) {
 			/* There are children */
-			fragment->last->prev = parentNode->last;
-			newchild->prev = parentNode->last->prev;
+			newchild->prev = parentNode->last;
 			parentNode->last->next = newchild;
 		} else {
 			/* No children, because they moved out when they became a fragment */
 			parentNode->children = newchild;
-			parentNode->last = newchild;
 		}
+		parentNode->last = fragment->last;
 	} else {
 		/* Insert fragment before insertion_point */
 		fragment->last->next = insertion_point;
diff --git a/ext/dom/tests/fragments_multiple_nodes_DOMParentNode.phpt b/ext/dom/tests/fragments_multiple_nodes_DOMParentNode.phpt
@@ -0,0 +1,38 @@
+--TEST--
+Handling fragments of multiple nodes for DOMParentNode
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = new DOMDocument();
+$dom->loadXML('<!DOCTYPE HTML><html><container/></html>');
+
+$container = $dom->documentElement->firstElementChild;
+
+$fragment = $dom->createDocumentFragment();
+$fragment->appendChild($dom->createElement('p', '1'));
+$fragment->appendChild($dom->createElement('b', '2'));
+$container->replaceWith($fragment);
+echo $dom->saveXML();
+
+$dom->documentElement->append('foo');
+echo $dom->saveXML();
+
+$fragment = $dom->createDocumentFragment();
+$fragment->appendChild($dom->createElement('p', '3'));
+$fragment->appendChild($dom->createElement('b', '4'));
+$dom->documentElement->prepend($fragment);
+echo $dom->saveXML();
+
+?>
+--EXPECT--
+<?xml version="1.0"?>
+<!DOCTYPE HTML>
+<html><p>1</p><b>2</b></html>
+<?xml version="1.0"?>
+<!DOCTYPE HTML>
+<html><p>1</p><b>2</b>foo</html>
+<?xml version="1.0"?>
+<!DOCTYPE HTML>
+<html><p>3</p><b>4</b><p>1</p><b>2</b>foo</html>
