From 2302b14aab3bf01a9a87d2ee87951a13bc7752a0 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Thu, 26 Mar 2020 01:00:55 +0100 Subject: [PATCH] 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 --- UPGRADING | 3 + Zend/tests/bug78868.phpt | 2 +- ext/spl/php_spl.c | 88 +++++-------------- ext/spl/php_spl.stub.php | 2 +- ext/spl/php_spl_arginfo.h | 2 +- ext/spl/tests/spl_autoload_001.phpt | 11 +-- ext/spl/tests/spl_autoload_005.phpt | 11 +-- ext/spl/tests/spl_autoload_007.phpt | 25 +++--- ext/spl/tests/spl_autoload_008.phpt | 10 +-- ...ith_spl_autoloader_call_as_autoloader.phpt | 14 +++ .../spl_autoload_warn_on_false_do_throw.phpt | 16 ++++ 11 files changed, 83 insertions(+), 101 deletions(-) create mode 100644 ext/spl/tests/spl_autoload_throw_with_spl_autoloader_call_as_autoloader.phpt create mode 100644 ext/spl/tests/spl_autoload_warn_on_false_do_throw.phpt diff --git a/UPGRADING b/UPGRADING index 3c03ee4dd4..5fa471f32a 100644 --- 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 f6b66c765e..7502662c9e 100644 --- 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 b6b04cb269..2603de7cad 100644 --- 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 8e2a6edf0a..b2c570b5f1 100755 --- 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 f02c9d22e3..01d881d740 100644 --- 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 6025ac6538..50c815c4ad 100644 --- 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 53fbf27663..6fe71434f8 100644 --- 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 db5f394260..375ba2ef5e 100644 --- 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 eadc861ca1..53cf8d4ece 100644 --- 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 index 0000000000..cde49dbac3 --- /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-- +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 index 0000000000..f16976e78c --- /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-- + +--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) -- 2.40.0