From e7baf86fdd1558b5a7f4f0c958a0d3aac7b7f37f Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Sat, 8 Mar 2025 19:38:42 +0100
Subject: [PATCH] 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.
---
 ext/dom/dom_iterators.c                     | 12 ++++----
 ext/dom/php_dom.h                           |  3 ++
 ext/dom/tests/unpack_foreach_behaviour.phpt | 31 +++++++++++++++++++++
 3 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 ext/dom/tests/unpack_foreach_behaviour.phpt

diff --git a/ext/dom/dom_iterators.c b/ext/dom/dom_iterators.c
index ff63342595ae9..c8256de239d93 100644
--- a/ext/dom/dom_iterators.c
+++ b/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);
 			}
 		}
 	}
diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h
index 2bccb2d5692d5..120af4267658c 100644
--- a/ext/dom/php_dom.h
+++ b/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;
 
diff --git a/ext/dom/tests/unpack_foreach_behaviour.phpt b/ext/dom/tests/unpack_foreach_behaviour.phpt
new file mode 100644
index 0000000000000..42fe896d9f786
--- /dev/null
+++ b/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"