Fix weird unpack behaviour in DOM

Engine pitfall: the iter index is only updated by foreach opcodes, so
the existing code that used it as an index for the nodes w.r.t. the
start did not work properly. Fix it by using our own counter.

Closes GH-18004.

Author: nielsdos

NEWS

@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.3.19
 
+- DOM:
+  . Fix weird unpack behaviour in DOM. (nielsdos)
+
 - GD:
   . Fixed bug GH-17984 (calls with arguments as array with references).
     (David Carlier)

ext/dom/dom_iterators.c

@@ -158,7 +158,7 @@ static void php_dom_iterator_current_key(zend_object_iterator *iter, zval *key)
 	zval *object = &iterator->intern.data;
 
 	if (instanceof_function(Z_OBJCE_P(object), dom_nodelist_class_entry)) {
-		ZVAL_LONG(key, iter->index);
+		ZVAL_LONG(key, iterator->index);
 	} else {
 		dom_object *intern = Z_DOMOBJ_P(&iterator->curobj);
 
@@ -189,6 +189,8 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */
 		return;
 	}
 
+	iterator->index++;
+
 	intern = Z_DOMOBJ_P(&iterator->curobj);
 	object = &iterator->intern.data;
 	nnmap = Z_DOMOBJ_P(object);
@@ -227,18 +229,18 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */
 							curnode = basenode->children;
 						}
 					} else {
-						previndex = iter->index - 1;
+						previndex = iterator->index - 1;
 						curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node;
 					}
 					curnode = dom_get_elements_by_tag_name_ns_raw(
-						basenode, curnode, (char *) objmap->ns, (char *) objmap->local, &previndex, iter->index);
+						basenode, curnode, (char *) objmap->ns, (char *) objmap->local, &previndex, iterator->index);
 				}
 			}
 		} else {
 			if (objmap->nodetype == XML_ENTITY_NODE) {
-				curnode = php_dom_libxml_hash_iter(objmap->ht, iter->index);
+				curnode = php_dom_libxml_hash_iter(objmap->ht, iterator->index);
 			} else {
-				curnode = php_dom_libxml_notation_iter(objmap->ht, iter->index);
+				curnode = php_dom_libxml_notation_iter(objmap->ht, iterator->index);
 			}
 		}
 	}

ext/dom/php_dom.h

@@ -98,6 +98,9 @@ typedef struct {
 	zend_object_iterator intern;
 	zval curobj;
 	HashPosition pos;
+	/* intern->index is only updated for FE_* opcodes, not for e.g. unpacking,
+	 * yet we need to track the position of the node relative to the start. */
+	zend_ulong index;
 	php_libxml_cache_tag cache_tag;
 } php_dom_iterator;
 

ext/dom/tests/unpack_foreach_behaviour.phpt

@@ -0,0 +1,31 @@
+--TEST--
+Unpacking vs foreach behaviour
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = new DOMDocument;
+$dom->loadXML('<root><a/><b/></root>');
+
+echo "--- By foreach: ---\n";
+
+foreach ($dom->documentElement->getElementsByTagName('*') as $node) {
+    var_dump($node->localName);
+}
+
+echo "--- By unpacking: ---\n";
+
+$iter = $dom->documentElement->getElementsByTagName('*');
+foreach ([...$iter] as $node) {
+    var_dump($node->localName);
+}
+
+?>
+--EXPECT--
+--- By foreach: ---
+string(1) "a"
+string(1) "b"
+--- By unpacking: ---
+string(1) "a"
+string(1) "b"