Skip to content

Commit 14bdb0c

Browse files
kocsismatenikic
kocsismate
authored and
nikic
committed
Fix consistency issues with array accesses warnings/exceptions
 * Change a number of "resource used as offset" notices to warnings, which were previously missed. * Throw the "resource used as offset" warning for isset() as well. * Make array_key_exists() behavior with regard to different key types consistent with isset() and normal array accesses. All key types now use the usual coercions and array/object keys throw TypeError. Closes GH-4887.
1 parent 0b0d4eb commit 14bdb0c

16 files changed

+128
-72
lines changed

‎.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,9 @@ php
247247
# Test results generated by `./run-tests.php`
248248
php_test_results_*.txt
249249

250+
# Temporary test information generated by `./run-tests.php`
251+
/run-test-info.php
252+
250253
# Temporary POST data placeholder files generated by `./run-tests.php`
251254
phpt.*
252255

‎Zend/tests/const_array_with_resource_key.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ var_dump(FOO);
88

99
?>
1010
--EXPECTF--
11-
Notice: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
11+
Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
1212
array(1) {
1313
[%d]=>
1414
int(42)

‎Zend/tests/isset_003.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Testing isset accessing undefined array itens and properties
2+
Testing isset accessing undefined array items and properties
33
--FILE--
44
<?php
55

‎Zend/tests/isset_array.phpt

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
--TEST--
2+
Using isset() with arrays
3+
--FILE--
4+
<?php
5+
6+
$array = [
7+
0 => true,
8+
"a" => true,
9+
];
10+
11+
var_dump(isset($array[0]));
12+
13+
var_dump(isset($array["a"]));
14+
15+
var_dump(isset($array[false]));
16+
17+
var_dump(isset($array[0.6]));
18+
19+
var_dump(isset($array[true]));
20+
21+
var_dump(isset($array[null]));
22+
23+
var_dump(isset($array[STDIN]));
24+
25+
try {
26+
isset($array[[]]);
27+
} catch (TypeError $exception) {
28+
echo $exception->getMessage() . "\n";
29+
}
30+
31+
try {
32+
isset($array[new stdClass()]);
33+
} catch (TypeError $exception) {
34+
echo $exception->getMessage() . "\n";
35+
}
36+
?>
37+
--EXPECTF--
38+
bool(true)
39+
bool(true)
40+
bool(true)
41+
bool(true)
42+
bool(false)
43+
bool(false)
44+
45+
Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
46+
bool(false)
47+
Illegal offset type in isset or empty
48+
Illegal offset type in isset or empty

‎Zend/zend_API.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1531,7 +1531,7 @@ ZEND_API int array_set_zval_key(HashTable *ht, zval *key, zval *value) /* {{{ */
15311531
result = zend_symtable_update(ht, ZSTR_EMPTY_ALLOC(), value);
15321532
break;
15331533
case IS_RESOURCE:
1534-
zend_error(E_NOTICE, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(key), Z_RES_HANDLE_P(key));
1534+
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(key), Z_RES_HANDLE_P(key));
15351535
result = zend_hash_index_update(ht, Z_RES_HANDLE_P(key), value);
15361536
break;
15371537
case IS_FALSE:

‎Zend/zend_ast.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,7 +436,7 @@ static int zend_ast_add_array_element(zval *result, zval *offset, zval *expr)
436436
zend_hash_index_update(Z_ARRVAL_P(result), zend_dval_to_lval(Z_DVAL_P(offset)), expr);
437437
break;
438438
case IS_RESOURCE:
439-
zend_error(E_NOTICE, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(offset), Z_RES_HANDLE_P(offset));
439+
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(offset), Z_RES_HANDLE_P(offset));
440440
zend_hash_index_update(Z_ARRVAL_P(result), Z_RES_HANDLE_P(offset), expr);
441441
break;
442442
default:
@@ -451,7 +451,7 @@ static int zend_ast_add_unpacked_element(zval *result, zval *expr) {
451451
HashTable *ht = Z_ARRVAL_P(expr);
452452
zval *val;
453453
zend_string *key;
454-
454+
455455
ZEND_HASH_FOREACH_STR_KEY_VAL(ht, key, val) {
456456
if (key) {
457457
zend_throw_error(NULL, "Cannot unpack array with string keys");

‎Zend/zend_execute.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2369,6 +2369,7 @@ static zend_never_inline zval* ZEND_FASTCALL zend_find_array_dim_slow(HashTable
23692369
hval = 1;
23702370
goto num_idx;
23712371
} else if (Z_TYPE_P(offset) == IS_RESOURCE) {
2372+
zend_use_resource_as_offset(offset);
23722373
hval = Z_RES_HANDLE_P(offset);
23732374
goto num_idx;
23742375
} else if (/*OP2_TYPE == IS_CV &&*/ Z_TYPE_P(offset) == IS_UNDEF) {
@@ -2478,14 +2479,27 @@ static zend_never_inline zend_bool ZEND_FASTCALL zend_array_key_exists_fast(Hash
24782479
} else if (EXPECTED(Z_ISREF_P(key))) {
24792480
key = Z_REFVAL_P(key);
24802481
goto try_again;
2482+
} else if (Z_TYPE_P(key) == IS_DOUBLE) {
2483+
hval = zend_dval_to_lval(Z_DVAL_P(key));
2484+
goto num_key;
2485+
} else if (Z_TYPE_P(key) == IS_FALSE) {
2486+
hval = 0;
2487+
goto num_key;
2488+
} else if (Z_TYPE_P(key) == IS_TRUE) {
2489+
hval = 1;
2490+
goto num_key;
2491+
} else if (Z_TYPE_P(key) == IS_RESOURCE) {
2492+
zend_use_resource_as_offset(key);
2493+
hval = Z_RES_HANDLE_P(key);
2494+
goto num_key;
24812495
} else if (Z_TYPE_P(key) <= IS_NULL) {
24822496
if (UNEXPECTED(Z_TYPE_P(key) == IS_UNDEF)) {
24832497
ZVAL_UNDEFINED_OP1();
24842498
}
24852499
str = ZSTR_EMPTY_ALLOC();
24862500
goto str_key;
24872501
} else {
2488-
zend_error(E_WARNING, "array_key_exists(): The first argument should be either a string or an integer");
2502+
zend_type_error("Illegal offset type");
24892503
return 0;
24902504
}
24912505
}

‎ext/opcache/jit/zend_jit_helpers.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ static int ZEND_FASTCALL zend_jit_fetch_dim_isset_helper(zend_array *ht, zval *d
400400
hval = zend_dval_to_lval(Z_DVAL_P(dim));
401401
goto num_index;
402402
case IS_RESOURCE:
403-
//zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(dim), Z_RES_HANDLE_P(dim));
403+
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(dim), Z_RES_HANDLE_P(dim));
404404
hval = Z_RES_HANDLE_P(dim);
405405
goto num_index;
406406
case IS_FALSE:

‎ext/spl/spl_array.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ static zval *spl_array_get_dimension_ptr(int check_inherited, spl_array_object *
348348
}
349349
return retval;
350350
case IS_RESOURCE:
351-
zend_error(E_NOTICE, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_P(offset)->handle, Z_RES_P(offset)->handle);
351+
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_P(offset)->handle, Z_RES_P(offset)->handle);
352352
index = Z_RES_P(offset)->handle;
353353
goto num_index;
354354
case IS_DOUBLE:

‎ext/spl/tests/bug62978.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ Notice: Undefined index: epic_magic in %sbug62978.php on line %d
4646
NULL
4747
bool(false)
4848

49-
Notice: Resource ID#%d used as offset, casting to integer (%d) in %sbug62978.php on line %d
49+
Warning: Resource ID#%d used as offset, casting to integer (%d) in %sbug62978.php on line %d
5050

5151
Notice: Undefined offset: %d in %sbug62978.php on line %d
5252
NULL

‎ext/standard/array.c

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "php_math.h"
4141
#include "zend_smart_str.h"
4242
#include "zend_bitset.h"
43+
#include "zend_exceptions.h"
4344
#include "ext/spl/spl_array.h"
4445

4546
/* {{{ defines */
@@ -765,6 +766,7 @@ PHP_FUNCTION(count)
765766

766767
switch (Z_TYPE_P(array)) {
767768
case IS_NULL:
769+
/* Intentionally not converted to an exception */
768770
php_error_docref(NULL, E_WARNING, "Parameter must be an array or an object that implements Countable");
769771
RETURN_LONG(0);
770772
break;
@@ -799,11 +801,13 @@ PHP_FUNCTION(count)
799801
}
800802

801803
/* If There's no handler and it doesn't implement Countable then add a warning */
804+
/* Intentionally not converted to an exception */
802805
php_error_docref(NULL, E_WARNING, "Parameter must be an array or an object that implements Countable");
803806
RETURN_LONG(1);
804807
break;
805808
}
806809
default:
810+
/* Intentionally not converted to an exception */
807811
php_error_docref(NULL, E_WARNING, "Parameter must be an array or an object that implements Countable");
808812
RETURN_LONG(1);
809813
break;
@@ -5212,7 +5216,7 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_
52125216
param_spec = "+f";
52135217
diff_data_compare_func = php_array_user_compare;
52145218
} else {
5215-
php_error_docref(NULL, E_WARNING, "data_compare_type is %d. This should never happen. Please report as a bug", data_compare_type);
5219+
ZEND_ASSERT(0 && "Invalid data_compare_type");
52165220
return;
52175221
}
52185222

@@ -6349,9 +6353,22 @@ PHP_FUNCTION(array_key_exists)
63496353
case IS_NULL:
63506354
RETVAL_BOOL(zend_hash_exists_ind(ht, ZSTR_EMPTY_ALLOC()));
63516355
break;
6356+
case IS_DOUBLE:
6357+
RETVAL_BOOL(zend_hash_index_exists(ht, zend_dval_to_lval(Z_DVAL_P(key))));
6358+
break;
6359+
case IS_FALSE:
6360+
RETVAL_BOOL(zend_hash_index_exists(ht, 0));
6361+
break;
6362+
case IS_TRUE:
6363+
RETVAL_BOOL(zend_hash_index_exists(ht, 1));
6364+
break;
6365+
case IS_RESOURCE:
6366+
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(key), Z_RES_HANDLE_P(key));
6367+
RETVAL_BOOL(zend_hash_index_exists(ht, Z_RES_HANDLE_P(key)));
6368+
break;
63526369
default:
6353-
php_error_docref(NULL, E_WARNING, "The first argument should be either a string or an integer");
6354-
RETVAL_FALSE;
6370+
zend_type_error("Illegal offset type");
6371+
break;
63556372
}
63566373
}
63576374
/* }}} */

‎ext/standard/basic_functions.stub.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ function krsort(array &$arg, int $sort_flags = SORT_REGULAR): bool {}
6363

6464
function ksort(array &$arg, int $sort_flags = SORT_REGULAR): bool {}
6565

66-
/** @param array|Countable $array */
66+
/** @param array|Countable $var */
6767
function count($var, int $mode = COUNT_NORAML): int {}
6868

6969
function natsort(array &$arg): bool {}
@@ -258,11 +258,8 @@ function array_filter(array $arg, callable $callback = UNKNOWN, int $use_keys =
258258

259259
function array_map(?callable $callback, array $arr1, array ...$arrays): array {}
260260

261-
/**
262-
* @param int|string $key
263-
* @param array|object $search
264-
*/
265-
function array_key_exists($key, $search): bool {}
261+
/** @param mixed $key */
262+
function array_key_exists($key, array $search): bool {}
266263

267264
function array_chunk(array $arg, int $size, bool $preserve_keys = false): array {}
268265

‎ext/standard/basic_functions_arginfo.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ ZEND_END_ARG_INFO()
341341

342342
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_array_key_exists, 0, 2, _IS_BOOL, 0)
343343
ZEND_ARG_INFO(0, key)
344-
ZEND_ARG_INFO(0, search)
344+
ZEND_ARG_TYPE_INFO(0, search, IS_ARRAY, 0)
345345
ZEND_END_ARG_INFO()
346346

347347
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_array_chunk, 0, 2, IS_ARRAY, 0)

‎ext/standard/tests/array/array_key_exists.phpt

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,11 @@ foreach ($search_arrays_v as $search_array) {
7070

7171
echo "\n*** Testing error conditions ***\n";
7272
// first args as array
73-
var_dump( array_key_exists(array(), array()) );
74-
// first argument as floating point value
75-
var_dump( array_key_exists(17.5, array(1,23) ) ) ;
73+
try {
74+
array_key_exists(array(), array());
75+
} catch (TypeError $exception) {
76+
echo $exception->getMessage() . "\n";
77+
}
7678

7779
echo "\n*** Testing operation on objects ***\n";
7880
class key_check
@@ -219,12 +221,7 @@ bool(false)
219221
bool(true)
220222

221223
*** Testing error conditions ***
222-
223-
Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
224-
bool(false)
225-
226-
Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
227-
bool(false)
224+
Illegal offset type
228225

229226
*** Testing operation on objects ***
230227
array_key_exists() expects parameter 2 to be array, object given

0 commit comments

Comments
 (0)