Skip to content

Commit 39e7219

Browse files
committed
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)
1 parent f3dba7e commit 39e7219

File tree

2 files changed

+53
-3
lines changed

2 files changed

+53
-3
lines changed

ext/session/session.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,9 +1083,10 @@ PHPAPI int php_session_register_module(const ps_module *ptr) /* {{{ */
10831083
/* }}} */
10841084

10851085
/* Dummy PS module function */
1086-
/* We consider any ID valid, so we return FAILURE to indicate that a session doesn't exist */
1086+
/* We consider any ID valid (thus also implying that a session with such an ID exists),
1087+
thus we always return SUCCESS */
10871088
PHPAPI int php_session_validate_sid(PS_VALIDATE_SID_ARGS) {
1088-
return FAILURE;
1089+
return SUCCESS;
10891090
}
10901091

10911092
/* Dummy PS module function */
@@ -2317,7 +2318,7 @@ PHP_FUNCTION(session_create_id)
23172318
int limit = 3;
23182319
while (limit--) {
23192320
new_id = PS(mod)->s_create_sid(&PS(mod_data));
2320-
if (!PS(mod)->s_validate_sid) {
2321+
if (!PS(mod)->s_validate_sid || (PS(mod_user_implemented) && Z_ISUNDEF(PS(mod_user_names).name.ps_validate_sid))) {
23212322
break;
23222323
} else {
23232324
/* Detect collision and retry */

ext/session/tests/gh9583-extra.phpt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
GH-9583: session_create_id() fails with user defined save handler that doesn't have a validateId() method
3+
--EXTENSIONS--
4+
session
5+
--SKIPIF--
6+
<?php include('skipif.inc'); ?>
7+
--FILE--
8+
<?php
9+
10+
class SessionHandlerTester implements \SessionHandlerInterface
11+
{
12+
13+
public function close(): bool { return true; }
14+
15+
public function destroy($id): bool { return true; }
16+
17+
public function gc($max_lifetime): int|false { return 1; }
18+
19+
public function open($path, $name): bool { return true; }
20+
21+
public function read($id): string { return ''; }
22+
23+
public function write($id, $data): bool { return true; }
24+
25+
//public function create_sid() { return uniqid(); }
26+
27+
//public function validateId($key) { return true; }
28+
}
29+
30+
$obj = new SessionHandlerTester();
31+
ini_set('session.use_strict_mode','1');
32+
session_set_save_handler($obj);
33+
session_start();
34+
35+
$originalSessionId = session_id();
36+
37+
session_write_close();
38+
39+
session_start();
40+
41+
$newSessionId = session_id();
42+
43+
echo 'validateId() ', (method_exists($obj, 'validateId') ? ('returns ' . ($obj->validateId(1) ? 'true' : 'false')) : 'is commented out'), "\n";
44+
var_dump($originalSessionId == $newSessionId);
45+
46+
?>
47+
--EXPECT--
48+
validateId() is commented out
49+
bool(true)

0 commit comments

Comments
 (0)