diff --git a/ext/session/mod_files.c b/ext/session/mod_files.c index 3a7f8aec4522..c3c616c6479d 100644 --- a/ext/session/mod_files.c +++ b/ext/session/mod_files.c @@ -101,13 +101,13 @@ const ps_module ps_mod_files = { PS_MOD_UPDATE_TIMESTAMP(files) }; -static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, const zend_string *key) +ZEND_ATTRIBUTE_NONNULL static char *ps_files_path_create(char *buf, size_t buflen, ps_files *data, const zend_string *key) { const char *p; int i; size_t n; - if (!data || ZSTR_LEN(key) <= data->dirdepth || + if (ZSTR_LEN(key) <= data->dirdepth || buflen < (ZSTR_LEN(data->basedir) + 2 * data->dirdepth + ZSTR_LEN(key) + 5 + sizeof(FILE_PREFIX))) { return NULL; } @@ -351,12 +351,12 @@ static int ps_files_cleanup_dir(const zend_string *dirname, zend_long maxlifetim return nrdels; } -static zend_result ps_files_key_exists(ps_files *data, const zend_string *key) +ZEND_ATTRIBUTE_NONNULL static zend_result ps_files_key_exists(ps_files *data, const zend_string *key) { char buf[MAXPATHLEN]; zend_stat_t sbuf = {0}; - if (!key || !ps_files_path_create(buf, sizeof(buf), data, key)) { + if (!ps_files_path_create(buf, sizeof(buf), data, key)) { return FAILURE; } if (VCWD_STAT(buf, &sbuf)) { @@ -678,8 +678,7 @@ PS_CREATE_SID_FUNC(files) } } /* Check collision */ - /* FIXME: mod_data(data) should not be NULL (User handler could be NULL) */ - if (data && ps_files_key_exists(data, sid) == SUCCESS) { + if (ps_files_key_exists(data, sid) == SUCCESS) { zend_string_release_ex(sid, false); sid = NULL; if (--maxfail < 0) { diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c index 71b8abdea8b0..5c6fe557771f 100644 --- a/ext/session/mod_user.c +++ b/ext/session/mod_user.c @@ -16,6 +16,8 @@ #include "php_session.h" #include "mod_user.h" +#include "zend_exceptions.h" + const ps_module ps_mod_user = { PS_MOD_UPDATE_TIMESTAMP(user) }; @@ -222,27 +224,23 @@ PS_CREATE_SID_FUNC(user) { /* maintain backwards compatibility */ if (!Z_ISUNDEF(PSF(create_sid))) { - zend_string *id = NULL; zval retval; ps_call_handler(&PSF(create_sid), 0, NULL, &retval); - - if (!Z_ISUNDEF(retval)) { - if (Z_TYPE(retval) == IS_STRING) { - id = zend_string_copy(Z_STR(retval)); - } - zval_ptr_dtor(&retval); - } else { - zend_throw_error(NULL, "No session id returned by function"); + /* Exception was thrown */ + if (Z_ISUNDEF(retval)) { return NULL; } - if (!id) { - zend_throw_error(NULL, "Session id must be a string"); + if (UNEXPECTED(Z_TYPE(retval) != IS_STRING)) { + /* Will no longer be needed in PHP 9 as the interface return type will be in effect */ + zend_throw_error(zend_ce_type_error, "Session id must be of type string, %s given", zend_zval_type_name(&retval)); + zval_ptr_dtor(&retval); return NULL; } + ZEND_ASSERT(Z_TYPE(retval) == IS_STRING); - return id; + return Z_STR(retval); } /* function as defined by PS_MOD */ diff --git a/ext/session/mod_user_class.c b/ext/session/mod_user_class.c index a6bd69c91a07..9626dd27dfb1 100644 --- a/ext/session/mod_user_class.c +++ b/ext/session/mod_user_class.c @@ -51,6 +51,7 @@ PHP_METHOD(SessionHandler, open) } zend_end_try(); if (SUCCESS == ret) { + ZEND_ASSERT(PS(mod_data) && "opened default session must have mod_data"); PS(mod_user_is_open) = true; } @@ -142,13 +143,18 @@ PHP_METHOD(SessionHandler, gc) PHP_METHOD(SessionHandler, create_sid) { - zend_string *id; - ZEND_PARSE_PARAMETERS_NONE(); PS_SANITY_CHECK; + if (!PS(mod_user_is_open)) { + php_error_docref(NULL, E_WARNING, "Parent session handler is not open, defaulting to session_create_id()"); + RETURN_STR(php_session_create_id(NULL)); + } - id = PS(default_mod)->s_create_sid(&PS(mod_data)); + zend_string *id = PS(default_mod)->s_create_sid(&PS(mod_data)); + if (UNEXPECTED(id == NULL)) { + zend_throw_error(NULL, "Failed to create session ID: %s (path: %s)", PS(mod)->s_name, ZSTR_VAL(PS(save_path))); + } RETURN_STR(id); } diff --git a/ext/session/session.c b/ext/session/session.c index 452a3446fc14..b1f1d2a36304 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -429,9 +429,9 @@ static zend_result php_session_initialize(void) } /* Open session handler first */ - if (PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)) == FAILURE - /* || PS(mod_data) == NULL */ /* FIXME: open must set valid PS(mod_data) with success */ - ) { + const zend_result open_status = PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)); + /* NOTE: PS(mod_data) might be null if the session is a custom userland session handler */ + if (open_status == FAILURE) { php_session_abort(); if (!EG(exception)) { php_error_docref(NULL, E_WARNING, "Failed to initialize storage module: %s (path: %s)", PS(mod)->s_name, ZSTR_VAL(PS(save_path))); @@ -2399,7 +2399,10 @@ PHP_FUNCTION(session_regenerate_id) zend_string_release_ex(PS(id), false); PS(id) = NULL; - if (PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)) == FAILURE) { + /* Open session handler first */ + const zend_result open_status = PS(mod)->s_open(&PS(mod_data), PS(save_path), PS(session_name)); + /* NOTE: PS(mod_data) might be null if the session is a custom userland session handler */ + if (open_status == FAILURE) { PS(session_status) = php_session_none; if (!EG(exception)) { zend_throw_error(NULL, "Failed to open session: %s (path: %s)", PS(mod)->s_name, ZSTR_VAL(PS(save_path))); @@ -2480,6 +2483,7 @@ PHP_FUNCTION(session_create_id) } } + /* NOTE: PS(mod_data) might be null if the session is a custom userland session handler */ if (!PS(in_save_handler) && PS(session_status) == php_session_active) { int limit = 3; while (limit--) { diff --git a/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt b/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt index b65c0671d940..c87df2ab458e 100644 --- a/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt +++ b/ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt @@ -44,6 +44,6 @@ var_dump(session_status() === PHP_SESSION_ACTIVE); ?> --EXPECT-- -Error: Session id must be a string +TypeError: Session id must be of type string, null given Exception: create_sid failed bool(true) diff --git a/ext/session/tests/user_session_module/session_set_save_handler_class_005.phpt b/ext/session/tests/user_session_module/session_set_save_handler_class_005.phpt index c989cf39d737..cf9460c53df8 100644 --- a/ext/session/tests/user_session_module/session_set_save_handler_class_005.phpt +++ b/ext/session/tests/user_session_module/session_set_save_handler_class_005.phpt @@ -37,11 +37,13 @@ session_unset(); --EXPECTF-- *** Testing session_set_save_handler() : incomplete implementation *** -Warning: SessionHandler::read(): Parent session handler is not open in %ssession_set_save_handler_class_005.php on line %d +Warning: SessionHandler::create_sid(): Parent session handler is not open, defaulting to session_create_id() in %s on line %d -Warning: SessionHandler::close(): Parent session handler is not open in %ssession_set_save_handler_class_005.php on line %d +Warning: SessionHandler::read(): Parent session handler is not open in %s on line %d -Warning: session_start(): Failed to read session data: user (%s) in %ssession_set_save_handler_class_005.php on line %d +Warning: SessionHandler::close(): Parent session handler is not open in %s on line %d + +Warning: session_start(): Failed to read session data: user (%s) in %s on line %d bool(false) string(0) "" string(4) "user" diff --git a/ext/session/tests/user_session_module/session_set_save_handler_class_016.phpt b/ext/session/tests/user_session_module/session_set_save_handler_class_016.phpt index 242035380ea0..61eecc7141d6 100644 --- a/ext/session/tests/user_session_module/session_set_save_handler_class_016.phpt +++ b/ext/session/tests/user_session_module/session_set_save_handler_class_016.phpt @@ -72,6 +72,8 @@ session_unset(); ?> --EXPECTF-- *** Testing session_set_save_handler() function: class with create_sid *** + +Warning: SessionHandler::create_sid(): Parent session handler is not open, defaulting to session_create_id() in %s on line %d string(%d) "%s" string(4) "user" array(1) { diff --git a/ext/session/tests/user_session_module/session_set_save_handler_sid_002.phpt b/ext/session/tests/user_session_module/session_set_save_handler_sid_002.phpt index 3b9a3f411efa..c9a10de4e442 100644 --- a/ext/session/tests/user_session_module/session_set_save_handler_sid_002.phpt +++ b/ext/session/tests/user_session_module/session_set_save_handler_sid_002.phpt @@ -1,5 +1,5 @@ --TEST-- -Test session_set_save_handler() function: create_sid +session_set_save_handler() with create_sid handler not returning string --INI-- session.save_path="{TMP}" --EXTENSIONS-- @@ -7,10 +7,6 @@ session --FILE-- path . $id); + return file_get_contents($this->path . $id); } public function write($id, $data): bool { @@ -55,26 +51,13 @@ class MySession2 implements SessionHandlerInterface, SessionIdInterface { } session_set_save_handler(new MySession2()); -session_start(); - -$_SESSION['foo'] = "hello"; - -var_dump(session_id(), ini_get('session.save_handler'), $_SESSION); -session_write_close(); -session_unset(); - -session_start(); -var_dump($_SESSION); +try { + session_start(); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), "\n"; +} -session_write_close(); -session_unset(); ?> ---EXPECTF-- -*** Testing session_set_save_handler() function: create_sid *** - -Fatal error: Uncaught Error: Session id must be a string in %s:%d -Stack trace: -#0 %s(%d): session_start() -#1 {main} - thrown in %s on line %d +--EXPECT-- +TypeError: Session id must be of type string, bool given