Skip to content

Commit 052376e

Browse files
committed
Fix #80332: Completely broken array access functionality with DOMNamedNodeMap
The problem is the usage of zval_get_long(). In particular, if the string is non-numeric the result of zval_get_long() will be 0 *without giving an error or warning*. This is misleading for users: users get the impression that they can use strings to access the map because it coincidentally works for the first item (which is at index 0). Of course, this fails with any other index which causes confusion and bugs. This patch adds proper support for using string offsets while accessing the map. It does so by detecting if it's a non-numeric string, and then using the getNamedItem() method instead of item(). I plan on refactoring this for 8.3 to avoid the double function call and useless object construction for notations.
1 parent 1c984a3 commit 052376e

File tree

2 files changed

+41
-8
lines changed

2 files changed

+41
-8
lines changed

ext/dom/php_dom.c

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,18 +1692,51 @@ static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int
16921692
return NULL;
16931693
}
16941694

1695-
ZVAL_LONG(&offset_copy, zval_get_long(offset));
1696-
1695+
if (Z_TYPE_P(offset) == IS_STRING) {
1696+
/* See zval_get_long_func() */
1697+
zend_long lval;
1698+
double dval;
1699+
zend_uchar is_numeric_string_type;
1700+
if (0 == (is_numeric_string_type = is_numeric_string(Z_STRVAL_P(offset), Z_STRLEN_P(offset), &lval, &dval, true))) {
1701+
/* exceptional case, switch to named lookup */
1702+
ZVAL_COPY(&offset_copy, offset);
1703+
zend_call_method_with_1_params(object, object->ce, NULL, "getNamedItem", rv, &offset_copy);
1704+
return rv;
1705+
} else if (EXPECTED(is_numeric_string_type == IS_LONG)) {
1706+
ZVAL_LONG(&offset_copy, lval);
1707+
} else {
1708+
ZVAL_LONG(&offset_copy, zend_dval_to_lval_cap(dval));
1709+
}
1710+
} else {
1711+
ZVAL_LONG(&offset_copy, zval_get_long(offset));
1712+
}
16971713
zend_call_method_with_1_params(object, object->ce, NULL, "item", rv, &offset_copy);
16981714

16991715
return rv;
17001716
} /* }}} end dom_nodelist_read_dimension */
17011717

17021718
static int dom_nodelist_has_dimension(zend_object *object, zval *member, int check_empty)
17031719
{
1704-
zend_long offset = zval_get_long(member);
1720+
zend_long offset;
17051721
zval rv;
17061722

1723+
if (Z_TYPE_P(member) == IS_STRING) {
1724+
/* See zval_get_long_func() */
1725+
double dval;
1726+
zend_uchar is_numeric_string_type;
1727+
if (0 == (is_numeric_string_type = is_numeric_string(Z_STRVAL_P(member), Z_STRLEN_P(member), &offset, &dval, true))) {
1728+
/* exceptional case, switch to named lookup */
1729+
zend_call_method_with_1_params(object, object->ce, NULL, "getNamedItem", &rv, member);
1730+
int ret = Z_TYPE(rv) != IS_NULL;
1731+
zval_ptr_dtor(&rv);
1732+
return ret;
1733+
} else if (is_numeric_string_type == IS_DOUBLE) {
1734+
offset = zend_dval_to_lval_cap(dval);
1735+
}
1736+
} else {
1737+
offset = zval_get_long(member);
1738+
}
1739+
17071740
if (offset < 0) {
17081741
return 0;
17091742
} else {

ext/dom/tests/bug80332.phpt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ Attribute [null] name: attr1
5858
Attribute [null] value: value1
5959
Attribute [null] isset: yes
6060

61-
Attribute ['attr2'] name: attr1
62-
Attribute ['attr2'] value: value1
61+
Attribute ['attr2'] name: attr2
62+
Attribute ['attr2'] value: value2
6363
Attribute ['attr2'] isset: yes
6464

65-
Attribute ['hi'] name: attr1
66-
Attribute ['hi'] value: value1
67-
Attribute ['hi'] isset: yes
65+
Attribute ['hi'] name: /
66+
Attribute ['hi'] value: /
67+
Attribute ['hi'] isset: no
6868

6969
Attribute ['0'] name: attr1
7070
Attribute ['0'] value: value1

0 commit comments

Comments
 (0)