Use ZPP callable check for spl_autoload_register.

This makes it always throw a TypeError, moreover this makes the
error message consistent.

Added a warning mentioning that the second parameter is now ignored
when passed false.

Closes GH-5301

diff --git a/UPGRADING b/UPGRADING
index 3c03ee4dd40ab9ef6a72b21d927a8f9aea436355..5fa471f32adc17ecb615dccc75734d4e66e2bf17 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -372,6 +372,9 @@ PHP 8.0 UPGRADE NOTES
   . SplDoublyLinkedList::push() now returns void instead of true
   . SplDoublyLinkedList::unshift() now returns void instead of true
   . SplQueue::enqueue() now returns void instead of true
+  . spl_autoload_register() will now always throw a TypeError on invalid
+    arguments, therefore the second argument $do_throw is ignored and a
+    notice will be emitted if it is set to false.
 
 - Standard:
   . assert() will no longer evaluate string arguments, instead they will be

diff --git a/Zend/tests/bug78868.phpt b/Zend/tests/bug78868.phpt
index f6b66c765e1020b57d3663859a597bc1bbdd97ba..7502662c9e739ffd1ab9d9d204d7d9dc62b4d8b8 100644 (file)
--- a/Zend/tests/bug78868.phpt
+++ b/Zend/tests/bug78868.phpt
@@ -21,7 +21,7 @@ function main_autoload($class_name) {
     eval("class B {const foo = 1;}");
 }
 
-spl_autoload_register('main_autoload', false);
+spl_autoload_register('main_autoload');
 
 $classA = new ReflectionClass("A");
 $props = $classA->getProperties();

diff --git a/ext/spl/php_spl.c b/ext/spl/php_spl.c
index b6b04cb269f48301b01d01732ba9e25af9bc00e4..2603de7cad472d6330d3ae24813c3ae6a27f9dab 100644 (file)
--- a/ext/spl/php_spl.c
+++ b/ext/spl/php_spl.c
@@ -508,86 +508,46 @@ PHP_FUNCTION(spl_autoload_call)
 PHP_FUNCTION(spl_autoload_register)
 {
        zend_string *func_name;
-       char *error = NULL;
        zend_string *lc_name;
-       zval *zcallable = NULL;
        zend_bool do_throw = 1;
        zend_bool prepend  = 0;
        zend_function *spl_func_ptr;
        autoload_func_info alfi;
        zend_object *obj_ptr;
+       zend_fcall_info fci = {0};
        zend_fcall_info_cache fcc;
 
-       if (zend_parse_parameters(ZEND_NUM_ARGS(), "|z!bb", &zcallable, &do_throw, &prepend) == FAILURE) {
-               RETURN_THROWS();
+       ZEND_PARSE_PARAMETERS_START(0, 3)
+               Z_PARAM_OPTIONAL
+               Z_PARAM_FUNC_OR_NULL(fci, fcc)
+               Z_PARAM_BOOL(do_throw)
+               Z_PARAM_BOOL(prepend)
+       ZEND_PARSE_PARAMETERS_END();
+
+       if (!do_throw) {
+               php_error_docref(NULL, E_NOTICE, "Argument #2 ($do_throw) has been ignored, "
+                       "spl_autoload_register() will always throw");
        }
 
-       if (zcallable) {
-               if (!zend_is_callable_ex(zcallable, NULL, 0, &func_name, &fcc, &error)) {
-                       alfi.ce = fcc.calling_scope;
-                       alfi.func_ptr = fcc.function_handler;
-                       obj_ptr = fcc.object;
-                       if (Z_TYPE_P(zcallable) == IS_ARRAY) {
-                               if (!obj_ptr && alfi.func_ptr && !(alfi.func_ptr->common.fn_flags & ZEND_ACC_STATIC)) {
-                                       if (do_throw) {
-                                               zend_throw_exception_ex(spl_ce_LogicException, 0, "Passed array specifies a non static method but no object (%s)", error);
-                                       }
-                                       if (error) {
-                                               efree(error);
-                                       }
-                                       zend_string_release_ex(func_name, 0);
-                                       RETURN_FALSE;
-                               } else if (do_throw) {
-                                       zend_throw_exception_ex(spl_ce_LogicException, 0, "Passed array does not specify %s %smethod (%s)", alfi.func_ptr ? "a callable" : "an existing", !obj_ptr ? "static " : "", error);
-                               }
-                               if (error) {
-                                       efree(error);
-                               }
-                               zend_string_release_ex(func_name, 0);
-                               RETURN_FALSE;
-                       } else if (Z_TYPE_P(zcallable) == IS_STRING) {
-                               if (do_throw) {
-                                       zend_throw_exception_ex(spl_ce_LogicException, 0, "Function '%s' not %s (%s)", ZSTR_VAL(func_name), alfi.func_ptr ? "callable" : "found", error);
-                               }
-                               if (error) {
-                                       efree(error);
-                               }
-                               zend_string_release_ex(func_name, 0);
-                               RETURN_FALSE;
-                       } else {
-                               if (do_throw) {
-                                       zend_throw_exception_ex(spl_ce_LogicException, 0, "Illegal value passed (%s)", error);
-                               }
-                               if (error) {
-                                       efree(error);
-                               }
-                               zend_string_release_ex(func_name, 0);
-                               RETURN_FALSE;
-                       }
-               } else if (fcc.function_handler->type == ZEND_INTERNAL_FUNCTION &&
-                                  fcc.function_handler->internal_function.handler == zif_spl_autoload_call) {
-                       if (do_throw) {
-                               zend_throw_exception_ex(spl_ce_LogicException, 0, "Function spl_autoload_call() cannot be registered");
-                       }
-                       if (error) {
-                               efree(error);
-                       }
-                       zend_string_release_ex(func_name, 0);
-                       RETURN_FALSE;
-               }
+       /* If first arg is not null */
+       if (ZEND_FCI_INITIALIZED(fci)) {
                alfi.ce = fcc.calling_scope;
                alfi.func_ptr = fcc.function_handler;
                obj_ptr = fcc.object;
-               if (error) {
-                       efree(error);
-               }
 
-               if (Z_TYPE_P(zcallable) == IS_OBJECT) {
-                       ZVAL_COPY(&alfi.closure, zcallable);
+               if (fcc.function_handler->type == ZEND_INTERNAL_FUNCTION &&
+                       fcc.function_handler->internal_function.handler == zif_spl_autoload_call) {
+                       zend_argument_value_error(1, "must not be the spl_autoload_call() function");
+                       RETURN_THROWS();
+               }
 
+               /* fci.function_name contains the callable zval */
+               func_name = zend_get_callable_name(&fci.function_name);
+               if (Z_TYPE(fci.function_name) == IS_OBJECT) {
+                       ZVAL_COPY(&alfi.closure, &fci.function_name);
                        lc_name = zend_string_alloc(ZSTR_LEN(func_name) + sizeof(uint32_t), 0);
                        zend_str_tolower_copy(ZSTR_VAL(lc_name), ZSTR_VAL(func_name), ZSTR_LEN(func_name));
-                       memcpy(ZSTR_VAL(lc_name) + ZSTR_LEN(func_name), &Z_OBJ_HANDLE_P(zcallable), sizeof(uint32_t));
+                       memcpy(ZSTR_VAL(lc_name) + ZSTR_LEN(func_name), &Z_OBJ_HANDLE(fci.function_name), sizeof(uint32_t));
                        ZSTR_VAL(lc_name)[ZSTR_LEN(lc_name)] = '\0';
                } else {
                        ZVAL_UNDEF(&alfi.closure);
@@ -599,7 +559,7 @@ PHP_FUNCTION(spl_autoload_register)
                                lc_name = zend_string_tolower(func_name);
                        }
                }
-               zend_string_release_ex(func_name, 0);
+               zend_string_release(func_name);
 
                if (SPL_G(autoload_functions) && zend_hash_exists(SPL_G(autoload_functions), lc_name)) {
                        if (!Z_ISUNDEF(alfi.closure)) {

diff --git a/ext/spl/php_spl.stub.php b/ext/spl/php_spl.stub.php
index 8e2a6edf0a93ea185bcdd885bb4b8fae1cd2806a..b2c570b5f135966549b06c0fff55cb18fdf28451 100755 (executable)
--- a/ext/spl/php_spl.stub.php
+++ b/ext/spl/php_spl.stub.php
@@ -17,7 +17,7 @@ function spl_autoload_extensions(?string $file_extensions = null): string {}
 
 function spl_autoload_functions(): array|false {}
 
-function spl_autoload_register($autoload_function = null, bool $throw = true, bool $prepend = false): bool {}
+function spl_autoload_register(?callable $autoload_function = null, bool $throw = true, bool $prepend = false): bool {}
 
 function spl_autoload_unregister($autoload_function): bool {}
 

diff --git a/ext/spl/php_spl_arginfo.h b/ext/spl/php_spl_arginfo.h
index f02c9d22e3284b4f41d8e9adfe5bab501cc8fc45..01d881d7407d01f64fcf94d14dbe575096736f2e 100644 (file)
--- a/ext/spl/php_spl_arginfo.h
+++ b/ext/spl/php_spl_arginfo.h
@@ -29,7 +29,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_spl_autoload_functions, 0, 0, MA
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_spl_autoload_register, 0, 0, _IS_BOOL, 0)
-       ZEND_ARG_INFO_WITH_DEFAULT_VALUE(0, autoload_function, "null")
+       ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, autoload_function, IS_CALLABLE, 1, "null")
        ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, throw, _IS_BOOL, 0, "true")
        ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, prepend, _IS_BOOL, 0, "false")
 ZEND_END_ARG_INFO()

diff --git a/ext/spl/tests/spl_autoload_001.phpt b/ext/spl/tests/spl_autoload_001.phpt
index 6025ac653833b54b26b876bc547f22ad64772f89..50c815c4adf494c146c29321a610ec42897cd3ff 100644 (file)
--- a/ext/spl/tests/spl_autoload_001.phpt
+++ b/ext/spl/tests/spl_autoload_001.phpt
@@ -64,13 +64,10 @@ var_dump(class_exists("TestClass", true));
 
 echo "===NOFUNCTION===\n";
 
-try
-{
+try {
     spl_autoload_register("unavailable_autoload_function");
-}
-catch(Exception $e)
-{
-    echo 'Exception: ' . $e->getMessage() . "\n";
+} catch(\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
 }
 
 ?>
@@ -103,4 +100,4 @@ TestFunc2(TestClass)
 %stestclass.class.inc
 bool(true)
 ===NOFUNCTION===
-Exception: Function 'unavailable_autoload_function' not found (function 'unavailable_autoload_function' not found or invalid function name)
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, function 'unavailable_autoload_function' not found or invalid function name

diff --git a/ext/spl/tests/spl_autoload_005.phpt b/ext/spl/tests/spl_autoload_005.phpt
index 53fbf27663fe2fec3d73dab277cb8a10167f514e..6fe71434f8376016d553457791d6f154a05a3554 100644 (file)
--- a/ext/spl/tests/spl_autoload_005.phpt
+++ b/ext/spl/tests/spl_autoload_005.phpt
@@ -19,13 +19,10 @@ class MyAutoLoader {
         }
 }
 
-try
-{
+try {
     spl_autoload_register(array('MyAutoLoader', 'autoLoad'), true);
-}
-catch(Exception $e)
-{
-    echo 'Exception: ' . $e->getMessage() . "\n";
+} catch(\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
 }
 
 // and
@@ -46,7 +43,7 @@ catch(Exception $e)
 
 ?>
 --EXPECT--
-Exception: Passed array specifies a non static method but no object (non-static method MyAutoLoader::autoLoad() cannot be called statically)
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, non-static method MyAutoLoader::autoLoad() cannot be called statically
 MyAutoLoader::autoLoad(TestClass)
 MyAutoLoader::autoThrow(TestClass)
 Exception: Unavailable

diff --git a/ext/spl/tests/spl_autoload_007.phpt b/ext/spl/tests/spl_autoload_007.phpt
index db5f394260f35fb5d0515c8bae73597690bab53a..375ba2ef5ecec88735e577127d9cd17d2821d6df 100644 (file)
--- a/ext/spl/tests/spl_autoload_007.phpt
+++ b/ext/spl/tests/spl_autoload_007.phpt
@@ -40,31 +40,28 @@ $funcs = array(
 foreach($funcs as $idx => $func)
 {
     if ($idx) echo "\n";
-    try
-    {
+    try {
         var_dump($func);
         spl_autoload_register($func);
         echo "ok\n";
-    }
-    catch (Exception $e)
-    {
-        echo $e->getMessage() . "\n";
+    } catch(\TypeError $e) {
+        echo $e->getMessage() . \PHP_EOL;
     }
 }
 
 ?>
 --EXPECTF--
 string(22) "MyAutoLoader::notExist"
-Function 'MyAutoLoader::notExist' not found (class 'MyAutoLoader' does not have a method 'notExist')
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, class 'MyAutoLoader' does not have a method 'notExist'
 
 string(22) "MyAutoLoader::noAccess"
-Function 'MyAutoLoader::noAccess' not callable (cannot access protected method MyAutoLoader::noAccess())
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, cannot access protected method MyAutoLoader::noAccess()
 
 string(22) "MyAutoLoader::autoLoad"
 ok
 
 string(22) "MyAutoLoader::dynaLoad"
-Function 'MyAutoLoader::dynaLoad' not callable (non-static method MyAutoLoader::dynaLoad() cannot be called statically)
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, non-static method MyAutoLoader::dynaLoad() cannot be called statically
 
 array(2) {
   [0]=>
@@ -72,7 +69,7 @@ array(2) {
   [1]=>
   string(8) "notExist"
 }
-Passed array does not specify an existing static method (class 'MyAutoLoader' does not have a method 'notExist')
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, class 'MyAutoLoader' does not have a method 'notExist'
 
 array(2) {
   [0]=>
@@ -80,7 +77,7 @@ array(2) {
   [1]=>
   string(8) "noAccess"
 }
-Passed array does not specify a callable static method (cannot access protected method MyAutoLoader::noAccess())
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, cannot access protected method MyAutoLoader::noAccess()
 
 array(2) {
   [0]=>
@@ -96,7 +93,7 @@ array(2) {
   [1]=>
   string(8) "dynaLoad"
 }
-Passed array specifies a non static method but no object (non-static method MyAutoLoader::dynaLoad() cannot be called statically)
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, non-static method MyAutoLoader::dynaLoad() cannot be called statically
 
 array(2) {
   [0]=>
@@ -105,7 +102,7 @@ array(2) {
   [1]=>
   string(8) "notExist"
 }
-Passed array does not specify an existing method (class 'MyAutoLoader' does not have a method 'notExist')
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, class 'MyAutoLoader' does not have a method 'notExist'
 
 array(2) {
   [0]=>
@@ -114,7 +111,7 @@ array(2) {
   [1]=>
   string(8) "noAccess"
 }
-Passed array does not specify a callable static method (cannot access protected method MyAutoLoader::noAccess())
+spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, cannot access protected method MyAutoLoader::noAccess()
 
 array(2) {
   [0]=>

diff --git a/ext/spl/tests/spl_autoload_008.phpt b/ext/spl/tests/spl_autoload_008.phpt
index eadc861ca16a2c44a0c3b954bc84c42d28cb579a..53cf8d4ecefd8795e3081ea52669a870aebcc460 100644 (file)
--- a/ext/spl/tests/spl_autoload_008.phpt
+++ b/ext/spl/tests/spl_autoload_008.phpt
@@ -52,10 +52,8 @@ foreach($funcs as $idx => $func)
 
             var_dump(class_exists("NoExistingTestClass", true));
         }
-    }
-    catch (Exception $e)
-    {
-        echo get_class($e) . ": " . $e->getMessage() . "\n";
+    } catch(\TypeError|\Exception $e) {
+        echo get_class($e) . ': ' . $e->getMessage() . \PHP_EOL;
     }
 
     spl_autoload_unregister($func);
@@ -78,7 +76,7 @@ Exception: Bla
 int(0)
 ====2====
 string(22) "MyAutoLoader::dynaLoad"
-LogicException: Function 'MyAutoLoader::dynaLoad' not callable (non-static method MyAutoLoader::dynaLoad() cannot be called statically)
+TypeError: spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, non-static method MyAutoLoader::dynaLoad() cannot be called statically
 int(0)
 ====3====
 array(2) {
@@ -98,7 +96,7 @@ array(2) {
   [1]=>
   string(8) "dynaLoad"
 }
-LogicException: Passed array specifies a non static method but no object (non-static method MyAutoLoader::dynaLoad() cannot be called statically)
+TypeError: spl_autoload_register(): Argument #1 ($autoload_function) must be a valid callback, non-static method MyAutoLoader::dynaLoad() cannot be called statically
 int(0)
 ====5====
 array(2) {

diff --git a/ext/spl/tests/spl_autoload_throw_with_spl_autoloader_call_as_autoloader.phpt b/ext/spl/tests/spl_autoload_throw_with_spl_autoloader_call_as_autoloader.phpt
new file mode 100644 (file)
index 0000000..cde49db
--- /dev/null
+++ b/ext/spl/tests/spl_autoload_throw_with_spl_autoloader_call_as_autoloader.phpt
@@ -0,0 +1,14 @@
+--TEST--
+spl_autoload_register() function - warn when using spl_autoload_call() as the autoloading function
+--FILE--
+<?php
+
+try {
+    spl_autoload_register('spl_autoload_call');
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+
+?>
+--EXPECT--
+spl_autoload_register(): Argument #1 ($autoload_function) must not be the spl_autoload_call() function

diff --git a/ext/spl/tests/spl_autoload_warn_on_false_do_throw.phpt b/ext/spl/tests/spl_autoload_warn_on_false_do_throw.phpt
new file mode 100644 (file)
index 0000000..f16976e
--- /dev/null
+++ b/ext/spl/tests/spl_autoload_warn_on_false_do_throw.phpt
@@ -0,0 +1,16 @@
+--TEST--
+spl_autoload_register() function - warn when using false as second argument for spl_autoload_register()
+--FILE--
+<?php
+function customAutolader($class) {
+    require_once __DIR__ . '/testclass.class.inc';
+}
+spl_autoload_register('customAutolader', false);
+
+spl_autoload_call('TestClass');
+var_dump(class_exists('TestClass', false));
+?>
+--EXPECTF--
+Notice: spl_autoload_register(): Argument #2 ($do_throw) has been ignored, spl_autoload_register() will always throw in %s on line %d
+%stestclass.class.inc
+bool(true)