Skip to content

Commit fc7c353

Browse files
ndosschendossche
committed
Fix phpGH-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. Closes phpGH-17580.
1 parent 5c066e0 commit fc7c353

File tree

3 files changed

+91
-9
lines changed

3 files changed

+91
-9
lines changed

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ PHP NEWS
3030
(nielsdos)
3131
. Fixed bug GH-17485 (upstream fix, Self-closing tag on void elements
3232
shouldn't be a parse error/warning in \Dom\HTMLDocument). (lexborisov)
33+
. Fixed bug GH-17572 (getElementsByTagName returns collections with
34+
tagName-based indexing). (nielsdos)
3335

3436
- Enchant:
3537
. Fix crashes in enchant when passing null bytes. (nielsdos)

‎ext/dom/dom_iterators.c‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ static void itemHashScanner (void *payload, void *data, xmlChar *name)
4949
}
5050
/* }}} */
5151

52+
static dom_nnodemap_object *php_dom_iterator_get_nnmap(const php_dom_iterator *iterator)
53+
{
54+
const zval *object = &iterator->intern.data;
55+
dom_object *nnmap = Z_DOMOBJ_P(object);
56+
return nnmap->ptr;
57+
}
58+
5259
xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID) /* {{{ */
5360
{
5461
xmlEntityPtr ret = xmlMalloc(sizeof(xmlEntity));
@@ -120,18 +127,22 @@ zval *php_dom_iterator_current_data(zend_object_iterator *iter) /* {{{ */
120127
static void php_dom_iterator_current_key(zend_object_iterator *iter, zval *key) /* {{{ */
121128
{
122129
php_dom_iterator *iterator = (php_dom_iterator *)iter;
123-
zval *object = &iterator->intern.data;
124-
zend_class_entry *ce = Z_OBJCE_P(object);
130+
dom_nnodemap_object *objmap = php_dom_iterator_get_nnmap(iterator);
125131

126-
/* Nodelists have the index as a key while named node maps have the name as a key. */
127-
if (instanceof_function(ce, dom_nodelist_class_entry) || instanceof_function(ce, dom_modern_nodelist_class_entry)) {
132+
/* Only dtd named node maps, i.e. the ones based on a libxml hash table or attribute collections,
133+
* are keyed by the name because in that case the name is unique. */
134+
if (!objmap->ht && objmap->nodetype != XML_ATTRIBUTE_NODE) {
128135
ZVAL_LONG(key, iter->index);
129136
} else {
130137
dom_object *intern = Z_DOMOBJ_P(&iterator->curobj);
131138

132139
if (intern != NULL && intern->ptr != NULL) {
133-
xmlNodePtr curnode = (xmlNodePtr)((php_libxml_node_ptr *)intern->ptr)->node;
134-
ZVAL_STRINGL(key, (char *) curnode->name, xmlStrlen(curnode->name));
140+
xmlNodePtr curnode = ((php_libxml_node_ptr *)intern->ptr)->node;
141+
if (objmap->nodetype == XML_ATTRIBUTE_NODE && php_dom_follow_spec_intern(intern)) {
142+
ZVAL_NEW_STR(key, dom_node_get_node_name_attribute_or_element(curnode, false));
143+
} else {
144+
ZVAL_STRINGL(key, (const char *) curnode->name, xmlStrlen(curnode->name));
145+
}
135146
} else {
136147
ZVAL_NULL(key);
137148
}
@@ -169,9 +180,7 @@ static void php_dom_iterator_move_forward(zend_object_iterator *iter) /* {{{ */
169180
}
170181

171182
dom_object *intern = Z_DOMOBJ_P(&iterator->curobj);
172-
zval *object = &iterator->intern.data;
173-
dom_object *nnmap = Z_DOMOBJ_P(object);
174-
dom_nnodemap_object *objmap = nnmap->ptr;
183+
dom_nnodemap_object *objmap = php_dom_iterator_get_nnmap(iterator);
175184

176185
if (intern != NULL && intern->ptr != NULL) {
177186
if (objmap->nodetype != XML_ENTITY_NODE &&

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

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
--TEST--
2+
GH-17572 (getElementsByTagName returns collections with tagName-based indexing, causing loss of elements when converted to arrays)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = Dom\XMLDocument::createFromString(<<<XML
9+
<!DOCTYPE root [
10+
<!ENTITY a "a">
11+
]>
12+
<root>
13+
<p/>
14+
<p xmlns:x="urn:x" x:a="1" b="2" a="3"/>
15+
</root>
16+
XML);
17+
18+
echo "--- querySelectorAll('p') ---\n";
19+
20+
foreach ($dom->querySelectorAll('p') as $k => $v) {
21+
var_dump($k, $v->nodeName);
22+
}
23+
24+
echo "--- getElementsByTagName('p') ---\n";
25+
26+
foreach ($dom->getElementsByTagName('p') as $k => $v) {
27+
var_dump($k, $v->nodeName);
28+
}
29+
30+
echo "--- entities ---\n";
31+
32+
foreach ($dom->doctype->entities as $k => $v) {
33+
var_dump($k, $v->nodeName);
34+
}
35+
36+
echo "--- attributes ---\n";
37+
38+
$attribs = $dom->getElementsByTagName('p')[1]->attributes;
39+
foreach ($attribs as $k => $v) {
40+
var_dump($k, $v->value);
41+
var_dump($attribs[$k]->value);
42+
}
43+
44+
?>
45+
--EXPECT--
46+
--- querySelectorAll('p') ---
47+
int(0)
48+
string(1) "p"
49+
int(1)
50+
string(1) "p"
51+
--- getElementsByTagName('p') ---
52+
int(0)
53+
string(1) "p"
54+
int(1)
55+
string(1) "p"
56+
--- entities ---
57+
string(1) "a"
58+
string(1) "a"
59+
--- attributes ---
60+
string(7) "xmlns:x"
61+
string(5) "urn:x"
62+
string(5) "urn:x"
63+
string(3) "x:a"
64+
string(1) "1"
65+
string(1) "1"
66+
string(1) "b"
67+
string(1) "2"
68+
string(1) "2"
69+
string(1) "a"
70+
string(1) "3"
71+
string(1) "3"

0 commit comments

Comments
 (0)