diff --git a/NEWS b/NEWS index d9b735d9830..c5f24c90b17 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ PHP NEWS - DOM: . adoptNode now respects the strict error checking property. (nielsdos) + . Align DOMChildNode parent checks with spec. (nielsdos) - Opcache: . Avoid resetting JIT counter handlers from multiple processes/threads. diff --git a/UPGRADING b/UPGRADING index 96303520018..1aa82831d52 100644 --- a/UPGRADING +++ b/UPGRADING @@ -46,6 +46,14 @@ PHP 8.3 UPGRADE NOTES iterating over the WeakMap (reachability via iteration is considered weak). Previously, such entries would never be automatically removed. +- DOM: + . DOMChildNode::after(), DOMChildNode::before(), DOMChildNode::replaceWith() + on a node that has no parent is now a no-op instead of a hierarchy exception, + which is the behaviour spec demands. + . Using the DOMParentNode and DOMChildNode methods without a document now works + instead of throwing a HIERARCHY_REQUEST_ERR DOMException. This is in line with + the behaviour spec demands. + - FFI: . C functions that have a return type of void now return null instead of returning the following object object(FFI\CData:void) { } diff --git a/ext/dom/parentnode.c b/ext/dom/parentnode.c index b6ed24665c3..fe928797a1d 100644 --- a/ext/dom/parentnode.c +++ b/ext/dom/parentnode.c @@ -240,8 +240,8 @@ static void dom_fragment_assign_parent_node(xmlNodePtr parentNode, xmlNodePtr fr static zend_result dom_sanity_check_node_list_for_insertion(php_libxml_ref_obj *document, xmlNodePtr parentNode, zval *nodes, int nodesc) { - if (document == NULL) { - php_dom_throw_error(HIERARCHY_REQUEST_ERR, 1); + if (UNEXPECTED(parentNode == NULL)) { + /* No error required, this must be a no-op per spec */ return FAILURE; } @@ -391,10 +391,9 @@ void dom_parent_node_after(dom_object *context, zval *nodes, uint32_t nodesc) /* Spec step 1 */ parentNode = prevsib->parent; - /* Spec step 2 */ - if (!parentNode) { - int stricterror = dom_get_strict_error(context->document); - php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); + + /* Sanity check for fragment, includes spec step 2 */ + if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) { return; } @@ -409,10 +408,6 @@ void dom_parent_node_after(dom_object *context, zval *nodes, uint32_t nodesc) doc = prevsib->doc; - if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) { - return; - } - php_libxml_invalidate_node_list_cache_from_doc(doc); /* Spec step 4: convert nodes into fragment */ @@ -448,10 +443,9 @@ void dom_parent_node_before(dom_object *context, zval *nodes, uint32_t nodesc) /* Spec step 1 */ parentNode = nextsib->parent; - /* Spec step 2 */ - if (!parentNode) { - int stricterror = dom_get_strict_error(context->document); - php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); + + /* Sanity check for fragment, includes spec step 2 */ + if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) { return; } @@ -466,10 +460,6 @@ void dom_parent_node_before(dom_object *context, zval *nodes, uint32_t nodesc) doc = nextsib->doc; - if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) { - return; - } - php_libxml_invalidate_node_list_cache_from_doc(doc); /* Spec step 4: convert nodes into fragment */ @@ -537,7 +527,7 @@ void dom_child_node_remove(dom_object *context) return; } - php_libxml_invalidate_node_list_cache_from_doc(context->document->ptr); + php_libxml_invalidate_node_list_cache_from_doc(child->doc); xmlUnlinkNode(child); } @@ -550,10 +540,9 @@ void dom_child_replace_with(dom_object *context, zval *nodes, uint32_t nodesc) /* Spec step 1 */ xmlNodePtr parentNode = child->parent; - /* Spec step 2 */ - if (!parentNode) { - int stricterror = dom_get_strict_error(context->document); - php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror); + + /* Sanity check for fragment, includes spec step 2 */ + if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) { return; } @@ -571,11 +560,8 @@ void dom_child_replace_with(dom_object *context, zval *nodes, uint32_t nodesc) viable_next_sibling = viable_next_sibling->next; } - if (UNEXPECTED(dom_sanity_check_node_list_for_insertion(context->document, parentNode, nodes, nodesc) != SUCCESS)) { - return; - } - - php_libxml_invalidate_node_list_cache_from_doc(context->document->ptr); + xmlDocPtr doc = parentNode->doc; + php_libxml_invalidate_node_list_cache_from_doc(doc); /* Spec step 4: convert nodes into fragment */ xmlNodePtr fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc); @@ -586,7 +572,6 @@ void dom_child_replace_with(dom_object *context, zval *nodes, uint32_t nodesc) /* Spec step 5: perform the replacement */ xmlNodePtr newchild = fragment->children; - xmlDocPtr doc = parentNode->doc; /* Unlink it unless it became a part of the fragment. * Freeing will be taken care of by the lifetime of the returned dom object. */ @@ -621,7 +606,7 @@ void dom_parent_node_replace_children(dom_object *context, zval *nodes, uint32_t return; } - php_libxml_invalidate_node_list_cache_from_doc(context->document->ptr); + php_libxml_invalidate_node_list_cache_from_doc(thisp->doc); dom_remove_all_children(thisp); diff --git a/ext/dom/tests/DOMChildNode_methods_without_parent.phpt b/ext/dom/tests/DOMChildNode_methods_without_parent.phpt new file mode 100644 index 00000000000..2591105cc37 --- /dev/null +++ b/ext/dom/tests/DOMChildNode_methods_without_parent.phpt @@ -0,0 +1,44 @@ +--TEST-- +DOMChildNode methods without a parent +--EXTENSIONS-- +dom +--FILE-- +loadXML(<< + + + +XML); + +$container = $doc->documentElement; +$child = $container->firstElementChild; + +$test = $doc->createElement('foo'); +foreach (['before', 'after', 'replaceWith'] as $method) { + echo "--- $method ---\n"; + $test->$method($child); + echo $doc->saveXML(); + echo $doc->saveXML($test), "\n"; +} +?> +--EXPECT-- +--- before --- + + + + + +--- after --- + + + + + +--- replaceWith --- + + + + + diff --git a/ext/dom/tests/bug79968.phpt b/ext/dom/tests/bug79968.phpt index bfd69e69c97..5ce1bcb7c6e 100644 --- a/ext/dom/tests/bug79968.phpt +++ b/ext/dom/tests/bug79968.phpt @@ -7,11 +7,14 @@ dom $cdata = new DOMText; -try { - $cdata->before("string"); -} catch (DOMException $e) { - echo $e->getMessage(); -} +$cdata->before("string"); +$cdata->after("string"); +$cdata->replaceWith("string"); + +$dom = new DOMDocument(); +$dom->adoptNode($cdata); +var_dump($dom->saveXML($cdata)); + ?> --EXPECT-- -Hierarchy Request Error +string(0) "" diff --git a/ext/dom/tests/element_child_and_parent_node_without_document.phpt b/ext/dom/tests/element_child_and_parent_node_without_document.phpt new file mode 100644 index 00000000000..a849135f669 --- /dev/null +++ b/ext/dom/tests/element_child_and_parent_node_without_document.phpt @@ -0,0 +1,21 @@ +--TEST-- +DOMElement: DOMChildNode, DOMParentNode modifications without a document +--EXTENSIONS-- +dom +--FILE-- +append("APPENDED"); +$element->prepend("PREPENDED"); +$element->after("AFTER"); +$element->before("BEFORE"); +$element->replaceWith("REPLACE"); + +$doc = new DOMDocument(); +$doc->adoptNode($element); +echo $doc->saveXML($element), "\n"; + +?> +--EXPECT-- +

PREPENDED Hello World! APPENDED