From b1d8e240e688cae810c83b364772bf140ac45f42 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 3 Jun 2023 16:41:44 +0200
Subject: [PATCH 1/3] Fix bug #67440: append_node of a DOMDocumentFragment does
not reconcile namespaces
The test was amended from the original issue report. For the test:
Co-authored-by: php@deep-freeze.ca
The problem is that the regular dom_reconcile_ns() only works on a
single node. We actually have to reconciliate the whole tree in case a
fragment was added. This also required to move some code around such
that this special case could be handled separately.
Closes GH-11362.
---
NEWS | 2 +
ext/dom/node.c | 78 ++++++++++++-------
ext/dom/parentnode.c | 18 +++--
ext/dom/php_dom.c | 75 +++++++++++++-----
ext/dom/php_dom.h | 1 +
ext/dom/tests/bug67440.phpt | 151 ++++++++++++++++++++++++++++++++++++
6 files changed, 270 insertions(+), 55 deletions(-)
create mode 100644 ext/dom/tests/bug67440.phpt
diff --git a/NEWS b/NEWS
index f2cc5b1be96..8395e5233c8 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,8 @@ PHP NEWS
(nielsdos)
. Fixed bug GH-11347 (Memory leak when calling a static method inside an
xpath query). (nielsdos)
+ . Fixed bug #67440 (append_node of a DOMDocumentFragment does not reconcile
+ namespaces). (nielsdos)
- Opcache:
. Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)
diff --git a/ext/dom/node.c b/ext/dom/node.c
index b291ccc99a3..bcf4ee487d3 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -943,12 +943,20 @@ PHP_METHOD(DOMNode, insertBefore)
return;
}
}
- } else if (child->type == XML_DOCUMENT_FRAG_NODE) {
- new_child = _php_dom_insert_fragment(parentp, refp->prev, refp, child, intern, childobj);
- }
-
- if (new_child == NULL) {
new_child = xmlAddPrevSibling(refp, child);
+ if (UNEXPECTED(NULL == new_child)) {
+ goto cannot_add;
+ }
+ } else if (child->type == XML_DOCUMENT_FRAG_NODE) {
+ xmlNodePtr last = child->last;
+ new_child = _php_dom_insert_fragment(parentp, refp->prev, refp, child, intern, childobj);
+ dom_reconcile_ns_list(parentp->doc, new_child, last);
+ } else {
+ new_child = xmlAddPrevSibling(refp, child);
+ if (UNEXPECTED(NULL == new_child)) {
+ goto cannot_add;
+ }
+ dom_reconcile_ns(parentp->doc, new_child);
}
} else {
if (child->parent != NULL){
@@ -985,23 +993,28 @@ PHP_METHOD(DOMNode, insertBefore)
return;
}
}
- } else if (child->type == XML_DOCUMENT_FRAG_NODE) {
- new_child = _php_dom_insert_fragment(parentp, parentp->last, NULL, child, intern, childobj);
- }
- if (new_child == NULL) {
new_child = xmlAddChild(parentp, child);
+ if (UNEXPECTED(NULL == new_child)) {
+ goto cannot_add;
+ }
+ } else if (child->type == XML_DOCUMENT_FRAG_NODE) {
+ xmlNodePtr last = child->last;
+ new_child = _php_dom_insert_fragment(parentp, parentp->last, NULL, child, intern, childobj);
+ dom_reconcile_ns_list(parentp->doc, new_child, last);
+ } else {
+ new_child = xmlAddChild(parentp, child);
+ if (UNEXPECTED(NULL == new_child)) {
+ goto cannot_add;
+ }
+ dom_reconcile_ns(parentp->doc, new_child);
}
}
- if (NULL == new_child) {
- zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
- RETURN_THROWS();
- }
-
- dom_reconcile_ns(parentp->doc, new_child);
-
DOM_RET_OBJ(new_child, &ret, intern);
-
+ return;
+cannot_add:
+ zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
+ RETURN_THROWS();
}
/* }}} end dom_node_insert_before */
@@ -1066,9 +1079,10 @@ PHP_METHOD(DOMNode, replaceChild)
xmlUnlinkNode(oldchild);
+ xmlNodePtr last = newchild->last;
newchild = _php_dom_insert_fragment(nodep, prevsib, nextsib, newchild, intern, newchildobj);
if (newchild) {
- dom_reconcile_ns(nodep->doc, newchild);
+ dom_reconcile_ns_list(nodep->doc, newchild, last);
}
} else if (oldchild != newchild) {
xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc);
@@ -1215,22 +1229,28 @@ PHP_METHOD(DOMNode, appendChild)
php_libxml_node_free_resource((xmlNodePtr) lastattr);
}
}
- } else if (child->type == XML_DOCUMENT_FRAG_NODE) {
- new_child = _php_dom_insert_fragment(nodep, nodep->last, NULL, child, intern, childobj);
- }
-
- if (new_child == NULL) {
new_child = xmlAddChild(nodep, child);
- if (new_child == NULL) {
- // TODO Convert to Error?
- php_error_docref(NULL, E_WARNING, "Couldn't append node");
- RETURN_FALSE;
+ if (UNEXPECTED(new_child == NULL)) {
+ goto cannot_add;
}
+ } else if (child->type == XML_DOCUMENT_FRAG_NODE) {
+ xmlNodePtr last = child->last;
+ new_child = _php_dom_insert_fragment(nodep, nodep->last, NULL, child, intern, childobj);
+ dom_reconcile_ns_list(nodep->doc, new_child, last);
+ } else {
+ new_child = xmlAddChild(nodep, child);
+ if (UNEXPECTED(new_child == NULL)) {
+ goto cannot_add;
+ }
+ dom_reconcile_ns(nodep->doc, new_child);
}
- dom_reconcile_ns(nodep->doc, new_child);
-
DOM_RET_OBJ(new_child, &ret, intern);
+ return;
+cannot_add:
+ // TODO Convert to Error?
+ php_error_docref(NULL, E_WARNING, "Couldn't append node");
+ RETURN_FALSE;
}
/* }}} end dom_node_append_child */
diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c
index c99a2a5a662..b7e8e3ba774 100644
--- a/ext/dom/parentnode.c
+++ b/ext/dom/parentnode.c
@@ -298,13 +298,14 @@ void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc)
parentNode->children = newchild;
}
- parentNode->last = fragment->last;
+ xmlNodePtr last = fragment->last;
+ parentNode->last = last;
newchild->prev = prevsib;
dom_fragment_assign_parent_node(parentNode, fragment);
- dom_reconcile_ns(parentNode->doc, newchild);
+ dom_reconcile_ns_list(parentNode->doc, newchild, last);
}
xmlFree(fragment);
@@ -335,13 +336,14 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc)
nextsib = parentNode->children;
if (newchild) {
+ xmlNodePtr last = fragment->last;
parentNode->children = newchild;
fragment->last->next = nextsib;
- nextsib->prev = fragment->last;
+ nextsib->prev = last;
dom_fragment_assign_parent_node(parentNode, fragment);
- dom_reconcile_ns(parentNode->doc, newchild);
+ dom_reconcile_ns_list(parentNode->doc, newchild, last);
}
xmlFree(fragment);
@@ -414,11 +416,13 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
newchild = fragment->children;
if (newchild) {
+ xmlNodePtr last = fragment->last;
+
/* Step 5: place fragment into the parent before viable_next_sibling */
dom_pre_insert(viable_next_sibling, parentNode, newchild, fragment);
dom_fragment_assign_parent_node(parentNode, fragment);
- dom_reconcile_ns(doc, newchild);
+ dom_reconcile_ns_list(doc, newchild, last);
}
xmlFree(fragment);
@@ -463,6 +467,8 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
newchild = fragment->children;
if (newchild) {
+ xmlNodePtr last = fragment->last;
+
/* Step 5: if viable_previous_sibling is null, set it to the parent's first child, otherwise viable_previous_sibling's next sibling */
if (!viable_previous_sibling) {
viable_previous_sibling = parentNode->children;
@@ -473,7 +479,7 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
dom_pre_insert(viable_previous_sibling, parentNode, newchild, fragment);
dom_fragment_assign_parent_node(parentNode, fragment);
- dom_reconcile_ns(doc, newchild);
+ dom_reconcile_ns_list(doc, newchild, last);
}
xmlFree(fragment);
diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c
index 1883767d2e4..df20093221f 100644
--- a/ext/dom/php_dom.c
+++ b/ext/dom/php_dom.c
@@ -1385,38 +1385,73 @@ void dom_set_old_ns(xmlDoc *doc, xmlNs *ns) {
}
/* }}} end dom_set_old_ns */
-void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
+static void dom_reconcile_ns_internal(xmlDocPtr doc, xmlNodePtr nodep)
{
xmlNsPtr nsptr, nsdftptr, curns, prevns = NULL;
- if (nodep->type == XML_ELEMENT_NODE) {
- /* Following if block primarily used for inserting nodes created via createElementNS */
- if (nodep->nsDef != NULL) {
- curns = nodep->nsDef;
- while (curns) {
- nsdftptr = curns->next;
- if (curns->href != NULL) {
- if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) &&
- (curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) {
- curns->next = NULL;
- if (prevns == NULL) {
- nodep->nsDef = nsdftptr;
- } else {
- prevns->next = nsdftptr;
- }
- dom_set_old_ns(doc, curns);
- curns = prevns;
+ /* Following if block primarily used for inserting nodes created via createElementNS */
+ if (nodep->nsDef != NULL) {
+ curns = nodep->nsDef;
+ while (curns) {
+ nsdftptr = curns->next;
+ if (curns->href != NULL) {
+ if((nsptr = xmlSearchNsByHref(doc, nodep->parent, curns->href)) &&
+ (curns->prefix == NULL || xmlStrEqual(nsptr->prefix, curns->prefix))) {
+ curns->next = NULL;
+ if (prevns == NULL) {
+ nodep->nsDef = nsdftptr;
+ } else {
+ prevns->next = nsdftptr;
}
+ dom_set_old_ns(doc, curns);
+ curns = prevns;
}
- prevns = curns;
- curns = nsdftptr;
}
+ prevns = curns;
+ curns = nsdftptr;
}
+ }
+}
+
+void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep) /* {{{ */
+{
+ if (nodep->type == XML_ELEMENT_NODE) {
+ dom_reconcile_ns_internal(doc, nodep);
xmlReconciliateNs(doc, nodep);
}
}
/* }}} */
+static void dom_reconcile_ns_list_internal(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last)
+{
+ ZEND_ASSERT(nodep != NULL);
+ while (true) {
+ if (nodep->type == XML_ELEMENT_NODE) {
+ dom_reconcile_ns_internal(doc, nodep);
+ if (nodep->children) {
+ dom_reconcile_ns_list_internal(doc, nodep->children, nodep->last /* process the whole children list */);
+ }
+ }
+ if (nodep == last) {
+ break;
+ }
+ nodep = nodep->next;
+ }
+}
+
+void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last)
+{
+ dom_reconcile_ns_list_internal(doc, nodep, last);
+ /* Outside of the recursion above because xmlReconciliateNs() performs its own recursion. */
+ while (true) {
+ xmlReconciliateNs(doc, nodep);
+ if (nodep == last) {
+ break;
+ }
+ nodep = nodep->next;
+ }
+}
+
/*
http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#ID-DocCrElNS
diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h
index fdfdd4e7a31..924d1397ca7 100644
--- a/ext/dom/php_dom.h
+++ b/ext/dom/php_dom.h
@@ -110,6 +110,7 @@ int dom_check_qname(char *qname, char **localname, char **prefix, int uri_len, i
xmlNsPtr dom_get_ns(xmlNodePtr node, char *uri, int *errorcode, char *prefix);
void dom_set_old_ns(xmlDoc *doc, xmlNs *ns);
void dom_reconcile_ns(xmlDocPtr doc, xmlNodePtr nodep);
+void dom_reconcile_ns_list(xmlDocPtr doc, xmlNodePtr nodep, xmlNodePtr last);
xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName);
void dom_normalize (xmlNodePtr nodep);
xmlNode *dom_get_elements_by_tag_name_ns_raw(xmlNodePtr nodep, char *ns, char *local, int *cur, int index);
diff --git a/ext/dom/tests/bug67440.phpt b/ext/dom/tests/bug67440.phpt
new file mode 100644
index 00000000000..3e30f69b9ae
--- /dev/null
+++ b/ext/dom/tests/bug67440.phpt
@@ -0,0 +1,151 @@
+--TEST--
+Bug #67440 (append_node of a DOMDocumentFragment does not reconcile namespaces)
+--EXTENSIONS--
+dom
+--FILE--
+loadXML('');
+ $fragment = $document->createDocumentFragment();
+ $fragment->appendChild($document->createTextNode("\n"));
+ $fragment->appendChild($document->createElementNS('http://example/ns', 'myns:childNode', '1'));
+ $fragment->appendChild($document->createTextNode("\n"));
+ $fragment->appendChild($document->createElementNS('http://example/ns', 'myns:childNode', '2'));
+ $fragment->appendChild($document->createTextNode("\n"));
+ return array($document, $fragment);
+}
+
+function case1($method) {
+ list($document, $fragment) = createDocument();
+ $document->documentElement->{$method}($fragment);
+ echo $document->saveXML();
+}
+
+function case2($method) {
+ list($document, $fragment) = createDocument();
+ $childNodes = iterator_to_array($fragment->childNodes);
+ foreach ($childNodes as $childNode) {
+ $document->documentElement->{$method}($childNode);
+ }
+ echo $document->saveXML();
+}
+
+function case3($method) {
+ list($document, $fragment) = createDocument();
+ $fragment->removeChild($fragment->firstChild);
+ $document->documentElement->{$method}($fragment);
+ echo $document->saveXML();
+}
+
+function case4($method) {
+ list($document, $fragment) = createDocument();
+ $fragment->childNodes[1]->appendChild($document->createElementNS('http://example/ns2', 'myns2:childNode', '3'));
+ $document->documentElement->{$method}($fragment);
+ echo $document->saveXML();
+}
+
+echo "== appendChild ==\n";
+echo "-- fragment to document element --\n"; case1('appendChild'); echo "\n";
+echo "-- children manually document element --\n"; case2('appendChild'); echo "\n";
+echo "-- fragment to document where first element is not a text node --\n"; case3('appendChild'); echo "\n";
+echo "-- fragment with namespace declarations in children --\n"; case4('appendChild'); echo "\n";
+
+echo "== insertBefore ==\n";
+echo "-- fragment to document element --\n"; case1('insertBefore'); echo "\n";
+echo "-- children manually document element --\n"; case2('insertBefore'); echo "\n";
+echo "-- fragment to document where first element is not a text node --\n"; case3('insertBefore'); echo "\n";
+echo "-- fragment with namespace declarations in children --\n"; case4('insertBefore'); echo "\n";
+
+echo "== insertAfter ==\n";
+echo "-- fragment to document element --\n"; case1('insertBefore'); echo "\n";
+echo "-- children manually document element --\n"; case2('insertBefore'); echo "\n";
+echo "-- fragment to document where first element is not a text node --\n"; case3('insertBefore'); echo "\n";
+echo "-- fragment with namespace declarations in children --\n"; case4('insertBefore'); echo "\n";
+
+?>
+--EXPECT--
+== appendChild ==
+-- fragment to document element --
+
+
+1
+2
+
+
+-- children manually document element --
+
+
+1
+2
+
+
+-- fragment to document where first element is not a text node --
+
+1
+2
+
+
+-- fragment with namespace declarations in children --
+
+
+13
+2
+
+
+== insertBefore ==
+-- fragment to document element --
+
+
+1
+2
+
+
+-- children manually document element --
+
+
+1
+2
+
+
+-- fragment to document where first element is not a text node --
+
+1
+2
+
+
+-- fragment with namespace declarations in children --
+
+
+13
+2
+
+
+== insertAfter ==
+-- fragment to document element --
+
+
+1
+2
+
+
+-- children manually document element --
+
+
+1
+2
+
+
+-- fragment to document where first element is not a text node --
+
+1
+2
+
+
+-- fragment with namespace declarations in children --
+
+
+13
+2
+
From 23f70025270c040e0d378b210120d9824af10ea6 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 3 Jun 2023 17:54:37 +0200
Subject: [PATCH 2/3] Fix bug #81642: DOMChildNode::replaceWith() bug when
replacing a node with itself
Closes GH-11363.
---
NEWS | 2 +
ext/dom/element.c | 5 +--
ext/dom/parentnode.c | 81 +++++++++++++++++++++++++++++--------
ext/dom/php_dom.h | 1 +
ext/dom/tests/bug81642.phpt | 49 ++++++++++++++++++++++
5 files changed, 118 insertions(+), 20 deletions(-)
create mode 100644 ext/dom/tests/bug81642.phpt
diff --git a/NEWS b/NEWS
index 8395e5233c8..39605d0d3c4 100644
--- 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
index 19cef583465..78113d72776 100644
--- 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
index b7e8e3ba774..a9dfda59622 100644
--- a/ext/dom/parentnode.c
+++ b/ext/dom/parentnode.c
@@ -485,6 +485,32 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
xmlFree(fragment);
}
+static zend_result dom_child_removal_preconditions(const xmlNodePtr child, int stricterror)
+{
+ 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 FAILURE;
+ }
+
+ if (!child->parent) {
+ php_dom_throw_error(NOT_FOUND_ERR, stricterror);
+ return FAILURE;
+ }
+
+ if (dom_node_children_valid(child->parent) == FAILURE) {
+ return FAILURE;
+ }
+
+ 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);
@@ -493,27 +519,11 @@ void dom_child_node_remove(dom_object *context)
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;
- }
-
- if (!child->parent) {
- php_dom_throw_error(NOT_FOUND_ERR, stricterror);
- return;
- }
-
- if (dom_node_children_valid(child->parent) == FAILURE) {
+ if (UNEXPECTED(dom_child_removal_preconditions(child, stricterror) != SUCCESS)) {
return;
}
children = child->parent->children;
- if (!children) {
- php_dom_throw_error(NOT_FOUND_ERR, stricterror);
- return;
- }
-
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
index 924d1397ca7..ac23d1fc25b 100644
--- 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
new file mode 100644
index 00000000000..7bf3dde5058
--- /dev/null
+++ 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--
+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) "
+
+"
+string(37) "
+
+
+"
+string(34) "
+
+foo
+"
+string(38) "
+bar
+
+foo
+"
From 0e34ac864a20bd03a35741db09f0bdf72ae56874 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sun, 4 Jun 2023 15:13:37 +0200
Subject: [PATCH 3/3] Fix bug #77686: Removed elements are still returned by
getElementById
From the moment an ID is created, libxml2's behaviour is to cache that element,
even if that element is not yet attached to the document. Similarly, only upon
destruction of the element the ID is actually removed by libxml2.
Since libxml2 has such behaviour deeply ingrained in the library, and uses the
cache for various purposes, it seems like a bad idea and lost cause to fight it.
Instead, we'll simply walk the tree upwards to check if the node is attached to
the document.
Closes GH-11369.
---
NEWS | 2 ++
ext/dom/document.c | 21 ++++++++++++++++++-
ext/dom/tests/bug77686.phpt | 40 +++++++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 1 deletion(-)
create mode 100644 ext/dom/tests/bug77686.phpt
diff --git a/NEWS b/NEWS
index 39605d0d3c4..122e4a48b86 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,8 @@ PHP NEWS
namespaces). (nielsdos)
. Fixed bug #81642 (DOMChildNode::replaceWith() bug when replacing a node
with itself). (nielsdos)
+ . Fixed bug #77686 (Removed elements are still returned by getElementById).
+ (nielsdos)
- Opcache:
. Fix allocation loop in zend_shared_alloc_startup(). (nielsdos)
diff --git a/ext/dom/document.c b/ext/dom/document.c
index c60198a3be1..93091df83a0 100644
--- a/ext/dom/document.c
+++ b/ext/dom/document.c
@@ -1008,6 +1008,19 @@ PHP_METHOD(DOMDocument, getElementsByTagNameNS)
}
/* }}} end dom_document_get_elements_by_tag_name_ns */
+static bool php_dom_is_node_attached(const xmlNode *node)
+{
+ ZEND_ASSERT(node != NULL);
+ node = node->parent;
+ while (node != NULL) {
+ if (node->type == XML_DOCUMENT_NODE || node->type == XML_HTML_DOCUMENT_NODE) {
+ return true;
+ }
+ node = node->parent;
+ }
+ return false;
+}
+
/* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-getElBId
Since: DOM Level 2
*/
@@ -1030,7 +1043,13 @@ PHP_METHOD(DOMDocument, getElementById)
attrp = xmlGetID(docp, (xmlChar *) idname);
- if (attrp && attrp->parent) {
+ /* From the moment an ID is created, libxml2's behaviour is to cache that element, even
+ * if that element is not yet attached to the document. Similarly, only upon destruction of
+ * the element the ID is actually removed by libxml2. Since libxml2 has such behaviour deeply
+ * ingrained in the library, and uses the cache for various purposes, it seems like a bad
+ * idea and lost cause to fight it. Instead, we'll simply walk the tree upwards to check
+ * if the node is attached to the document. */
+ if (attrp && attrp->parent && php_dom_is_node_attached(attrp->parent)) {
DOM_RET_OBJ((xmlNodePtr) attrp->parent, &ret, intern);
} else {
RETVAL_NULL();
diff --git a/ext/dom/tests/bug77686.phpt b/ext/dom/tests/bug77686.phpt
new file mode 100644
index 00000000000..ddd7c336478
--- /dev/null
+++ b/ext/dom/tests/bug77686.phpt
@@ -0,0 +1,40 @@
+--TEST--
+Bug #77686 (Removed elements are still returned by getElementById)
+--EXTENSIONS--
+dom
+--FILE--
+loadHTML('beforehello
after');
+$body = $doc->getElementById('x');
+$div = $doc->getElementById('y');
+var_dump($doc->getElementById('y')->textContent);
+
+// Detached from document, should not find it anymore
+$body->removeChild($div);
+var_dump($doc->getElementById('y'));
+
+// Added again, should find it
+$body->appendChild($div);
+var_dump($doc->getElementById('y')->textContent);
+
+// Should find root element without a problem
+var_dump($doc->getElementById('htmlelement')->textContent);
+
+// Created element but not yet attached, should not find it before it is added
+$new_element = $doc->createElement('p');
+$new_element->textContent = 'my new text';
+$new_element->setAttribute('id', 'myp');
+var_dump($doc->getElementById('myp'));
+$body->appendChild($new_element);
+var_dump($doc->getElementById('myp')->textContent);
+
+?>
+--EXPECT--
+string(5) "hello"
+NULL
+string(5) "hello"
+string(16) "beforeafterhello"
+NULL
+string(11) "my new text"