Skip to content

Promote the warning of array_key_exists() to exception #4887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@ php
# Test results generated by `./run-tests.php`
php_test_results_*.txt

# Temporary test information generated by `./run-tests.php`
/run-test-info.php

# Temporary POST data placeholder files generated by `./run-tests.php`
phpt.*

Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/const_array_with_resource_key.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var_dump(FOO);

?>
--EXPECTF--
Notice: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
array(1) {
[%d]=>
int(42)
Expand Down
2 changes: 1 addition & 1 deletion Zend/tests/isset_003.phpt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
Testing isset accessing undefined array itens and properties
Testing isset accessing undefined array items and properties
--FILE--
<?php

Expand Down
48 changes: 48 additions & 0 deletions Zend/tests/isset_array.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
--TEST--
Using isset() with arrays
--FILE--
<?php

$array = [
0 => true,
"a" => true,
];

var_dump(isset($array[0]));

var_dump(isset($array["a"]));

var_dump(isset($array[false]));

var_dump(isset($array[0.6]));

var_dump(isset($array[true]));

var_dump(isset($array[null]));

var_dump(isset($array[STDIN]));

try {
isset($array[[]]);
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}

try {
isset($array[new stdClass()]);
} catch (TypeError $exception) {
echo $exception->getMessage() . "\n";
}
?>
--EXPECTF--
bool(true)
bool(true)
bool(true)
bool(true)
bool(false)
bool(false)

Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
bool(false)
Illegal offset type in isset or empty
Illegal offset type in isset or empty
2 changes: 1 addition & 1 deletion Zend/zend_API.c
Original file line number Diff line number Diff line change
Expand Up @@ -1531,7 +1531,7 @@ ZEND_API int array_set_zval_key(HashTable *ht, zval *key, zval *value) /* {{{ */
result = zend_symtable_update(ht, ZSTR_EMPTY_ALLOC(), value);
break;
case IS_RESOURCE:
zend_error(E_NOTICE, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(key), Z_RES_HANDLE_P(key));
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(key), Z_RES_HANDLE_P(key));
result = zend_hash_index_update(ht, Z_RES_HANDLE_P(key), value);
break;
case IS_FALSE:
Expand Down
4 changes: 2 additions & 2 deletions Zend/zend_ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ static int zend_ast_add_array_element(zval *result, zval *offset, zval *expr)
zend_hash_index_update(Z_ARRVAL_P(result), zend_dval_to_lval(Z_DVAL_P(offset)), expr);
break;
case IS_RESOURCE:
zend_error(E_NOTICE, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(offset), Z_RES_HANDLE_P(offset));
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(offset), Z_RES_HANDLE_P(offset));
zend_hash_index_update(Z_ARRVAL_P(result), Z_RES_HANDLE_P(offset), expr);
break;
default:
Expand All @@ -451,7 +451,7 @@ static int zend_ast_add_unpacked_element(zval *result, zval *expr) {
HashTable *ht = Z_ARRVAL_P(expr);
zval *val;
zend_string *key;

ZEND_HASH_FOREACH_STR_KEY_VAL(ht, key, val) {
if (key) {
zend_throw_error(NULL, "Cannot unpack array with string keys");
Expand Down
16 changes: 15 additions & 1 deletion Zend/zend_execute.c
Original file line number Diff line number Diff line change
Expand Up @@ -2369,6 +2369,7 @@ static zend_never_inline zval* ZEND_FASTCALL zend_find_array_dim_slow(HashTable
hval = 1;
goto num_idx;
} else if (Z_TYPE_P(offset) == IS_RESOURCE) {
zend_use_resource_as_offset(offset);
hval = Z_RES_HANDLE_P(offset);
goto num_idx;
} else if (/*OP2_TYPE == IS_CV &&*/ Z_TYPE_P(offset) == IS_UNDEF) {
Expand Down Expand Up @@ -2478,14 +2479,27 @@ static zend_never_inline zend_bool ZEND_FASTCALL zend_array_key_exists_fast(Hash
} else if (EXPECTED(Z_ISREF_P(key))) {
key = Z_REFVAL_P(key);
goto try_again;
} else if (Z_TYPE_P(key) == IS_DOUBLE) {
hval = zend_dval_to_lval(Z_DVAL_P(key));
goto num_key;
} else if (Z_TYPE_P(key) == IS_FALSE) {
hval = 0;
goto num_key;
} else if (Z_TYPE_P(key) == IS_TRUE) {
hval = 1;
goto num_key;
} else if (Z_TYPE_P(key) == IS_RESOURCE) {
zend_use_resource_as_offset(key);
hval = Z_RES_HANDLE_P(key);
goto num_key;
} else if (Z_TYPE_P(key) <= IS_NULL) {
if (UNEXPECTED(Z_TYPE_P(key) == IS_UNDEF)) {
ZVAL_UNDEFINED_OP1();
}
str = ZSTR_EMPTY_ALLOC();
goto str_key;
} else {
zend_error(E_WARNING, "array_key_exists(): The first argument should be either a string or an integer");
zend_type_error("Illegal offset type");
return 0;
}
}
Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/jit/zend_jit_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ static int ZEND_FASTCALL zend_jit_fetch_dim_isset_helper(zend_array *ht, zval *d
hval = zend_dval_to_lval(Z_DVAL_P(dim));
goto num_index;
case IS_RESOURCE:
//zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(dim), Z_RES_HANDLE_P(dim));
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(dim), Z_RES_HANDLE_P(dim));
hval = Z_RES_HANDLE_P(dim);
goto num_index;
case IS_FALSE:
Expand Down
2 changes: 1 addition & 1 deletion ext/spl/spl_array.c
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ static zval *spl_array_get_dimension_ptr(int check_inherited, spl_array_object *
}
return retval;
case IS_RESOURCE:
zend_error(E_NOTICE, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_P(offset)->handle, Z_RES_P(offset)->handle);
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_P(offset)->handle, Z_RES_P(offset)->handle);
index = Z_RES_P(offset)->handle;
goto num_index;
case IS_DOUBLE:
Expand Down
2 changes: 1 addition & 1 deletion ext/spl/tests/bug62978.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ Notice: Undefined index: epic_magic in %sbug62978.php on line %d
NULL
bool(false)

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

Notice: Undefined offset: %d in %sbug62978.php on line %d
NULL
23 changes: 20 additions & 3 deletions ext/standard/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "php_math.h"
#include "zend_smart_str.h"
#include "zend_bitset.h"
#include "zend_exceptions.h"
#include "ext/spl/spl_array.h"

/* {{{ defines */
Expand Down Expand Up @@ -765,6 +766,7 @@ PHP_FUNCTION(count)

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

/* If There's no handler and it doesn't implement Countable then add a warning */
/* Intentionally not converted to an exception */
php_error_docref(NULL, E_WARNING, "Parameter must be an array or an object that implements Countable");
RETURN_LONG(1);
break;
}
default:
/* Intentionally not converted to an exception */
php_error_docref(NULL, E_WARNING, "Parameter must be an array or an object that implements Countable");
RETURN_LONG(1);
break;
Expand Down Expand Up @@ -5212,7 +5216,7 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_
param_spec = "+f";
diff_data_compare_func = php_array_user_compare;
} else {
php_error_docref(NULL, E_WARNING, "data_compare_type is %d. This should never happen. Please report as a bug", data_compare_type);
ZEND_ASSERT(0 && "Invalid data_compare_type");
return;
}

Expand Down Expand Up @@ -6349,9 +6353,22 @@ PHP_FUNCTION(array_key_exists)
case IS_NULL:
RETVAL_BOOL(zend_hash_exists_ind(ht, ZSTR_EMPTY_ALLOC()));
break;
case IS_DOUBLE:
RETVAL_BOOL(zend_hash_index_exists(ht, zend_dval_to_lval(Z_DVAL_P(key))));
break;
case IS_FALSE:
RETVAL_BOOL(zend_hash_index_exists(ht, 0));
break;
case IS_TRUE:
RETVAL_BOOL(zend_hash_index_exists(ht, 1));
break;
case IS_RESOURCE:
zend_error(E_WARNING, "Resource ID#%d used as offset, casting to integer (%d)", Z_RES_HANDLE_P(key), Z_RES_HANDLE_P(key));
RETVAL_BOOL(zend_hash_index_exists(ht, Z_RES_HANDLE_P(key)));
break;
default:
php_error_docref(NULL, E_WARNING, "The first argument should be either a string or an integer");
RETVAL_FALSE;
zend_type_error("Illegal offset type");
break;
}
}
/* }}} */
Expand Down
9 changes: 3 additions & 6 deletions ext/standard/basic_functions.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function krsort(array &$arg, int $sort_flags = SORT_REGULAR): bool {}

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

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

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

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

/**
* @param int|string $key
* @param array|object $search
*/
function array_key_exists($key, $search): bool {}
/** @param mixed $key */
function array_key_exists($key, array $search): bool {}

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

Expand Down
2 changes: 1 addition & 1 deletion ext/standard/basic_functions_arginfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_array_key_exists, 0, 2, _IS_BOOL, 0)
ZEND_ARG_INFO(0, key)
ZEND_ARG_INFO(0, search)
ZEND_ARG_TYPE_INFO(0, search, IS_ARRAY, 0)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_array_chunk, 0, 2, IS_ARRAY, 0)
Expand Down
15 changes: 6 additions & 9 deletions ext/standard/tests/array/array_key_exists.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,11 @@ foreach ($search_arrays_v as $search_array) {

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

echo "\n*** Testing operation on objects ***\n";
class key_check
Expand Down Expand Up @@ -219,12 +221,7 @@ bool(false)
bool(true)

*** Testing error conditions ***

Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
bool(false)

Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
bool(false)
Illegal offset type

*** Testing operation on objects ***
array_key_exists() expects parameter 2 to be array, object given
Expand Down
Loading