diff --git a/ext/dom/dom_iterators.c b/ext/dom/dom_iterators.c index a6e6493ccb7..ef81ecaaf73 100644 --- a/ext/dom/dom_iterators.c +++ b/ext/dom/dom_iterators.c @@ -188,8 +188,19 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */ } else { if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) { - curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node; - curnode = curnode->next; + + /* Note: keep legacy behaviour for non-spec mode. */ + if (php_dom_follow_spec_intern(intern) && php_dom_is_cache_tag_stale_from_doc_ptr(&iterator->cache_tag, intern->document)) { + php_dom_mark_cache_tag_up_to_date_from_doc_ref(&iterator->cache_tag, intern->document); + curnode = dom_fetch_first_iteration_item(objmap); + zend_ulong index = 0; + while (curnode != NULL && index++ < iter->index) { + curnode = curnode->next; + } + } else { + curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node; + curnode = curnode->next; + } } else { /* The collection is live, we nav the tree from the base object if we cannot * use the cache to restart from the last point. */ diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index e64695f07b1..26035832d95 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -262,6 +262,13 @@ static zend_always_inline void php_dom_mark_cache_tag_up_to_date_from_node(php_l } } +static zend_always_inline void php_dom_mark_cache_tag_up_to_date_from_doc_ref(php_libxml_cache_tag *cache_tag, php_libxml_ref_obj *document) +{ + ZEND_ASSERT(cache_tag != NULL); + ZEND_ASSERT(document != NULL); + cache_tag->modification_nr = document->cache_tag.modification_nr; +} + static zend_always_inline bool php_dom_follow_spec_node(const xmlNode *node) { ZEND_ASSERT(node != NULL); diff --git a/ext/dom/tests/gh13863.phpt b/ext/dom/tests/gh13863.phpt new file mode 100644 index 00000000000..e9a7058c341 --- /dev/null +++ b/ext/dom/tests/gh13863.phpt @@ -0,0 +1,164 @@ +--TEST-- +GH-13863 (Removal during NodeList iteration breaks loop) +--EXTENSIONS-- +dom +--FILE-- + + + + + + + + EOXML; + + if ($type === DomType::Legacy) { + $doc = new DOMDocument(); + $doc->preserveWhiteSpace = false; + $doc->loadXML($xml); + } else { + $doc = DOM\XMLDocument::createFromString($xml, LIBXML_NOBLANKS); + } + + return $doc; +} + +foreach ([DomType::Legacy, DomType::Modern] as $case) { + $name = match ($case) { + DomType::Legacy => 'Legacy', + DomType::Modern => 'Modern', + }; + + echo "--- $name test remove index 2 at index 2 ---\n"; + + $doc = createTestDoc($case); + + foreach ($doc->documentElement->childNodes as $i => $node) { + var_dump($doc->documentElement->childNodes->item($i) === $node); + var_dump($i, $node->localName); + if ($i === 2) { + $node->remove(); + } + } + + echo $doc->saveXML(), "\n"; + + echo "--- $name test remove index 1 at index 2 ---\n"; + + $doc = createTestDoc($case); + + foreach ($doc->documentElement->childNodes as $i => $node) { + var_dump($doc->documentElement->childNodes->item($i) === $node); + var_dump($i, $node->localName); + if ($i === 2) { + $doc->documentElement->childNodes[1]->remove(); + } + } + + echo $doc->saveXML(), "\n"; + + echo "--- $name test remove all ---\n"; + + $doc = createTestDoc($case); + + foreach ($doc->documentElement->childNodes as $i => $node) { + var_dump($doc->documentElement->childNodes->item($i) === $node); + var_dump($i, $node->localName); + $node->remove(); + } + + echo $doc->saveXML(), "\n"; +} + +?> +--EXPECT-- +--- Legacy test remove index 2 at index 2 --- +bool(true) +int(0) +string(5) "item1" +bool(true) +int(1) +string(5) "item2" +bool(true) +int(2) +string(5) "item3" + + + +--- Legacy test remove index 1 at index 2 --- +bool(true) +int(0) +string(5) "item1" +bool(true) +int(1) +string(5) "item2" +bool(true) +int(2) +string(5) "item3" +bool(false) +int(3) +string(5) "item4" +bool(false) +int(4) +string(5) "item5" + + + +--- Legacy test remove all --- +bool(true) +int(0) +string(5) "item1" + + + +--- Modern test remove index 2 at index 2 --- +bool(true) +int(0) +string(5) "item1" +bool(true) +int(1) +string(5) "item2" +bool(true) +int(2) +string(5) "item3" +bool(true) +int(3) +string(5) "item5" + + +--- Modern test remove index 1 at index 2 --- +bool(true) +int(0) +string(5) "item1" +bool(true) +int(1) +string(5) "item2" +bool(true) +int(2) +string(5) "item3" +bool(true) +int(3) +string(5) "item5" + + +--- Modern test remove all --- +bool(true) +int(0) +string(5) "item1" +bool(true) +int(1) +string(5) "item3" +bool(true) +int(2) +string(5) "item5" + +