ext/session: Preserve exceptions thrown by create_sid()

NEWS

@@ -68,6 +68,10 @@ PHP                                                                        NEWS
   . Fixed a heap over-read in the interactive shell prompt when cli.prompt is
     set to an empty string. (Ilia Alshanetsky)
 
+- Session:
+  . Fixed exceptions from user-defined create_sid() handlers being replaced
+    by return-value validation errors. (Ilia Alshanetsky)
+
 - Sockets:
   . Fixed socket_select() silently truncating sets larger than FD_SETSIZE on
     Windows. (David Carlier)

ext/session/mod_user.c

@@ -237,12 +237,16 @@ PS_CREATE_SID_FUNC(user)
 			}
 			zval_ptr_dtor(&retval);
 		} else {
-			zend_throw_error(NULL, "No session id returned by function");
+			if (!EG(exception)) {
+				zend_throw_error(NULL, "No session id returned by function");
+			}
 			return NULL;
 		}
 
 		if (!id) {
-			zend_throw_error(NULL, "Session id must be a string");
+			if (!EG(exception)) {
+				zend_throw_error(NULL, "Session id must be a string");
+			}
 			return NULL;
 		}
 

ext/session/tests/user_session_module/session_create_id_create_sid_throws.phpt

@@ -36,14 +36,13 @@ try {
     session_create_id();
 } catch (Throwable $e) {
     echo $e::class, ": ", $e->getMessage(), PHP_EOL;
-    $previous = $e->getPrevious();
-    echo $previous::class, ": ", $previous->getMessage(), PHP_EOL;
+    var_dump($e->getPrevious());
 }
 
 var_dump(session_status() === PHP_SESSION_ACTIVE);
 
 ?>
 --EXPECT--
-Error: Session id must be a string
 Exception: create_sid failed
+NULL
 bool(true)

ext/session/tests/user_session_module/session_create_sid_return_destructor_throws.phpt

@@ -0,0 +1,37 @@
+--TEST--
+Exceptions from destruction of an invalid create_sid() return value are preserved
+--EXTENSIONS--
+session
+--FILE--
+<?php
+
+class FailingHandler implements SessionHandlerInterface, SessionIdInterface
+{
+    public function open($path, $name): bool { return true; }
+    public function close(): bool { return true; }
+    public function read($id): string|false { return ''; }
+    public function write($id, $data): bool { return true; }
+    public function destroy($id): bool { return true; }
+    public function gc($max_lifetime): int|false { return 0; }
+
+    #[ReturnTypeWillChange]
+    public function create_sid()
+    {
+        return new class {
+            public function __destruct()
+            {
+                throw new RuntimeException('destructor failed');
+            }
+        };
+    }
+}
+
+session_set_save_handler(new FailingHandler());
+try {
+    session_start();
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+?>
+--EXPECT--
+RuntimeException: destructor failed

ext/session/tests/user_session_module/session_regenerate_id_create_sid_throws.phpt

@@ -0,0 +1,57 @@
+--TEST--
+session_regenerate_id() preserves exceptions from create_sid(), including collision retries
+--EXTENSIONS--
+session
+--INI--
+session.use_cookies=0
+session.cache_limiter=
+session.use_strict_mode=1
+session.gc_probability=0
+--FILE--
+<?php
+
+ob_start();
+
+class FailingHandler implements SessionHandlerInterface, SessionIdInterface, SessionUpdateTimestampHandlerInterface
+{
+    public int $calls = 0;
+    public int $throwAt;
+
+    public function open($path, $name): bool { return true; }
+    public function close(): bool { return true; }
+    public function read($id): string|false { return ''; }
+    public function write($id, $data): bool { return true; }
+    public function destroy($id): bool { return true; }
+    public function gc($max_lifetime): int|false { return 0; }
+    public function updateTimestamp($id, $data): bool { return true; }
+    public function validateId($id): bool { return true; }
+
+    public function create_sid(): string
+    {
+        if (++$this->calls === $this->throwAt) {
+            throw new RuntimeException('create_sid failed');
+        }
+        return 'session' . $this->calls;
+    }
+}
+
+foreach ([2, 3] as $throwAt) {
+    $handler = new FailingHandler();
+    $handler->throwAt = $throwAt;
+    session_set_save_handler($handler);
+    session_id('');
+    session_start();
+
+    try {
+        session_regenerate_id();
+    } catch (Throwable $e) {
+        echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+    }
+    echo 'create_sid calls: ', $handler->calls, PHP_EOL;
+}
+?>
+--EXPECT--
+RuntimeException: create_sid failed
+create_sid calls: 2
+RuntimeException: create_sid failed
+create_sid calls: 3

ext/session/tests/user_session_module/session_start_create_sid_throws.phpt

@@ -0,0 +1,64 @@
+--TEST--
+session_start() preserves exceptions from create_sid()
+--EXTENSIONS--
+session
+--INI--
+session.use_cookies=0
+session.cache_limiter=
+session.gc_probability=0
+--FILE--
+<?php
+
+ob_start();
+
+class FailingHandler implements SessionHandlerInterface, SessionIdInterface, SessionUpdateTimestampHandlerInterface
+{
+    public bool $invalidReturn = false;
+
+    public function open($path, $name): bool { return true; }
+    public function close(): bool { return true; }
+    public function read($id): string|false { return ''; }
+    public function write($id, $data): bool { return true; }
+    public function destroy($id): bool { return true; }
+    public function gc($max_lifetime): int|false { return 0; }
+    public function updateTimestamp($id, $data): bool { return true; }
+
+    public function create_sid(): string
+    {
+        if ($this->invalidReturn) {
+            return [];
+        }
+        throw new RuntimeException('create_sid failed');
+    }
+
+    public function validateId(string $id): bool
+    {
+        return false;
+    }
+}
+
+$handler = new FailingHandler();
+session_set_save_handler($handler);
+
+foreach ([false, true] as $strict) {
+    foreach ([false, true] as $invalidReturn) {
+        echo 'strict mode: ', (int) $strict, ', invalid return: ', (int) $invalidReturn, PHP_EOL;
+        $handler->invalidReturn = $invalidReturn;
+        session_id($strict ? 'rejected' : '');
+        try {
+            session_start(['use_strict_mode' => $strict]);
+        } catch (Throwable $e) {
+            echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+        }
+    }
+}
+?>
+--EXPECT--
+strict mode: 0, invalid return: 0
+RuntimeException: create_sid failed
+strict mode: 0, invalid return: 1
+TypeError: FailingHandler::create_sid(): Return value must be of type string, array returned
+strict mode: 1, invalid return: 0
+RuntimeException: create_sid failed
+strict mode: 1, invalid return: 1
+TypeError: FailingHandler::create_sid(): Return value must be of type string, array returned