Fix bug #81642: DOMChildNode::replaceWith() bug when replacing a node…

… with itself Closes GH-11363.
php · Jun 4, 2023 · 23f7002 · 23f7002
1 parent b1d8e24
commit 23f7002
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 14 deletions.
diff --git a/NEWS b/NEWS
@@ -23,6 +23,8 @@ PHP                                                                        NEWS
     xpath query). (nielsdos)
   . Fixed bug #67440 (append_node of a DOMDocumentFragment does not reconcile
     namespaces). (nielsdos)
+  . Fixed bug #81642 (DOMChildNode::replaceWith() bug when replacing a node
+    with itself). (nielsdos)
 
 - Opcache:
   . Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)

diff --git a/ext/dom/element.c b/ext/dom/element.c
@@ -1234,7 +1234,7 @@ PHP_METHOD(DOMElement, prepend)
 }
 /* }}} end DOMElement::prepend */
 
-/* {{{ URL: https://dom.spec.whatwg.org/#dom-parentnode-prepend
+/* {{{ URL: https://dom.spec.whatwg.org/#dom-parentnode-replacechildren
 Since: DOM Living Standard (DOM4)
 */
 PHP_METHOD(DOMElement, replaceWith)
@@ -1251,8 +1251,7 @@ PHP_METHOD(DOMElement, replaceWith)
 	id = ZEND_THIS;
 	DOM_GET_OBJ(context, id, xmlNodePtr, intern);
 
-	dom_parent_node_after(intern, args, argc);
-	dom_child_node_remove(intern);
+	dom_child_replace_with(intern, args, argc);
 }
 /* }}} end DOMElement::prepend */
 

diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c
@@ -485,35 +485,45 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
 	xmlFree(fragment);
 }
 
-void dom_child_node_remove(dom_object *context)
+static zend_result dom_child_removal_preconditions(const xmlNodePtr child, int stricterror)
 {
-	xmlNode *child = dom_object_get_node(context);
-	xmlNodePtr children;
-	int stricterror;
-
-	stricterror = dom_get_strict_error(context->document);
-
 	if (dom_node_is_read_only(child) == SUCCESS ||
 		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
 		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
-		return;
+		return FAILURE;
 	}
 
 	if (!child->parent) {
 		php_dom_throw_error(NOT_FOUND_ERR, stricterror);
-		return;
+		return FAILURE;
 	}
 
 	if (dom_node_children_valid(child->parent) == FAILURE) {
-		return;
+		return FAILURE;
 	}
 
-	children = child->parent->children;
+	xmlNodePtr children = child->parent->children;
 	if (!children) {
 		php_dom_throw_error(NOT_FOUND_ERR, stricterror);
+		return FAILURE;
+	}
+
+	return SUCCESS;
+}
+
+void dom_child_node_remove(dom_object *context)
+{
+	xmlNode *child = dom_object_get_node(context);
+	xmlNodePtr children;
+	int stricterror;
+
+	stricterror = dom_get_strict_error(context->document);
+
+	if (UNEXPECTED(dom_child_removal_preconditions(child, stricterror) != SUCCESS)) {
 		return;
 	}
 
+	children = child->parent->children;
 	while (children) {
 		if (children == child) {
 			xmlUnlinkNode(child);
@@ -525,4 +535,41 @@ void dom_child_node_remove(dom_object *context)
 	php_dom_throw_error(NOT_FOUND_ERR, stricterror);
 }
 
+void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc)
+{
+	xmlNodePtr child = dom_object_get_node(context);
+	xmlNodePtr parentNode = child->parent;
+
+	int stricterror = dom_get_strict_error(context->document);
+	if (UNEXPECTED(dom_child_removal_preconditions(child, stricterror) != SUCCESS)) {
+		return;
+	}
+
+	xmlNodePtr insertion_point = child->next;
+
+	xmlNodePtr fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
+	if (UNEXPECTED(fragment == NULL)) {
+		return;
+	}
+
+	xmlNodePtr newchild = fragment->children;
+	xmlDocPtr doc = parentNode->doc;
+
+	if (newchild) {
+		xmlNodePtr last = fragment->last;
+
+		/* Unlink and free it unless it became a part of the fragment. */
+		if (child->parent != fragment) {
+			xmlUnlinkNode(child);
+		}
+
+		dom_pre_insert(insertion_point, parentNode, newchild, fragment);
+
+		dom_fragment_assign_parent_node(parentNode, fragment);
+		dom_reconcile_ns_list(doc, newchild, last);
+	}
+
+	xmlFree(fragment);
+}
+
 #endif
diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h
@@ -132,6 +132,7 @@ void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc);
 void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc);
 void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc);
 void dom_child_node_remove(dom_object *context);
+void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc);
 
 #define DOM_GET_OBJ(__ptr, __id, __prtype, __intern) { \
 	__intern = Z_DOMOBJ_P(__id); \

diff --git a/ext/dom/tests/bug81642.phpt b/ext/dom/tests/bug81642.phpt
@@ -0,0 +1,49 @@
+--TEST--
+Bug #81642 (DOMChildNode::replaceWith() bug when replacing a node with itself)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+// Replace with itself
+$doc = new DOMDocument();
+$doc->appendChild($target = $doc->createElement('test'));
+$target->replaceWith($target);
+var_dump($doc->saveXML());
+
+// Replace with itself + another element
+$doc = new DOMDocument();
+$doc->appendChild($target = $doc->createElement('test'));
+$target->replaceWith($target, $doc->createElement('foo'));
+var_dump($doc->saveXML());
+
+// Replace with text node
+$doc = new DOMDocument();
+$doc->appendChild($target = $doc->createElement('test'));
+$target->replaceWith($target, 'foo');
+var_dump($doc->saveXML());
+
+// Replace with text node variant 2
+$doc = new DOMDocument();
+$doc->appendChild($target = $doc->createElement('test'));
+$target->replaceWith('bar', $target, 'foo');
+var_dump($doc->saveXML());
+
+?>
+--EXPECT--
+string(30) "<?xml version="1.0"?>
+<test/>
+"
+string(37) "<?xml version="1.0"?>
+<test/>
+<foo/>
+"
+string(34) "<?xml version="1.0"?>
+<test/>
+foo
+"
+string(38) "<?xml version="1.0"?>
+bar
+<test/>
+foo
+"