Skip to content

Commit ac08808

Browse files
krakjoekrakjoe
committed
Merge branch 'PHP-8.1'
* PHP-8.1: Fix O(N) performance of DOMNode::replaceChild() and DOMNode::removeChild()
2 parents e638725 + 781e6b4 commit ac08808

File tree

4 files changed

+86
-47
lines changed

4 files changed

+86
-47
lines changed

‎ext/dom/node.c‎

Lines changed: 26 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,9 +1000,9 @@ PHP_METHOD(DOMNode, insertBefore)
10001000
PHP_METHOD(DOMNode, replaceChild)
10011001
{
10021002
zval *id, *newnode, *oldnode;
1003-
xmlNodePtr children, newchild, oldchild, nodep;
1003+
xmlNodePtr newchild, oldchild, nodep;
10041004
dom_object *intern, *newchildobj, *oldchildobj;
1005-
int foundoldchild = 0, stricterror;
1005+
int stricterror;
10061006

10071007
int ret;
10081008

@@ -1020,8 +1020,7 @@ PHP_METHOD(DOMNode, replaceChild)
10201020
DOM_GET_OBJ(newchild, newnode, xmlNodePtr, newchildobj);
10211021
DOM_GET_OBJ(oldchild, oldnode, xmlNodePtr, oldchildobj);
10221022

1023-
children = nodep->children;
1024-
if (!children) {
1023+
if (!nodep->children) {
10251024
RETURN_FALSE;
10261025
}
10271026

@@ -1043,42 +1042,32 @@ PHP_METHOD(DOMNode, replaceChild)
10431042
RETURN_FALSE;
10441043
}
10451044

1046-
/* check for the old child and whether the new child is already a child */
1047-
while (children) {
1048-
if (children == oldchild) {
1049-
foundoldchild = 1;
1050-
break;
1051-
}
1052-
children = children->next;
1045+
if (oldchild->parent != nodep) {
1046+
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
1047+
RETURN_FALSE;
10531048
}
10541049

1055-
if (foundoldchild) {
1056-
if (newchild->type == XML_DOCUMENT_FRAG_NODE) {
1057-
xmlNodePtr prevsib, nextsib;
1058-
prevsib = oldchild->prev;
1059-
nextsib = oldchild->next;
1050+
if (newchild->type == XML_DOCUMENT_FRAG_NODE) {
1051+
xmlNodePtr prevsib, nextsib;
1052+
prevsib = oldchild->prev;
1053+
nextsib = oldchild->next;
10601054

1061-
xmlUnlinkNode(oldchild);
1055+
xmlUnlinkNode(oldchild);
10621056

1063-
newchild = _php_dom_insert_fragment(nodep, prevsib, nextsib, newchild, intern, newchildobj);
1064-
if (newchild) {
1065-
dom_reconcile_ns(nodep->doc, newchild);
1066-
}
1067-
} else if (oldchild != newchild) {
1068-
if (newchild->doc == NULL && nodep->doc != NULL) {
1069-
xmlSetTreeDoc(newchild, nodep->doc);
1070-
newchildobj->document = intern->document;
1071-
php_libxml_increment_doc_ref((php_libxml_node_object *)newchildobj, NULL);
1072-
}
1073-
xmlReplaceNode(oldchild, newchild);
1057+
newchild = _php_dom_insert_fragment(nodep, prevsib, nextsib, newchild, intern, newchildobj);
1058+
if (newchild) {
10741059
dom_reconcile_ns(nodep->doc, newchild);
10751060
}
1076-
DOM_RET_OBJ(oldchild, &ret, intern);
1077-
return;
1078-
} else {
1079-
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
1080-
RETURN_FALSE;
1061+
} else if (oldchild != newchild) {
1062+
if (newchild->doc == NULL && nodep->doc != NULL) {
1063+
xmlSetTreeDoc(newchild, nodep->doc);
1064+
newchildobj->document = intern->document;
1065+
php_libxml_increment_doc_ref((php_libxml_node_object *)newchildobj, NULL);
1066+
}
1067+
xmlReplaceNode(oldchild, newchild);
1068+
dom_reconcile_ns(nodep->doc, newchild);
10811069
}
1070+
DOM_RET_OBJ(oldchild, &ret, intern);
10821071
}
10831072
/* }}} end dom_node_replace_child */
10841073

@@ -1088,7 +1077,7 @@ PHP_METHOD(DOMNode, replaceChild)
10881077
PHP_METHOD(DOMNode, removeChild)
10891078
{
10901079
zval *id, *node;
1091-
xmlNodePtr children, child, nodep;
1080+
xmlNodePtr child, nodep;
10921081
dom_object *intern, *childobj;
10931082
int ret, stricterror;
10941083

@@ -1113,23 +1102,13 @@ PHP_METHOD(DOMNode, removeChild)
11131102
RETURN_FALSE;
11141103
}
11151104

1116-
children = nodep->children;
1117-
if (!children) {
1105+
if (!nodep->children || child->parent != nodep) {
11181106
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
11191107
RETURN_FALSE;
11201108
}
11211109

1122-
while (children) {
1123-
if (children == child) {
1124-
xmlUnlinkNode(child);
1125-
DOM_RET_OBJ(child, &ret, intern);
1126-
return;
1127-
}
1128-
children = children->next;
1129-
}
1130-
1131-
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
1132-
RETURN_FALSE;
1110+
xmlUnlinkNode(child);
1111+
DOM_RET_OBJ(child, &ret, intern);
11331112
}
11341113
/* }}} end dom_node_remove_child */
11351114

‎ext/dom/tests/DOMNode_removeChild_error1.phpt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
removeChild() where the node is not a child
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$document = new DOMDocument();
8+
$real_parent = $document->createElement('real');
9+
$parent = $document->createElement('p');
10+
$child1 = $document->createElement('child1');
11+
$child2 = $document->createElement('child2');
12+
$real_parent->appendChild($child1);
13+
$parent->appendChild($child2);
14+
try {
15+
$parent->removeChild($child1);
16+
} catch (DOMException $e) {
17+
echo "DOMException: " . $e->getMessage();
18+
}
19+
--EXPECT--
20+
DOMException: Not Found Error

‎ext/dom/tests/DOMNode_replaceChild_error1.phpt‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
replaceChild() where the old node is not a child
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$document = new DOMDocument();
8+
$real_parent = $document->createElement('real');
9+
$parent = $document->createElement('p');
10+
$child1 = $document->createElement('child1');
11+
$child2 = $document->createElement('child2');
12+
$child3 = $document->createElement('child3');
13+
$real_parent->appendChild($child1);
14+
$parent->appendChild($child2);
15+
try {
16+
$parent->replaceChild($child3, $child1);
17+
} catch (DOMException $e) {
18+
echo "DOMException: " . $e->getMessage();
19+
}
20+
--EXPECT--
21+
DOMException: Not Found Error

‎ext/dom/tests/DOMNode_replaceChild_error2.phpt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
replaceChild() where the new node is a grandparent of the old node
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$document = new DOMDocument();
8+
$a = $document->createElement('a');
9+
$b = $document->createElement('b');
10+
$c = $document->createElement('c');
11+
$a->appendChild($b);
12+
$b->appendChild($c);
13+
try {
14+
$b->replaceChild($a, $c);
15+
} catch (DOMException $e) {
16+
echo "DOMException: " . $e->getMessage();
17+
}
18+
--EXPECT--
19+
DOMException: Hierarchy Request Error

0 commit comments

Comments
 (0)