Actually fix GH-9583

The issue is that PS(mod)->s_validate_sid is always defined for user modules, thus we need to check that the actual callable is set
Add another regression test to ensure current working behaviour is not broken (which was by the previous incorrect fix)

Closes GH-9638

Author: Girgias

ext/session/session.c

@@ -1083,9 +1083,10 @@ PHPAPI int php_session_register_module(const ps_module *ptr) /* {{{ */
 /* }}} */
 
 /* Dummy PS module function */
-/* We consider any ID valid, so we return FAILURE to indicate that a session doesn't exist */
+/* We consider any ID valid (thus also implying that a session with such an ID exists),
+	thus we always return SUCCESS */
 PHPAPI int php_session_validate_sid(PS_VALIDATE_SID_ARGS) {
-	return FAILURE;
+	return SUCCESS;
 }
 
 /* Dummy PS module function */
@@ -2255,18 +2256,24 @@ PHP_FUNCTION(session_regenerate_id)
 		}
 		RETURN_THROWS();
 	}
-	if (PS(use_strict_mode) && PS(mod)->s_validate_sid &&
-		PS(mod)->s_validate_sid(&PS(mod_data), PS(id)) == SUCCESS) {
-		zend_string_release_ex(PS(id), 0);
-		PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
-		if (!PS(id)) {
-			PS(mod)->s_close(&PS(mod_data));
-			PS(session_status) = php_session_none;
-			if (!EG(exception)) {
-				zend_throw_error(NULL, "Failed to create session ID by collision: %s (path: %s)", PS(mod)->s_name, PS(save_path));
+	if (PS(use_strict_mode)) {
+		if ((!PS(mod_user_implemented) && PS(mod)->s_validate_sid) || !Z_ISUNDEF(PS(mod_user_names).name.ps_validate_sid)) {
+			int limit = 3;
+			/* Try to generate non-existing ID */
+			while (limit-- && PS(mod)->s_validate_sid(&PS(mod_data), PS(id)) == SUCCESS) {
+				zend_string_release_ex(PS(id), 0);
+				PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
+				if (!PS(id)) {
+					PS(mod)->s_close(&PS(mod_data));
+					PS(session_status) = php_session_none;
+					if (!EG(exception)) {
+						zend_throw_error(NULL, "Failed to create session ID by collision: %s (path: %s)", PS(mod)->s_name, PS(save_path));
+					}
+					RETURN_THROWS();
+				}
 			}
-			RETURN_THROWS();
 		}
+		// TODO warn that ID cannot be verified? else { }
 	}
 	/* Read is required to make new session data at this point. */
 	if (PS(mod)->s_read(&PS(mod_data), PS(id), &data, PS(gc_maxlifetime)) == FAILURE) {
@@ -2293,7 +2300,6 @@ PHP_FUNCTION(session_regenerate_id)
 /* }}} */
 
 /* {{{ Generate new session ID. Intended for user save handlers. */
-/* This is not used yet */
 PHP_FUNCTION(session_create_id)
 {
 	zend_string *prefix = NULL, *new_id;
@@ -2317,7 +2323,7 @@ PHP_FUNCTION(session_create_id)
 		int limit = 3;
 		while (limit--) {
 			new_id = PS(mod)->s_create_sid(&PS(mod_data));
-			if (!PS(mod)->s_validate_sid) {
+			if (!PS(mod)->s_validate_sid || (PS(mod_user_implemented) && Z_ISUNDEF(PS(mod_user_names).name.ps_validate_sid))) {
 				break;
 			} else {
 				/* Detect collision and retry */

ext/session/tests/gh9583-extra.phpt

@@ -0,0 +1,49 @@
+--TEST--
+GH-9583: session_create_id() fails with user defined save handler that doesn't have a validateId() method
+--EXTENSIONS--
+session
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+class SessionHandlerTester implements \SessionHandlerInterface
+{
+
+    public function close(): bool { return true; }
+
+    public function destroy($id): bool { return true; }
+
+    public function gc($max_lifetime): int|false { return 1; }
+
+    public function open($path, $name): bool { return true; }
+
+    public function read($id): string { return ''; }
+
+    public function write($id, $data): bool { return true; }
+
+    //public function create_sid() { return uniqid(); }
+
+    //public function validateId($key) { return true; }
+}
+
+$obj = new SessionHandlerTester();
+ini_set('session.use_strict_mode','1');
+session_set_save_handler($obj);
+session_start();
+
+$originalSessionId = session_id();
+
+session_write_close();
+
+session_start();
+
+$newSessionId = session_id();
+
+echo 'validateId() ', (method_exists($obj, 'validateId') ? ('returns ' . ($obj->validateId(1) ? 'true' : 'false')) : 'is commented out'), "\n";
+var_dump($originalSessionId == $newSessionId);
+
+?>
+--EXPECT--
+validateId() is commented out
+bool(true)