From cd925468f0027bdaae31dd11b3701b23400eb1b2 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 24 Aug 2026 16:14:44 -0400 Subject: [PATCH] ext/session: Close session without retrying write() after handler bailout A zend_bailout() from inside a userland save handler, such as a fatal error in write(), left PS(in_save_handler) set, so the next handler call was refused with "Cannot call session save handler in a recursive manner": request shutdown retried the failed write(), or, when the bailout came from the shutdown flush itself, close() never ran. Clear the flag once the handler returns or bails out, and record the bailout, as well as one from php_session_flush() itself. While it is recorded, a flush only closes the handler, so neither a failed encode or write() nor the empty data left by a bailed-out read() is written, and session_regenerate_id() refuses. A bailout in open() keeps an already open session active so it can still be closed. Initializing the session clears the record. --- NEWS | 4 ++ ext/session/mod_user.c | 25 +++++--- ext/session/php_session.h | 1 + ext/session/session.c | 20 ++++++- .../bailout_in_encode_closes_handler.phpt | 48 ++++++++++++++++ .../tests/bailout_in_read_keeps_data.phpt | 57 +++++++++++++++++++ .../bailout_in_write_no_sticky_flag.phpt | 32 +++++++++++ .../user_session_module/bug60634_error_3.phpt | 3 +- .../user_session_module/bug60634_error_4.phpt | 3 +- 9 files changed, 181 insertions(+), 12 deletions(-) create mode 100644 ext/session/tests/bailout_in_encode_closes_handler.phpt create mode 100644 ext/session/tests/bailout_in_read_keeps_data.phpt create mode 100644 ext/session/tests/bailout_in_write_no_sticky_flag.phpt 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