Fix GH-17572: getElementsByTagName returns collections with tagName-based indexing, causing loss of elements when converted to arrays

Only dtd named node maps should have string-based indexing.
The ce check is fragile, just check for the presence of an xml hash
table.

Author: nielsdos

ext/dom/dom_iterators.c

@@ -49,6 +49,13 @@ static void itemHashScanner (void *payload, void *data, xmlChar *name)
 }
 /* }}} */
 
+static dom_nnodemap_object *php_dom_iterator_get_nnmap(const php_dom_iterator *iterator)
+{
+	const zval *object = &iterator->intern.data;
+	dom_object *nnmap = Z_DOMOBJ_P(object);
+	return nnmap->ptr;
+}
+
 xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID) /* {{{ */
 {
 	xmlEntityPtr ret = xmlMalloc(sizeof(xmlEntity));
@@ -120,11 +127,11 @@ zval *php_dom_iterator_current_data(zend_object_iterator *iter) /* {{{ */
 static void php_dom_iterator_current_key(zend_object_iterator *iter, zval *key) /* {{{ */
 {
 	php_dom_iterator *iterator = (php_dom_iterator *)iter;
-	zval *object = &iterator->intern.data;
-	zend_class_entry *ce = Z_OBJCE_P(object);
+	dom_nnodemap_object *objmap = php_dom_iterator_get_nnmap(iterator);
 
-	/* Nodelists have the index as a key while named node maps have the name as a key. */
-	if (instanceof_function(ce, dom_nodelist_class_entry) || instanceof_function(ce, dom_modern_nodelist_class_entry)) {
+	/* Only (dtd) named node maps, i.e. the ones based on a libxml hash table, are keyed by the name
+	 * because in that case the name is unique. */
+	if (!objmap->ht) {
 		ZVAL_LONG(key, iter->index);
 	} else {
 		dom_object *intern = Z_DOMOBJ_P(&iterator->curobj);
@@ -169,9 +176,7 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */
 	}
 
 	dom_object *intern = Z_DOMOBJ_P(&iterator->curobj);
-	zval *object = &iterator->intern.data;
-	dom_object *nnmap = Z_DOMOBJ_P(object);
-	dom_nnodemap_object *objmap = nnmap->ptr;
+	dom_nnodemap_object *objmap = php_dom_iterator_get_nnmap(iterator);
 
 	if (intern != NULL && intern->ptr != NULL) {
 		if (objmap->nodetype != XML_ENTITY_NODE &&

ext/dom/tests/modern/xml/gh17572.phpt

@@ -0,0 +1,39 @@
+--TEST--
+GH-17572 (getElementsByTagName returns collections with tagName-based indexing, causing loss of elements when converted to arrays)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = Dom\XMLDocument::createFromString(<<<XML
+<!DOCTYPE root [
+    <!ENTITY a "a">
+]>
+<root>
+    <p/><p/>
+</root>
+XML);
+
+foreach ($dom->querySelectorAll('p') as $k => $v) {
+    var_dump($k, $v->nodeName);
+}
+
+foreach ($dom->getElementsByTagName('p') as $k => $v) {
+    var_dump($k, $v->nodeName);
+}
+foreach ($dom->doctype->entities as $k => $v) {
+    var_dump($k, $v->nodeName);
+}
+
+?>
+--EXPECT--
+int(0)
+string(1) "p"
+int(1)
+string(1) "p"
+int(0)
+string(1) "p"
+int(1)
+string(1) "p"
+string(1) "a"
+string(1) "a"