From 8dc2a69c399d3497a20e7369856d27e477d91d1e Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Thu, 10 Mar 2022 13:16:48 +0100 Subject: [PATCH] Improve sesson write failure message for user error handlers Fixes GH-7787 --- NEWS | 4 ++ ext/session/php_session.h | 1 + ext/session/session.c | 25 +++++++++- ext/session/tests/gh7787.phpt | 90 +++++++++++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 ext/session/tests/gh7787.phpt diff --git a/NEWS b/NEWS index 0bb7114961c14..dd939687ecaf6 100644 --- a/NEWS +++ b/NEWS @@ -35,4 +35,8 @@ PHP NEWS . add ZipArchive::getStreamName() method . add ZipArchive::getStreamIndex() method +- Session: + . Fixed bug GH-7787 (Improve session write failure message for user error + handlers). (ilutov) + <<< NOTE: Insert NEWS from last stable release here prior to actual release! >>> diff --git a/ext/session/php_session.h b/ext/session/php_session.h index bc8c3548c7059..ef139196ca900 100644 --- a/ext/session/php_session.h +++ b/ext/session/php_session.h @@ -176,6 +176,7 @@ typedef struct _php_ps_globals { } mod_user_names; int mod_user_implemented; int mod_user_is_open; + zend_string *mod_user_class_name; const struct ps_serializer_struct *serializer; zval http_session_vars; bool auto_start; diff --git a/ext/session/session.c b/ext/session/session.c index 32e2f33ae3028..e38e4f68cb742 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -472,6 +472,9 @@ static void php_session_save_current_state(int write) /* {{{ */ if (write) { IF_SESSION_VARS() { + zend_string *handler_class_name = PS(mod_user_class_name); + const char *handler_function_name; + if (PS(mod_data) || PS(mod_user_implemented)) { zend_string *val; @@ -483,12 +486,15 @@ static void php_session_save_current_state(int write) /* {{{ */ && zend_string_equals(val, PS(session_vars)) ) { ret = PS(mod)->s_update_timestamp(&PS(mod_data), PS(id), val, PS(gc_maxlifetime)); + handler_function_name = handler_class_name != NULL ? "updateTimestamp" : "update_timestamp"; } else { ret = PS(mod)->s_write(&PS(mod_data), PS(id), val, PS(gc_maxlifetime)); + handler_function_name = "write"; } zend_string_release_ex(val, 0); } else { ret = PS(mod)->s_write(&PS(mod_data), PS(id), ZSTR_EMPTY_ALLOC(), PS(gc_maxlifetime)); + handler_function_name = "write"; } } @@ -499,9 +505,14 @@ static void php_session_save_current_state(int write) /* {{{ */ "is correct (%s)", PS(mod)->s_name, PS(save_path)); + } else if (handler_class_name != NULL) { + php_error_docref(NULL, E_WARNING, "Failed to write session data using user " + "defined save handler. (session.save_path: %s, handler: %s::%s)", PS(save_path), + ZSTR_VAL(handler_class_name), handler_function_name); } else { php_error_docref(NULL, E_WARNING, "Failed to write session data using user " - "defined save handler. (session.save_path: %s)", PS(save_path)); + "defined save handler. (session.save_path: %s, handler: %s)", PS(save_path), + handler_function_name); } } } @@ -2042,6 +2053,12 @@ PHP_FUNCTION(session_set_save_handler) ++i; } ZEND_HASH_FOREACH_END(); + + if (PS(mod_user_class_name)) { + zend_string_release(PS(mod_user_class_name)); + } + PS(mod_user_class_name) = zend_string_copy(Z_OBJCE_P(obj)->name); + if (register_shutdown) { /* create shutdown function */ php_shutdown_function_entry shutdown_function_entry; @@ -2095,6 +2112,11 @@ PHP_FUNCTION(session_set_save_handler) RETURN_FALSE; } + if (PS(mod_user_class_name)) { + zend_string_release(PS(mod_user_class_name)); + } + PS(mod_user_class_name) = NULL; + /* remove shutdown function */ remove_user_shutdown_function("session_shutdown", sizeof("session_shutdown") - 1); @@ -2775,6 +2797,7 @@ static PHP_GINIT_FUNCTION(ps) /* {{{ */ ps_globals->session_status = php_session_none; ps_globals->default_mod = NULL; ps_globals->mod_user_implemented = 0; + ps_globals->mod_user_class_name = NULL; ps_globals->mod_user_is_open = 0; ps_globals->session_vars = NULL; ps_globals->set_handler = 0; diff --git a/ext/session/tests/gh7787.phpt b/ext/session/tests/gh7787.phpt new file mode 100644 index 0000000000000..b38398827b087 --- /dev/null +++ b/ext/session/tests/gh7787.phpt @@ -0,0 +1,90 @@ +--TEST-- +GH-7787: Provide more SessionHandler failure information +--EXTENSIONS-- +session +--SKIPIF-- + +--INI-- +session.use_strict_mode=0 +session.save_handler=files +--FILE-- + true, + fn() => true, + fn() => 'foo|s:3:"foo";', + fn() => false, + fn() => true, + fn() => true, + fn() => sha1(random_bytes(32)), + fn() => true, + fn() => false, +); + +session_start(); +$_SESSION['foo'] = 'bar'; +session_write_close(); + +session_start(); +session_write_close(); + +?> +--EXPECTF-- +Warning: session_write_close(): Failed to write session data using user defined save handler. (session.save_path: , handler: MySessionHandler::write) in %s on line %d + +Warning: session_write_close(): Failed to write session data using user defined save handler. (session.save_path: , handler: MySessionHandler::updateTimestamp) in %s on line %d + +Warning: session_write_close(): Failed to write session data using user defined save handler. (session.save_path: , handler: write) in %s on line %d + +Warning: session_write_close(): Failed to write session data using user defined save handler. (session.save_path: , handler: update_timestamp) in %s on line %d