Skip to content

Commit 5b79c53

Browse files
committed
Merge branch 'PHP-8.1' into PHP-8.2
* PHP-8.1: Fix bug #77686: Removed elements are still returned by getElementById Fix bug #81642: DOMChildNode::replaceWith() bug when replacing a node with itself Fix bug #67440: append_node of a DOMDocumentFragment does not reconcile namespaces
2 parents 8126bea + 0e34ac8 commit 5b79c53

File tree

10 files changed

+438
-64
lines changed

10 files changed

+438
-64
lines changed

NEWS

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ PHP NEWS
2121
(nielsdos)
2222
. Fixed bug GH-11347 (Memory leak when calling a static method inside an
2323
xpath query). (nielsdos)
24+
. Fixed bug #67440 (append_node of a DOMDocumentFragment does not reconcile
25+
namespaces). (nielsdos)
26+
. Fixed bug #81642 (DOMChildNode::replaceWith() bug when replacing a node
27+
with itself). (nielsdos)
28+
. Fixed bug #77686 (Removed elements are still returned by getElementById).
29+
(nielsdos)
2430

2531
- Opcache:
2632
. Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)

ext/dom/document.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,19 @@ PHP_METHOD(DOMDocument, getElementsByTagNameNS)
10081008
}
10091009
/* }}} end dom_document_get_elements_by_tag_name_ns */
10101010

1011+
static bool php_dom_is_node_attached(const xmlNode *node)
1012+
{
1013+
ZEND_ASSERT(node != NULL);
1014+
node = node->parent;
1015+
while (node != NULL) {
1016+
if (node->type == XML_DOCUMENT_NODE || node->type == XML_HTML_DOCUMENT_NODE) {
1017+
return true;
1018+
}
1019+
node = node->parent;
1020+
}
1021+
return false;
1022+
}
1023+
10111024
/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-getElBId
10121025
Since: DOM Level 2
10131026
*/
@@ -1030,7 +1043,13 @@ PHP_METHOD(DOMDocument, getElementById)
10301043

10311044
attrp = xmlGetID(docp, (xmlChar *) idname);
10321045

1033-
if (attrp && attrp->parent) {
1046+
/* From the moment an ID is created, libxml2's behaviour is to cache that element, even
1047+
* if that element is not yet attached to the document. Similarly, only upon destruction of
1048+
* the element the ID is actually removed by libxml2. Since libxml2 has such behaviour deeply
1049+
* ingrained in the library, and uses the cache for various purposes, it seems like a bad
1050+
* idea and lost cause to fight it. Instead, we'll simply walk the tree upwards to check
1051+
* if the node is attached to the document. */
1052+
if (attrp && attrp->parent && php_dom_is_node_attached(attrp->parent)) {
10341053
DOM_RET_OBJ((xmlNodePtr) attrp->parent, &ret, intern);
10351054
} else {
10361055
RETVAL_NULL();

ext/dom/element.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,7 +1234,7 @@ PHP_METHOD(DOMElement, prepend)
12341234
}
12351235
/* }}} end DOMElement::prepend */
12361236

1237-
/* {{{ URL: https://dom.spec.whatwg.org/#dom-parentnode-prepend
1237+
/* {{{ URL: https://dom.spec.whatwg.org/#dom-parentnode-replacechildren
12381238
Since: DOM Living Standard (DOM4)
12391239
*/
12401240
PHP_METHOD(DOMElement, replaceWith)
@@ -1251,8 +1251,7 @@ PHP_METHOD(DOMElement, replaceWith)
12511251
id = ZEND_THIS;
12521252
DOM_GET_OBJ(context, id, xmlNodePtr, intern);
12531253

1254-
dom_parent_node_after(intern, args, argc);
1255-
dom_child_node_remove(intern);
1254+
dom_child_replace_with(intern, args, argc);
12561255
}
12571256
/* }}} end DOMElement::prepend */
12581257

ext/dom/node.c

Lines changed: 43 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -943,12 +943,20 @@ PHP_METHOD(DOMNode, insertBefore)
943943
return;
944944
}
945945
}
946+
new_child = xmlAddPrevSibling(refp, child);
947+
if (UNEXPECTED(NULL == new_child)) {
948+
goto cannot_add;
949+
}
946950
} else if (child->type == XML_DOCUMENT_FRAG_NODE) {
951+
xmlNodePtr last = child->last;
947952
new_child = _php_dom_insert_fragment(parentp, refp->prev, refp, child, intern, childobj);
948-
}
949-
950-
if (new_child == NULL) {
953+
dom_reconcile_ns_list(parentp->doc, new_child, last);
954+
} else {
951955
new_child = xmlAddPrevSibling(refp, child);
956+
if (UNEXPECTED(NULL == new_child)) {
957+
goto cannot_add;
958+
}
959+
dom_reconcile_ns(parentp->doc, new_child);
952960
}
953961
} else {
954962
if (child->parent != NULL){
@@ -985,23 +993,28 @@ PHP_METHOD(DOMNode, insertBefore)
985993
return;
986994
}
987995
}
996+
new_child = xmlAddChild(parentp, child);
997+
if (UNEXPECTED(NULL == new_child)) {
998+
goto cannot_add;
999+
}
9881000
} else if (child->type == XML_DOCUMENT_FRAG_NODE) {
1001+
xmlNodePtr last = child->last;
9891002
new_child = _php_dom_insert_fragment(parentp, parentp->last, NULL, child, intern, childobj);
990-
}
991-
if (new_child == NULL) {
1003+
dom_reconcile_ns_list(parentp->doc, new_child, last);
1004+
} else {
9921005
new_child = xmlAddChild(parentp, child);
1006+
if (UNEXPECTED(NULL == new_child)) {
1007+
goto cannot_add;
1008+
}
1009+
dom_reconcile_ns(parentp->doc, new_child);
9931010
}
9941011
}
9951012

996-
if (NULL == new_child) {
997-
zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
998-
RETURN_THROWS();
999-
}
1000-
1001-
dom_reconcile_ns(parentp->doc, new_child);
1002-
10031013
DOM_RET_OBJ(new_child, &ret, intern);
1004-
1014+
return;
1015+
cannot_add:
1016+
zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
1017+
RETURN_THROWS();
10051018
}
10061019
/* }}} end dom_node_insert_before */
10071020

@@ -1066,9 +1079,10 @@ PHP_METHOD(DOMNode, replaceChild)
10661079

10671080
xmlUnlinkNode(oldchild);
10681081

1082+
xmlNodePtr last = newchild->last;
10691083
newchild = _php_dom_insert_fragment(nodep, prevsib, nextsib, newchild, intern, newchildobj);
10701084
if (newchild) {
1071-
dom_reconcile_ns(nodep->doc, newchild);
1085+
dom_reconcile_ns_list(nodep->doc, newchild, last);
10721086
}
10731087
} else if (oldchild != newchild) {
10741088
xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc);
@@ -1215,22 +1229,28 @@ PHP_METHOD(DOMNode, appendChild)
12151229
php_libxml_node_free_resource((xmlNodePtr) lastattr);
12161230
}
12171231
}
1232+
new_child = xmlAddChild(nodep, child);
1233+
if (UNEXPECTED(new_child == NULL)) {
1234+
goto cannot_add;
1235+
}
12181236
} else if (child->type == XML_DOCUMENT_FRAG_NODE) {
1237+
xmlNodePtr last = child->last;
12191238
new_child = _php_dom_insert_fragment(nodep, nodep->last, NULL, child, intern, childobj);
1220-
}
1221-
1222-
if (new_child == NULL) {
1239+
dom_reconcile_ns_list(nodep->doc, new_child, last);
1240+
} else {
12231241
new_child = xmlAddChild(nodep, child);
1224-
if (new_child == NULL) {
1225-
// TODO Convert to Error?
1226-
php_error_docref(NULL, E_WARNING, "Couldn't append node");
1227-
RETURN_FALSE;
1242+
if (UNEXPECTED(new_child == NULL)) {
1243+
goto cannot_add;
12281244
}
1245+
dom_reconcile_ns(nodep->doc, new_child);
12291246
}
12301247

1231-
dom_reconcile_ns(nodep->doc, new_child);
1232-
12331248
DOM_RET_OBJ(new_child, &ret, intern);
1249+
return;
1250+
cannot_add:
1251+
// TODO Convert to Error?
1252+
php_error_docref(NULL, E_WARNING, "Couldn't append node");
1253+
RETURN_FALSE;
12341254
}
12351255
/* }}} end dom_node_append_child */
12361256

ext/dom/parentnode.c

Lines changed: 70 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -298,13 +298,14 @@ void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc)
298298
parentNode->children = newchild;
299299
}
300300

301-
parentNode->last = fragment->last;
301+
xmlNodePtr last = fragment->last;
302+
parentNode->last = last;
302303

303304
newchild->prev = prevsib;
304305

305306
dom_fragment_assign_parent_node(parentNode, fragment);
306307

307-
dom_reconcile_ns(parentNode->doc, newchild);
308+
dom_reconcile_ns_list(parentNode->doc, newchild, last);
308309
}
309310

310311
xmlFree(fragment);
@@ -335,13 +336,14 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc)
335336
nextsib = parentNode->children;
336337

337338
if (newchild) {
339+
xmlNodePtr last = fragment->last;
338340
parentNode->children = newchild;
339341
fragment->last->next = nextsib;
340-
nextsib->prev = fragment->last;
342+
nextsib->prev = last;
341343

342344
dom_fragment_assign_parent_node(parentNode, fragment);
343345

344-
dom_reconcile_ns(parentNode->doc, newchild);
346+
dom_reconcile_ns_list(parentNode->doc, newchild, last);
345347
}
346348

347349
xmlFree(fragment);
@@ -414,11 +416,13 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
414416
newchild = fragment->children;
415417

416418
if (newchild) {
419+
xmlNodePtr last = fragment->last;
420+
417421
/* Step 5: place fragment into the parent before viable_next_sibling */
418422
dom_pre_insert(viable_next_sibling, parentNode, newchild, fragment);
419423

420424
dom_fragment_assign_parent_node(parentNode, fragment);
421-
dom_reconcile_ns(doc, newchild);
425+
dom_reconcile_ns_list(doc, newchild, last);
422426
}
423427

424428
xmlFree(fragment);
@@ -463,6 +467,8 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
463467
newchild = fragment->children;
464468

465469
if (newchild) {
470+
xmlNodePtr last = fragment->last;
471+
466472
/* Step 5: if viable_previous_sibling is null, set it to the parent's first child, otherwise viable_previous_sibling's next sibling */
467473
if (!viable_previous_sibling) {
468474
viable_previous_sibling = parentNode->children;
@@ -473,41 +479,51 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
473479
dom_pre_insert(viable_previous_sibling, parentNode, newchild, fragment);
474480

475481
dom_fragment_assign_parent_node(parentNode, fragment);
476-
dom_reconcile_ns(doc, newchild);
482+
dom_reconcile_ns_list(doc, newchild, last);
477483
}
478484

479485
xmlFree(fragment);
480486
}
481487

482-
void dom_child_node_remove(dom_object *context)
488+
static zend_result dom_child_removal_preconditions(const xmlNodePtr child, int stricterror)
483489
{
484-
xmlNode *child = dom_object_get_node(context);
485-
xmlNodePtr children;
486-
int stricterror;
487-
488-
stricterror = dom_get_strict_error(context->document);
489-
490490
if (dom_node_is_read_only(child) == SUCCESS ||
491491
(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
492492
php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
493-
return;
493+
return FAILURE;
494494
}
495495

496496
if (!child->parent) {
497497
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
498-
return;
498+
return FAILURE;
499499
}
500500

501501
if (dom_node_children_valid(child->parent) == FAILURE) {
502-
return;
502+
return FAILURE;
503503
}
504504

505-
children = child->parent->children;
505+
xmlNodePtr children = child->parent->children;
506506
if (!children) {
507507
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
508+
return FAILURE;
509+
}
510+
511+
return SUCCESS;
512+
}
513+
514+
void dom_child_node_remove(dom_object *context)
515+
{
516+
xmlNode *child = dom_object_get_node(context);
517+
xmlNodePtr children;
518+
int stricterror;
519+
520+
stricterror = dom_get_strict_error(context->document);
521+
522+
if (UNEXPECTED(dom_child_removal_preconditions(child, stricterror) != SUCCESS)) {
508523
return;
509524
}
510525

526+
children = child->parent->children;
511527
while (children) {
512528
if (children == child) {
513529
xmlUnlinkNode(child);
@@ -519,4 +535,41 @@ void dom_child_node_remove(dom_object *context)
519535
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
520536
}
521537

538+
void dom_child_replace_with(dom_object *context, zval *nodes, int nodesc)
539+
{
540+
xmlNodePtr child = dom_object_get_node(context);
541+
xmlNodePtr parentNode = child->parent;
542+
543+
int stricterror = dom_get_strict_error(context->document);
544+
if (UNEXPECTED(dom_child_removal_preconditions(child, stricterror) != SUCCESS)) {
545+
return;
546+
}
547+
548+
xmlNodePtr insertion_point = child->next;
549+
550+
xmlNodePtr fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
551+
if (UNEXPECTED(fragment == NULL)) {
552+
return;
553+
}
554+
555+
xmlNodePtr newchild = fragment->children;
556+
xmlDocPtr doc = parentNode->doc;
557+
558+
if (newchild) {
559+
xmlNodePtr last = fragment->last;
560+
561+
/* Unlink and free it unless it became a part of the fragment. */
562+
if (child->parent != fragment) {
563+
xmlUnlinkNode(child);
564+
}
565+
566+
dom_pre_insert(insertion_point, parentNode, newchild, fragment);
567+
568+
dom_fragment_assign_parent_node(parentNode, fragment);
569+
dom_reconcile_ns_list(doc, newchild, last);
570+
}
571+
572+
xmlFree(fragment);
573+
}
574+
522575
#endif

0 commit comments

Comments
 (0)