diff --git a/NEWS b/NEWS index 140975629a81..0ea5e959f218 100644 --- a/NEWS +++ b/NEWS @@ -37,6 +37,10 @@ PHP NEWS . Fixed infinite loop in sapi_header_op() for headers of 4 GiB or more. (Martin Mirchev) +- Session: + . Fixed session save handler wrongly reported as recursive after a fatal + error inside a handler callback. (Ilia Alshanetsky) + - SPL: . Fixed bug GH-24264 (Unwind/GracefulExit discarded in spl_iterators.c). (Lazizbek Ergashev) diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c index 174a0e4822e2..dc77b7c5f473 100644 --- a/ext/session/mod_user.c +++ b/ext/session/mod_user.c @@ -25,23 +25,32 @@ const ps_module ps_mod_user = { static void ps_call_handler(zval *func, int argc, zval *argv, zval *retval) { int i; + bool bailout = false; if (PS(in_save_handler)) { PS(in_save_handler) = false; ZVAL_UNDEF(retval); php_error_docref(NULL, E_WARNING, "Cannot call session save handler in a recursive manner"); } else { PS(in_save_handler) = true; - if (call_user_function(NULL, NULL, func, retval, argc, argv) == FAILURE) { - zval_ptr_dtor(retval); - ZVAL_UNDEF(retval); - } else if (Z_ISUNDEF_P(retval)) { - ZVAL_NULL(retval); - } + zend_try { + if (call_user_function(NULL, NULL, func, retval, argc, argv) == FAILURE) { + zval_ptr_dtor(retval); + ZVAL_UNDEF(retval); + } else if (Z_ISUNDEF_P(retval)) { + ZVAL_NULL(retval); + } + } zend_catch { + bailout = true; + PS(handler_bailout) = true; + } zend_end_try(); PS(in_save_handler) = false; } for (i = 0; i < argc; i++) { zval_ptr_dtor(&argv[i]); } + if (bailout) { + zend_bailout(); + } } #define PSF(a) PS(mod_user_names).ps_##a @@ -91,7 +100,9 @@ PS_OPEN_FUNC(user) zend_try { ps_call_handler(&PSF(open), 2, args, &retval); } zend_catch { - PS(session_status) = php_session_none; + if (!PS(mod_user_implemented)) { + PS(session_status) = php_session_none; + } if (!Z_ISUNDEF(retval)) { zval_ptr_dtor(&retval); } diff --git a/ext/session/php_session.h b/ext/session/php_session.h index e2b7a7e5a7d3..06ebc4652b90 100644 --- a/ext/session/php_session.h +++ b/ext/session/php_session.h @@ -184,6 +184,7 @@ typedef struct _php_ps_globals { bool mod_user_uses_object_methods_as_handlers; bool use_trans_sid; /* contains the INI value of whether to use trans-sid */ bool random_seeded; + bool handler_bailout; bool in_rfc1867_update; /* php_session_rfc1867_update() is running */ } php_ps_globals; diff --git a/ext/session/session.c b/ext/session/session.c index cca448b5a415..9c268175326c 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -114,6 +114,7 @@ static inline void php_rinit_session_globals(void) PS(session_status) = php_session_none; PS(in_save_handler) = false; PS(set_handler) = false; + PS(handler_bailout) = false; PS(mod_data) = NULL; PS(mod_user_is_open) = false; PS(mod_user_uses_object_methods_as_handlers) = false; @@ -421,6 +422,7 @@ static zend_result php_session_initialize(void) zend_string *val = NULL; PS(session_status) = php_session_active; + PS(handler_bailout) = false; if (!PS(mod)) { PS(session_status) = php_session_disabled; @@ -1804,8 +1806,18 @@ PHPAPI zend_result php_session_start(void) PHPAPI bool php_session_flush(bool write) { if (PS(session_status) == php_session_active) { - php_session_save_current_state(write); + bool bailout = false; + zend_try { + php_session_save_current_state(write && !PS(handler_bailout)); + } zend_catch { + bailout = true; + } zend_end_try(); + if (bailout) { + PS(handler_bailout) = true; + zend_bailout(); + } PS(session_status) = php_session_none; + PS(handler_bailout) = false; return true; } return false; @@ -2395,6 +2407,11 @@ PHP_FUNCTION(session_regenerate_id) RETURN_FALSE; } + if (PS(handler_bailout)) { + php_error_docref(NULL, E_WARNING, "Session ID cannot be regenerated after a fatal error in session handling"); + RETURN_FALSE; + } + /* Process old session data */ if (del_ses) { if (PS(mod)->s_destroy(&PS(mod_data), PS(id)) == FAILURE) { @@ -2950,6 +2967,7 @@ static PHP_GINIT_FUNCTION(ps) ps_globals->mod_user_uses_object_methods_as_handlers = false; ps_globals->session_vars = NULL; ps_globals->set_handler = false; + ps_globals->handler_bailout = false; ps_globals->session_started_filename = NULL; ps_globals->session_started_lineno = 0; /* Unset user defined handlers */ diff --git a/ext/session/tests/bailout_in_encode_closes_handler.phpt b/ext/session/tests/bailout_in_encode_closes_handler.phpt new file mode 100644 index 000000000000..fd24e6ce9123 --- /dev/null +++ b/ext/session/tests/bailout_in_encode_closes_handler.phpt @@ -0,0 +1,48 @@ +--TEST-- +GH-15529: Bailout while encoding session data still closes a SessionHandler-based handler +--EXTENSIONS-- +session +zend_test +--INI-- +session.save_handler=files +session.serialize_handler=php +--FILE-- + +--CLEAN-- + +--EXPECTF-- +Fatal error: Bailout in %s on line %d +close diff --git a/ext/session/tests/bailout_in_read_keeps_data.phpt b/ext/session/tests/bailout_in_read_keeps_data.phpt new file mode 100644 index 000000000000..ee84bd2ee798 --- /dev/null +++ b/ext/session/tests/bailout_in_read_keeps_data.phpt @@ -0,0 +1,57 @@ +--TEST-- +GH-15529: Bailout in read() does not overwrite the stored session data +--EXTENSIONS-- +session +zend_test +--INI-- +session.save_handler=files +session.serialize_handler=php +session.use_cookies=0 +session.use_strict_mode=0 +session.cache_limiter= +--FILE-- + +--CLEAN-- + +--EXPECTF-- +Fatal error: Bailout in %s on line %d + +Warning: session_regenerate_id(): Session ID cannot be regenerated after a fatal error in session handling in %s on line %d +bool(false) +string(6) "x|i:1;" +close diff --git a/ext/session/tests/bailout_in_write_no_sticky_flag.phpt b/ext/session/tests/bailout_in_write_no_sticky_flag.phpt new file mode 100644 index 000000000000..a7d8f213f963 --- /dev/null +++ b/ext/session/tests/bailout_in_write_no_sticky_flag.phpt @@ -0,0 +1,32 @@ +--TEST-- +GH-15529: Bailout in userland save handler write() is not retried, close() still runs +--EXTENSIONS-- +session +zend_test +--FILE-- + +--EXPECTF-- +write + +Fatal error: Bailout in %s on line %d +close diff --git a/ext/session/tests/user_session_module/bug60634_error_3.phpt b/ext/session/tests/user_session_module/bug60634_error_3.phpt index 0118ede70ab6..d7a6ca704e21 100644 --- a/ext/session/tests/user_session_module/bug60634_error_3.phpt +++ b/ext/session/tests/user_session_module/bug60634_error_3.phpt @@ -50,5 +50,4 @@ Stack trace: #0 [internal function]: write(%s, '') #1 {main} thrown in %s on line %d - -Warning: PHP Request Shutdown: Cannot call session save handler in a recursive manner in Unknown on line 0 +close: goodbye cruel world diff --git a/ext/session/tests/user_session_module/bug60634_error_4.phpt b/ext/session/tests/user_session_module/bug60634_error_4.phpt index 3a4248eae3fb..99c322231884 100644 --- a/ext/session/tests/user_session_module/bug60634_error_4.phpt +++ b/ext/session/tests/user_session_module/bug60634_error_4.phpt @@ -45,5 +45,4 @@ Stack trace: #0 [internal function]: write('%s', '') #1 {main} thrown in %s on line %d - -Warning: PHP Request Shutdown: Cannot call session save handler in a recursive manner in Unknown on line 0 +close: goodbye cruel world