Skip to content

Commit 30156d5

Browse files
rtheunissennikic
rtheunissen
authored and
nikic
committed
Fixed bug #63217
Don't automatically convert literal string keys to integers on array access, as we may be dealing with an ArrayAccess object, rather than a plain array.
1 parent 6d61814 commit 30156d5

File tree

13 files changed

+271
-152
lines changed

13 files changed

+271
-152
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ PHP NEWS
1414
. Fixed bug #76439 (Changed behaviour in unclosed HereDoc). (Nikita, tpunt)
1515
. Added syslog.facility and syslog.ident INI entries for customizing syslog
1616
logging. (Philip Prindeville)
17+
. Fixed bug #63217 (Constant numeric strings become integers when used as
18+
ArrayAccess offset). (Rudi Theunissen)
1719

1820
- DOM:
1921
. Fixed bug #76285 (DOMDocument::formatOutput attribute sometimes ignored).

‎UPGRADING

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ Core:
4444
the following "FOO;" will cause a syntax error. This issue can always be
4545
resolved by choosing an ending label that does not occur within the contents
4646
of the string.
47+
. Array accesses of type $obj["123"], where $obj implements ArrayAccess and
48+
"123" is an integral string literal will no longer result in an implicit
49+
conversion to integer, i.e., $obj->offsetGet("123") will be called instead
50+
of $obj->offsetGet(123). This matches existing behavior for non-literals.
51+
The behavior of arrays is not affected in any way, they continue to
52+
implicitly convert integeral string keys to integers.
4753
. In PHP, static properties are shared between inheriting classes, unless the
4854
static property is explicitly overridden in a child class. However, due to
4955
an implementation artifact it was possible to separate the static properties

‎Zend/tests/bug29883.phpt

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,18 @@ Bug #29883 (isset gives invalid values on strings)
44
<?php
55
$x = "bug";
66
var_dump(isset($x[-10]));
7+
var_dump(isset($x[1]));
78
var_dump(isset($x["1"]));
8-
echo $x["1"]."\n";
9+
var_dump($x[-10])."\n";
10+
var_dump($x[1])."\n";
11+
var_dump($x["1"])."\n";
912
?>
10-
--EXPECT--
13+
--EXPECTF--
1114
bool(false)
1215
bool(true)
13-
u
16+
bool(true)
17+
18+
Notice: Uninitialized string offset: -10 in %s on line 6
19+
string(0) ""
20+
string(1) "u"
21+
string(1) "u"

‎Zend/tests/bug55135.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ unset($array[1]);
1919
print_r($array);
2020

2121
$array = array(1 => 2);
22-
$a = 1;
22+
2323
unset($array["1"]);
2424
print_r($array);
2525
?>

‎Zend/tests/bug63217.phpt

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
--TEST--
2+
Bug #63217 (Constant numeric strings become integers when used as ArrayAccess offset)
3+
--INI--
4+
opcache.enable_cli=1
5+
opcache.enable=1
6+
opcache.optimization_level=-1
7+
--FILE--
8+
<?php
9+
class Test implements ArrayAccess {
10+
public function offsetExists($offset) {
11+
echo "offsetExists given ";
12+
var_dump($offset);
13+
}
14+
public function offsetUnset($offset) {
15+
echo "offsetUnset given ";
16+
var_dump($offset);
17+
}
18+
public function offsetSet($offset, $value) {
19+
echo "offsetSet given ";
20+
var_dump($offset);
21+
}
22+
public function offsetGet($offset) {
23+
echo "offsetGet given ";
24+
var_dump($offset);
25+
}
26+
}
27+
28+
$test = new Test;
29+
30+
/* These should all produce string(...) "..." output and not int(...) */
31+
isset($test['0']);
32+
isset($test['123']);
33+
unset($test['0']);
34+
unset($test['123']);
35+
$test['0'] = true;
36+
$test['123'] = true;
37+
$foo = $test['0'];
38+
$foo = $test['123'];
39+
40+
/* These caused the same bug, but in opcache rather than the compiler */
41+
isset($test[(string)'0']);
42+
isset($test[(string)'123']);
43+
unset($test[(string)'0']);
44+
unset($test[(string)'123']);
45+
$test[(string)'0'] = true;
46+
$test[(string)'123'] = true;
47+
$foo = $test[(string)'0'];
48+
$foo = $test[(string)'123'];
49+
50+
/**
51+
* @see https://github.com/php/php-src/pull/2607#issuecomment-313781748
52+
*/
53+
function test(): string {
54+
$array["10"] = 42;
55+
foreach ($array as $key => $value) {
56+
return $key;
57+
}
58+
}
59+
60+
var_dump(test());
61+
62+
/**
63+
* Make sure we don't break arrays.
64+
*/
65+
$array = [];
66+
67+
$key = '123';
68+
69+
$array[$key] = 1;
70+
$array['321'] = 2;
71+
$array['abc'] = 3;
72+
73+
var_dump($array);
74+
75+
/**
76+
* Make sure that we haven't broken ArrayObject
77+
*/
78+
$ao = new ArrayObject();
79+
80+
$key = '123';
81+
82+
$ao = [];
83+
$ao[$key] = 1;
84+
$ao['321'] = 2;
85+
$ao['abc'] = 3;
86+
87+
var_dump($ao);
88+
89+
?>
90+
--EXPECT--
91+
offsetExists given string(1) "0"
92+
offsetExists given string(3) "123"
93+
offsetUnset given string(1) "0"
94+
offsetUnset given string(3) "123"
95+
offsetSet given string(1) "0"
96+
offsetSet given string(3) "123"
97+
offsetGet given string(1) "0"
98+
offsetGet given string(3) "123"
99+
offsetExists given string(1) "0"
100+
offsetExists given string(3) "123"
101+
offsetUnset given string(1) "0"
102+
offsetUnset given string(3) "123"
103+
offsetSet given string(1) "0"
104+
offsetSet given string(3) "123"
105+
offsetGet given string(1) "0"
106+
offsetGet given string(3) "123"
107+
string(2) "10"
108+
array(3) {
109+
[123]=>
110+
int(1)
111+
[321]=>
112+
int(2)
113+
["abc"]=>
114+
int(3)
115+
}
116+
array(3) {
117+
[123]=>
118+
int(1)
119+
[321]=>
120+
int(2)
121+
["abc"]=>
122+
int(3)
123+
}

‎Zend/tests/empty_str_offset.phpt

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,24 @@ var_dump(empty($str[5])); // 1
1717
var_dump(empty($str[8]));
1818
var_dump(empty($str[10000]));
1919
// non-numeric offsets
20-
print "- string ---\n";
21-
var_dump(empty($str['-1']));
20+
print "- string literal ---\n";
21+
var_dump(empty($str['-1'])); // 3
2222
var_dump(empty($str['-10']));
23-
var_dump(empty($str['-4'])); // 0
2423
var_dump(empty($str['0']));
2524
var_dump(empty($str['1']));
2625
var_dump(empty($str['4'])); // 0
2726
var_dump(empty($str['1.5']));
2827
var_dump(empty($str['good']));
2928
var_dump(empty($str['3 and a half']));
29+
print "- string variable ---\n";
30+
var_dump(empty($str[$key = '-1'])); // 3
31+
var_dump(empty($str[$key = '-10']));
32+
var_dump(empty($str[$key = '0']));
33+
var_dump(empty($str[$key = '1']));
34+
var_dump(empty($str[$key = '4'])); // 0
35+
var_dump(empty($str[$key = '1.5']));
36+
var_dump(empty($str[$key = 'good']));
37+
var_dump(empty($str[$key = '3 and a half']));
3038
print "- bool ---\n";
3139
var_dump(empty($str[true]));
3240
var_dump(empty($str[false]));
@@ -54,7 +62,7 @@ var_dump(empty($str[$f]));
5462
print "done\n";
5563

5664
?>
57-
--EXPECT--
65+
--EXPECTF--
5866
- empty ---
5967
bool(false)
6068
bool(true)
@@ -65,9 +73,17 @@ bool(true)
6573
bool(false)
6674
bool(true)
6775
bool(true)
68-
- string ---
76+
- string literal ---
6977
bool(false)
7078
bool(true)
79+
bool(false)
80+
bool(false)
81+
bool(true)
82+
bool(true)
83+
bool(true)
84+
bool(true)
85+
- string variable ---
86+
bool(false)
7187
bool(true)
7288
bool(false)
7389
bool(false)

‎Zend/tests/isset_str_offset.phpt

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,24 @@ var_dump(isset($str[5])); // 1
1616
var_dump(isset($str[8]));
1717
var_dump(isset($str[10000]));
1818
// non-numeric offsets
19-
print "- string ---\n";
20-
var_dump(isset($str['-1']));
19+
print "- string literal ---\n";
20+
var_dump(isset($str['-1'])); // 3
2121
var_dump(isset($str['-10']));
2222
var_dump(isset($str['0']));
2323
var_dump(isset($str['1']));
2424
var_dump(isset($str['4'])); // 0
2525
var_dump(isset($str['1.5']));
2626
var_dump(isset($str['good']));
2727
var_dump(isset($str['3 and a half']));
28+
print "- string variable ---\n";
29+
var_dump(isset($str[$key = '-1'])); // 3
30+
var_dump(isset($str[$key = '-10']));
31+
var_dump(isset($str[$key = '0']));
32+
var_dump(isset($str[$key = '1']));
33+
var_dump(isset($str[$key = '4'])); // 0
34+
var_dump(isset($str[$key = '1.5']));
35+
var_dump(isset($str[$key = 'good']));
36+
var_dump(isset($str[$key = '3 and a half']));
2837
print "- bool ---\n";
2938
var_dump(isset($str[true]));
3039
var_dump(isset($str[false]));
@@ -51,7 +60,7 @@ var_dump(isset($str[$f]));
5160
print "done\n";
5261

5362
?>
54-
--EXPECT--
63+
--EXPECTF--
5564
- isset ---
5665
bool(true)
5766
bool(false)
@@ -61,7 +70,16 @@ bool(true)
6170
bool(true)
6271
bool(false)
6372
bool(false)
64-
- string ---
73+
- string literal ---
74+
bool(true)
75+
bool(false)
76+
bool(true)
77+
bool(true)
78+
bool(true)
79+
bool(false)
80+
bool(false)
81+
bool(false)
82+
- string variable ---
6583
bool(true)
6684
bool(false)
6785
bool(true)

‎Zend/zend_compile.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2672,7 +2672,6 @@ static zend_op *zend_delayed_compile_dim(znode *result, zend_ast *ast, uint32_t
26722672
dim_node.op_type = IS_UNUSED;
26732673
} else {
26742674
zend_compile_expr(&dim_node, dim_ast);
2675-
zend_handle_numeric_op(&dim_node);
26762675
}
26772676

26782677
opline = zend_delayed_emit_op(result, ZEND_FETCH_DIM_R, &var_node, &dim_node);

‎Zend/zend_execute.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1731,10 +1731,8 @@ static zend_always_inline zval *zend_fetch_dimension_address_inner(HashTable *ht
17311731
}
17321732
} else if (EXPECTED(Z_TYPE_P(dim) == IS_STRING)) {
17331733
offset_key = Z_STR_P(dim);
1734-
if (dim_type != IS_CONST) {
1735-
if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
1736-
goto num_index;
1737-
}
1734+
if (ZEND_HANDLE_NUMERIC(offset_key, hval)) {
1735+
goto num_index;
17381736
}
17391737
str_index:
17401738
retval = zend_hash_find_ex(ht, offset_key, dim_type == IS_CONST);

‎Zend/zend_vm_def.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5538,10 +5538,8 @@ ZEND_VM_C_LABEL(unset_dim_array):
55385538
ZEND_VM_C_LABEL(offset_again):
55395539
if (EXPECTED(Z_TYPE_P(offset) == IS_STRING)) {
55405540
key = Z_STR_P(offset);
5541-
if (OP2_TYPE != IS_CONST) {
5542-
if (ZEND_HANDLE_NUMERIC(key, hval)) {
5543-
ZEND_VM_C_GOTO(num_index_dim);
5544-
}
5541+
if (ZEND_HANDLE_NUMERIC(key, hval)) {
5542+
ZEND_VM_C_GOTO(num_index_dim);
55455543
}
55465544
ZEND_VM_C_LABEL(str_index_dim):
55475545
if (ht == &EG(symbol_table)) {
@@ -6286,10 +6284,8 @@ ZEND_VM_C_LABEL(isset_dim_obj_array):
62866284
ZEND_VM_C_LABEL(isset_again):
62876285
if (EXPECTED(Z_TYPE_P(offset) == IS_STRING)) {
62886286
str = Z_STR_P(offset);
6289-
if (OP2_TYPE != IS_CONST) {
6290-
if (ZEND_HANDLE_NUMERIC(str, hval)) {
6291-
ZEND_VM_C_GOTO(num_index_prop);
6292-
}
6287+
if (ZEND_HANDLE_NUMERIC(str, hval)) {
6288+
ZEND_VM_C_GOTO(num_index_prop);
62936289
}
62946290
value = zend_hash_find_ex_ind(ht, str, OP2_TYPE == IS_CONST);
62956291
} else if (EXPECTED(Z_TYPE_P(offset) == IS_LONG)) {

0 commit comments

Comments
 (0)