From b3e33be44391bdd9816a7ec6e68394327d7da24a Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 21 Mar 2023 20:41:18 +0100 Subject: [PATCH] Forward shutdown exceptions to user error handlers Fixes GH-10695 Closes GH-110905 --- UPGRADING | 1 + Zend/Optimizer/zend_func_infos.h | 1 + Zend/tests/gh10695_1.phpt | 16 ++++++++++++++++ Zend/tests/gh10695_2.phpt | 18 ++++++++++++++++++ Zend/tests/gh10695_3.phpt | 19 +++++++++++++++++++ Zend/tests/gh10695_4.phpt | 19 +++++++++++++++++++ Zend/tests/gh10695_5.phpt | 17 +++++++++++++++++ Zend/tests/gh10695_6.phpt | 14 ++++++++++++++ Zend/tests/gh10695_7.phpt | 16 ++++++++++++++++ Zend/zend.c | 3 +++ Zend/zend_exceptions.c | 6 +++++- Zend/zend_execute_API.c | 6 +++++- ext/zend_test/test.c | 18 ++++++++++++++++++ ext/zend_test/test.stub.php | 3 +++ ext/zend_test/test_arginfo.h | 7 ++++++- ext/zend_test/tests/gh10695_1.phpt | 14 ++++++++++++++ ext/zend_test/tests/gh10695_2.phpt | 18 ++++++++++++++++++ 17 files changed, 193 insertions(+), 3 deletions(-) create mode 100644 Zend/tests/gh10695_1.phpt create mode 100644 Zend/tests/gh10695_2.phpt create mode 100644 Zend/tests/gh10695_3.phpt create mode 100644 Zend/tests/gh10695_4.phpt create mode 100644 Zend/tests/gh10695_5.phpt create mode 100644 Zend/tests/gh10695_6.phpt create mode 100644 Zend/tests/gh10695_7.phpt create mode 100644 ext/zend_test/tests/gh10695_1.phpt create mode 100644 ext/zend_test/tests/gh10695_2.phpt diff --git a/UPGRADING b/UPGRADING index 4d292bb8100..1f6df9e8803 100644 --- a/UPGRADING +++ b/UPGRADING @@ -107,6 +107,7 @@ PHP 8.3 UPGRADE NOTES longer accepts paths containing the parent directory (`..`). Previously, only paths starting with `..` were disallowed. This could easily be circumvented by prepending `./` to the path. + . User exception handlers now catch exceptions during shutdown. - Dom: . Changed DOMCharacterData::appendData() tentative return type to true. diff --git a/Zend/Optimizer/zend_func_infos.h b/Zend/Optimizer/zend_func_infos.h index 34b8b9c4cbb..a77ec0a49f7 100644 --- a/Zend/Optimizer/zend_func_infos.h +++ b/Zend/Optimizer/zend_func_infos.h @@ -689,6 +689,7 @@ static const func_info_t func_infos[] = { F1("serialize", MAY_BE_STRING), F1("xml_error_string", MAY_BE_STRING|MAY_BE_NULL), F1("xml_parser_get_option", MAY_BE_STRING|MAY_BE_LONG|MAY_BE_BOOL), + FN("zend_test_create_throwing_resource", MAY_BE_RESOURCE), FN("zip_open", MAY_BE_RESOURCE|MAY_BE_LONG|MAY_BE_FALSE), FN("zip_read", MAY_BE_RESOURCE|MAY_BE_FALSE), F1("ob_gzhandler", MAY_BE_STRING|MAY_BE_FALSE), diff --git a/Zend/tests/gh10695_1.phpt b/Zend/tests/gh10695_1.phpt new file mode 100644 index 00000000000..308658cc102 --- /dev/null +++ b/Zend/tests/gh10695_1.phpt @@ -0,0 +1,16 @@ +--TEST-- +GH-10695: Exceptions in register_shutdown_function() are caught by set_exception_handler() +--FILE-- +getMessage() . "\n"; +}); + +register_shutdown_function(function () { + echo "register_shutdown_function()\n"; + throw new \Exception('shutdown'); +}); +?> +--EXPECT-- +register_shutdown_function() +Caught: shutdown diff --git a/Zend/tests/gh10695_2.phpt b/Zend/tests/gh10695_2.phpt new file mode 100644 index 00000000000..ffb1ba0fa57 --- /dev/null +++ b/Zend/tests/gh10695_2.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-10695: Exceptions in destructor during shutdown are caught +--FILE-- +getMessage() . "\n"; +}); + +const FOO = new Foo; +?> +--EXPECT-- +Caught: Foo::__destruct diff --git a/Zend/tests/gh10695_3.phpt b/Zend/tests/gh10695_3.phpt new file mode 100644 index 00000000000..5c5fb1645b8 --- /dev/null +++ b/Zend/tests/gh10695_3.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-10695: Uncaught exceptions are not caught again +--FILE-- +getMessage() . "\n"; + }); +}); + +throw new \Exception('main'); +?> +--EXPECTF-- +Fatal error: Uncaught Exception: main in %s:%d +Stack trace: +#0 {main} + thrown in %s on line %d +shutdown diff --git a/Zend/tests/gh10695_4.phpt b/Zend/tests/gh10695_4.phpt new file mode 100644 index 00000000000..c97477d4440 --- /dev/null +++ b/Zend/tests/gh10695_4.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-10695: Exception handlers are not called twice +--FILE-- +getMessage() . "\n"; + throw new \Exception('exception handler'); +}); + +throw new \Exception('main'); +?> +--EXPECTF-- +Caught: main + +Fatal error: Uncaught Exception: exception handler in %s:%d +Stack trace: +#0 [internal function]: {closure}(Object(Exception)) +#1 {main} + thrown in %s on line %d diff --git a/Zend/tests/gh10695_5.phpt b/Zend/tests/gh10695_5.phpt new file mode 100644 index 00000000000..d287cbf5914 --- /dev/null +++ b/Zend/tests/gh10695_5.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-10695: Exception handlers can register another exception handler +--FILE-- +getMessage() . "\n"; + set_exception_handler(function (\Throwable $exception) { + echo 'Caught: ' . $exception->getMessage() . "\n"; + }); + throw new \Exception('exception handler'); +}); + +throw new \Exception('main'); +?> +--EXPECT-- +Caught: main +Caught: exception handler diff --git a/Zend/tests/gh10695_6.phpt b/Zend/tests/gh10695_6.phpt new file mode 100644 index 00000000000..e0fc05e0d76 --- /dev/null +++ b/Zend/tests/gh10695_6.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-10695: Exceptions from output buffering handlers during shutdown are caught +--FILE-- +getMessage() . "\n"; +}); + +ob_start(function () { + throw new \Exception('ob_start'); +}); +?> +--EXPECT-- +Caught: ob_start diff --git a/Zend/tests/gh10695_7.phpt b/Zend/tests/gh10695_7.phpt new file mode 100644 index 00000000000..12ec5fc4541 --- /dev/null +++ b/Zend/tests/gh10695_7.phpt @@ -0,0 +1,16 @@ +--TEST-- +GH-10695: Exceptions in error handlers during shutdown are caught +--FILE-- +getMessage() . "\n"; +}); +set_error_handler(function ($errno, $errstr) { + throw new \Exception($errstr); +}); +register_shutdown_function(function () { + trigger_error('main', E_USER_ERROR); +}); +?> +--EXPECT-- +Caught: main diff --git a/Zend/zend.c b/Zend/zend.c index 0e3cfb4381f..ee3f5272c96 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -1819,6 +1819,7 @@ ZEND_API ZEND_COLD void zend_user_exception_handler(void) /* {{{ */ EG(exception) = NULL; ZVAL_OBJ(¶ms[0], old_exception); ZVAL_COPY_VALUE(&orig_user_exception_handler, &EG(user_exception_handler)); + ZVAL_UNDEF(&EG(user_exception_handler)); if (call_user_function(CG(function_table), NULL, &orig_user_exception_handler, &retval2, 1, params) == SUCCESS) { zval_ptr_dtor(&retval2); @@ -1830,6 +1831,8 @@ ZEND_API ZEND_COLD void zend_user_exception_handler(void) /* {{{ */ } else { EG(exception) = old_exception; } + + zval_ptr_dtor(&orig_user_exception_handler); } /* }}} */ ZEND_API zend_result zend_execute_scripts(int type, zval *retval, int file_count, ...) /* {{{ */ diff --git a/Zend/zend_exceptions.c b/Zend/zend_exceptions.c index 8dea4804233..ed062958f50 100644 --- a/Zend/zend_exceptions.c +++ b/Zend/zend_exceptions.c @@ -199,7 +199,11 @@ ZEND_API ZEND_COLD void zend_throw_exception_internal(zend_object *exception) /* return; } if (EG(exception)) { - zend_exception_error(EG(exception), E_ERROR); + if (Z_TYPE(EG(user_exception_handler)) != IS_UNDEF) { + zend_user_exception_handler(); + } else { + zend_exception_error(EG(exception), E_ERROR); + } zend_bailout(); } zend_error_noreturn(E_CORE_ERROR, "Exception thrown without a stack frame"); diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c index c1a5e31f7e6..dfb13005f64 100644 --- a/Zend/zend_execute_API.c +++ b/Zend/zend_execute_API.c @@ -1010,7 +1010,11 @@ cleanup_args: if (UNEXPECTED(EG(exception))) { if (UNEXPECTED(!EG(current_execute_data))) { - zend_throw_exception_internal(NULL); + if (Z_TYPE(EG(user_exception_handler)) != IS_UNDEF) { + zend_user_exception_handler(); + } else { + zend_throw_exception_internal(NULL); + } } else if (EG(current_execute_data)->func && ZEND_USER_CODE(EG(current_execute_data)->func->common.type)) { zend_rethrow_exception(EG(current_execute_data)); diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 55ff614a51a..809031ca33a 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -31,6 +31,7 @@ #include "Zend/Optimizer/zend_optimizer.h" #include "test_arginfo.h" #include "zend_call_stack.h" +#include "zend_exceptions.h" ZEND_DECLARE_MODULE_GLOBALS(zend_test) @@ -53,6 +54,8 @@ static zend_class_entry *zend_test_string_enum; static zend_class_entry *zend_test_int_enum; static zend_object_handlers zend_test_class_handlers; +static int le_throwing_resource; + static ZEND_FUNCTION(zend_test_func) { RETVAL_STR_COPY(EX(func)->common.function_name); @@ -910,6 +913,11 @@ static void custom_zend_execute_ex(zend_execute_data *execute_data) old_zend_execute_ex(execute_data); } +static void le_throwing_resource_dtor(zend_resource *rsrc) +{ + zend_throw_exception(NULL, "Throwing resource destructor called", 0); +} + PHP_MINIT_FUNCTION(zend_test) { zend_test_interface = register_class__ZendTestInterface(); @@ -981,6 +989,8 @@ PHP_MINIT_FUNCTION(zend_test) zend_test_observer_init(INIT_FUNC_ARGS_PASSTHRU); zend_test_fiber_init(); + le_throwing_resource = zend_register_list_destructors_ex(le_throwing_resource_dtor, NULL, "throwing resource", module_number); + return SUCCESS; } @@ -1150,3 +1160,11 @@ PHP_ZEND_TEST_API ssize_t copy_file_range(int fd_in, off64_t *off_in, int fd_out return original_copy_file_range(fd_in, off_in, fd_out, off_out, len, flags); } #endif + + +static PHP_FUNCTION(zend_test_create_throwing_resource) +{ + ZEND_PARSE_PARAMETERS_NONE(); + zend_resource *res = zend_register_resource(NULL, le_throwing_resource); + ZVAL_RES(return_value, res); +} diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 30980cf7045..7228021b314 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -196,6 +196,9 @@ namespace { function zend_test_crash(?string $message = null): void {} function zend_test_fill_packed_array(array &$array): void {} + + /** @return resource */ + function zend_test_create_throwing_resource() {} } namespace ZendTestNS { diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index 6a01f6cd016..d3b26b652d0 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: eb6dd9bae381ca8163307e8a0f54bf3983b79cb5 */ + * Stub hash: 58ae12118458386ab204a2400966c25c833baeae */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0) ZEND_END_ARG_INFO() @@ -122,6 +122,9 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_fill_packed_array, 0, ZEND_ARG_TYPE_INFO(1, array, IS_ARRAY, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_zend_test_create_throwing_resource, 0, 0, 0) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_namespaced_func, 0, 0, _IS_BOOL, 0) ZEND_END_ARG_INFO() @@ -227,6 +230,7 @@ static ZEND_FUNCTION(zend_test_is_string_marked_as_valid_utf8); static ZEND_FUNCTION(zend_get_map_ptr_last); static ZEND_FUNCTION(zend_test_crash); static ZEND_FUNCTION(zend_test_fill_packed_array); +static ZEND_FUNCTION(zend_test_create_throwing_resource); static ZEND_FUNCTION(ZendTestNS2_namespaced_func); static ZEND_FUNCTION(ZendTestNS2_namespaced_deprecated_func); static ZEND_FUNCTION(ZendTestNS2_ZendSubNS_namespaced_func); @@ -289,6 +293,7 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(zend_get_map_ptr_last, arginfo_zend_get_map_ptr_last) ZEND_FE(zend_test_crash, arginfo_zend_test_crash) ZEND_FE(zend_test_fill_packed_array, arginfo_zend_test_fill_packed_array) + ZEND_FE(zend_test_create_throwing_resource, arginfo_zend_test_create_throwing_resource) ZEND_NS_FALIAS("ZendTestNS2", namespaced_func, ZendTestNS2_namespaced_func, arginfo_ZendTestNS2_namespaced_func) ZEND_NS_DEP_FALIAS("ZendTestNS2", namespaced_deprecated_func, ZendTestNS2_namespaced_deprecated_func, arginfo_ZendTestNS2_namespaced_deprecated_func) ZEND_NS_FALIAS("ZendTestNS2", namespaced_aliased_func, zend_test_void_return, arginfo_ZendTestNS2_namespaced_aliased_func) diff --git a/ext/zend_test/tests/gh10695_1.phpt b/ext/zend_test/tests/gh10695_1.phpt new file mode 100644 index 00000000000..6ba4a788979 --- /dev/null +++ b/ext/zend_test/tests/gh10695_1.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-10695: Exceptions in resource dtors during shutdown are caught +--EXTENSIONS-- +zend_test +--FILE-- +getMessage() . "\n"; +}); + +$resource = zend_test_create_throwing_resource(); +?> +--EXPECT-- +Caught: Throwing resource destructor called diff --git a/ext/zend_test/tests/gh10695_2.phpt b/ext/zend_test/tests/gh10695_2.phpt new file mode 100644 index 00000000000..d60dbf8e7da --- /dev/null +++ b/ext/zend_test/tests/gh10695_2.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-10695: Uncaught exception in exception handler catching resource dtor exception +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECTF-- +Fatal error: Uncaught Exception: Caught in %s:%d +Stack trace: +#0 [internal function]: {closure}(Object(Exception)) +#1 {main} + thrown in %s on line %d