php/archived-php-src
1
0
Fork 0
mirror of https://github.com/php/php-src.git synced 2026-03-24 00:02:20 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix GH-13863: Removal during NodeList iteration breaks loop

Browse Source 
The list is live, so upon cache invalidation we should rewalk the tree
to sync the index again with the node list. We keep the legacy behaviour
for the old DOM classes.

Closes GH-13934.
This commit is contained in:
Niels Dossche
2024-04-10 14:35:37 +02:00
parent 2bf9da6e3d
commit 78ccea4e40
3 changed files with 184 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
ext/dom/dom_iterators.c
View File

@@ -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. */

7
ext/dom/php_dom.h
View File

@@ -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);

164
ext/dom/tests/gh13863.phpt Normal file
View File

@@ -0,0 +1,164 @@
--TEST--
GH-13863 (Removal during NodeList iteration breaks loop)
--EXTENSIONS--
dom
--FILE--
<?php
enum DomType {
case Legacy;
case Modern;
}
function createTestDoc(DomType $type) {
$xml = <<<EOXML
<x>
<item1 />
<item2 />
<item3 />
<item4 />
<item5 />
</x>
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"
<?xml version="1.0"?>
<x><item1/><item2/><item4/><item5/></x>
--- 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"
<?xml version="1.0"?>
<x><item1/><item3/><item4/><item5/></x>
--- Legacy test remove all ---
bool(true)
int(0)
string(5) "item1"
<?xml version="1.0"?>
<x><item2/><item3/><item4/><item5/></x>
--- 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"
<?xml version="1.0" encoding="UTF-8"?>
<x><item1/><item2/><item4/><item5/></x>
--- 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"
<?xml version="1.0" encoding="UTF-8"?>
<x><item1/><item3/><item4/><item5/></x>
--- 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"
<?xml version="1.0" encoding="UTF-8"?>
<x><item2/><item4/></x>