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.

diff --git a/.gitignore b/.gitignore
index df0fc80a5c5e7b77ca7013d83ec7ba0d607025e3..587f4abc42593c96e70cd485591673647ba263b5 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -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.*
 

diff --git a/Zend/tests/const_array_with_resource_key.phpt b/Zend/tests/const_array_with_resource_key.phpt
index 0bf546af50930620b6932fcb1ff8e97f1d976871..1e3dee473403fedc8ea058495c612cd147c47931 100644 (file)
--- a/Zend/tests/const_array_with_resource_key.phpt
+++ b/Zend/tests/const_array_with_resource_key.phpt
@@ -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)

diff --git a/Zend/tests/isset_003.phpt b/Zend/tests/isset_003.phpt
index c80a174dafb5b657a3acb7b386be0ce1e9cbba8e..7b29c1e6b8758a86fa23d1d3708cb1b7a0911aab 100644 (file)
--- a/Zend/tests/isset_003.phpt
+++ b/Zend/tests/isset_003.phpt
@@ -1,5 +1,5 @@
 --TEST--
-Testing isset accessing undefined array itens and properties
+Testing isset accessing undefined array items and properties
 --FILE--
 <?php
 

diff --git a/Zend/tests/isset_array.phpt b/Zend/tests/isset_array.phpt
new file mode 100644 (file)
index 0000000..c7c1876
--- /dev/null
+++ b/Zend/tests/isset_array.phpt
@@ -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

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
index 5781a95bfd027d75db345b1cec0e06699b907ff2..387f6e1a9822edc6b592a46a941befa8b52c1aa3 100644 (file)
--- a/Zend/zend_API.c
+++ b/Zend/zend_API.c
@@ -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:

diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c
index 660e6d5b6f11d3ad53a43939a89d04554983dbf8..f0b524b30e82971f58a2a00fbe572232239ed215 100644 (file)
--- a/Zend/zend_ast.c
+++ b/Zend/zend_ast.c
@@ -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:
@@ -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");

diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
index 345335b70890cd1a2b4561330049d6ab55bd444d..8278f698cdda41c7798fd1bf695ba6083340a607 100644 (file)
--- a/Zend/zend_execute.c
+++ b/Zend/zend_execute.c
@@ -2369,6 +2369,7 @@ str_idx:
                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) {
@@ -2478,6 +2479,19 @@ num_key:
        } 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();
@@ -2485,7 +2499,7 @@ num_key:
                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;
        }
 }

diff --git a/ext/opcache/jit/zend_jit_helpers.c b/ext/opcache/jit/zend_jit_helpers.c
index da96660a10bc3ae701f09886273f9c94010839ba..f581841aab5ced256de45ae3800fffbceb6db831 100644 (file)
--- a/ext/opcache/jit/zend_jit_helpers.c
+++ b/ext/opcache/jit/zend_jit_helpers.c
@@ -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:

diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c
index 4e9f9bde3ce0d2a0f560369c10b3b823a85df7b9..bf33287fd0228ddad4536f64aefa4fe0e3aff51a 100644 (file)
--- a/ext/spl/spl_array.c
+++ b/ext/spl/spl_array.c
@@ -348,7 +348,7 @@ fetch_dim_string:
                }
                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:

diff --git a/ext/spl/tests/bug62978.phpt b/ext/spl/tests/bug62978.phpt
index ec6275346e218ac5f2fc1589acc6912777508ad7..7e61d27b8950706f2693bfe706d83bcbd150dfd1 100644 (file)
--- a/ext/spl/tests/bug62978.phpt
+++ b/ext/spl/tests/bug62978.phpt
@@ -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

diff --git a/ext/standard/array.c b/ext/standard/array.c
index ffa695f26dc15ff73da21c154c8736c19e5f3bea..b8e727d103ad4bb8017f09575fa5dc12e0021797 100644 (file)
--- a/ext/standard/array.c
+++ b/ext/standard/array.c
@@ -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 */
@@ -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;
@@ -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;
@@ -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;
                }
 
@@ -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;
        }
 }
 /* }}} */

diff --git a/ext/standard/basic_functions.stub.php b/ext/standard/basic_functions.stub.php
index de4d22bc3cea199f1fe4cb0b4e2610f9667be115..c596d216e8173248e2594963d0af412fcc292b4a 100755 (executable)
--- a/ext/standard/basic_functions.stub.php
+++ b/ext/standard/basic_functions.stub.php
@@ -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 {}
@@ -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 {}
 

diff --git a/ext/standard/basic_functions_arginfo.h b/ext/standard/basic_functions_arginfo.h
index 103b44f1526a82ab70ad44a6d7e54db9053f2fe0..200891a563042e5b17429e8df9c85ffd2b52bc88 100755 (executable)
--- a/ext/standard/basic_functions_arginfo.h
+++ b/ext/standard/basic_functions_arginfo.h
@@ -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)

diff --git a/ext/standard/tests/array/array_key_exists.phpt b/ext/standard/tests/array/array_key_exists.phpt
index 551ef85a6704c142c540029688b8edee42ab0e77..a436e7363ef9406cf3fb526be0187bfb3d3c5e19 100644 (file)
--- a/ext/standard/tests/array/array_key_exists.phpt
+++ b/ext/standard/tests/array/array_key_exists.phpt
@@ -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
@@ -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

diff --git a/ext/standard/tests/array/array_key_exists_variation1.phpt b/ext/standard/tests/array/array_key_exists_variation1.phpt
index c6e8ba78066fad10955f81aefda00b8c7b4e906d..2f25dbadee58d6ff34b525086e4c5d281680a5d1 100644 (file)
--- a/ext/standard/tests/array/array_key_exists_variation1.phpt
+++ b/ext/standard/tests/array/array_key_exists_variation1.phpt
@@ -15,7 +15,7 @@ Test array_key_exists() function : usage variations - Pass different data types
 echo "*** Testing array_key_exists() : usage variations ***\n";
 
 // Initialise function arguments not being substituted
-$search = array ('zero', 'key' => 'val', 'two');
+$search = array ('zero', 'key' => 'val', 'two', 10 => 'value');
 
 //get an unset variable
 $unset_var = 10;
@@ -90,7 +90,11 @@ $inputs = array(
 $iterator = 1;
 foreach($inputs as $input) {
   echo "\n-- Iteration $iterator --\n";
-  var_dump( array_key_exists($input, $search) );
+  try {
+      var_dump( array_key_exists($input, $search) );
+  } catch (TypeError $exception) {
+      echo $exception->getMessage() . "\n";
+  }
   $iterator++;
 };
 
@@ -114,29 +118,19 @@ bool(false)
 bool(false)
 
 -- Iteration 5 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 
 -- Iteration 6 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
 bool(false)
 
 -- Iteration 7 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
 bool(false)
 
 -- Iteration 8 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 
 -- Iteration 9 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 
 -- Iteration 10 --
 bool(false)
@@ -145,24 +139,16 @@ bool(false)
 bool(false)
 
 -- Iteration 12 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 
 -- Iteration 13 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 
 -- Iteration 14 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 
 -- Iteration 15 --
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 
 -- Iteration 16 --
 bool(false)
@@ -171,9 +157,7 @@ bool(false)
 bool(false)
 
 -- Iteration 18 --
-
-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
 
 -- Iteration 19 --
 bool(true)
@@ -185,9 +169,7 @@ bool(true)
 bool(true)
 
 -- Iteration 22 --
-
-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
 
 -- Iteration 23 --
 bool(false)
@@ -197,6 +179,6 @@ bool(false)
 
 -- Iteration 25 --
 
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
+Warning: Resource ID#%d used as offset, casting to integer (%d) in %s on line %d
 bool(false)
 Done

diff --git a/ext/standard/tests/array/array_key_exists_variation3.phpt b/ext/standard/tests/array/array_key_exists_variation3.phpt
index 867b4d98486ec180dc0b2ad5840a820ba652f4c7..11f0125be2c3c9d8bda4de22f3437542b9de1a05 100644 (file)
--- a/ext/standard/tests/array/array_key_exists_variation3.phpt
+++ b/ext/standard/tests/array/array_key_exists_variation3.phpt
@@ -23,7 +23,11 @@ $iterator = 1;
 foreach($keys as $key) {
        echo "\n-- Iteration $iterator --\n";
        echo "Pass float as \$key:\n";
-       var_dump(array_key_exists($key, $search));
+       try {
+               var_dump(array_key_exists($key, $search));
+       } catch (TypeError $exception) {
+               echo $exception->getMessage() . "\n";
+       }
        echo "Cast float to int:\n";
        var_dump(array_key_exists((int)$key, $search));
 }
@@ -35,25 +39,19 @@ echo "Done";
 
 -- Iteration 1 --
 Pass float as $key:
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 Cast float to int:
 bool(true)
 
 -- Iteration 1 --
 Pass float as $key:
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 Cast float to int:
 bool(true)
 
 -- Iteration 1 --
 Pass float as $key:
-
-Warning: array_key_exists(): The first argument should be either a string or an integer in %s on line %d
-bool(false)
+bool(true)
 Cast float to int:
 bool(true)
 Done