Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5366285

Browse filesBrowse files
michael-grundermichael-grunder
committed
Fix session handlers for php 8.6.0
PHP 8.6 is making session strict mode the default. This slightly changes the semantics around session ids. This fix mostly just allows us to disable strict mode for our tests which need to use their own ids to confirm locking functionality. Also we add some logic to the session handlers to keep track of if we need to generate an ID which should allow us to preserve lazy write semantics. Fixes #2898
1 parent e112247 commit 5366285
Copy full SHA for 5366285

5 files changed

+66-16Lines changed: 66 additions & 16 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎cluster_library.h‎

Copy file name to clipboardExpand all lines: cluster_library.h
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ typedef struct redisCluster {
185185
/* RedisCluster failover options (never, on error, to load balance) */
186186
short failover;
187187

188+
/* Whether the current session read found no backing key */
189+
zend_bool session_key_missing;
190+
188191
/* Hash table of seed host/ports */
189192
HashTable *seeds;
190193

Collapse file

‎redis_session.c‎

Copy file name to clipboardExpand all lines: redis_session.c
+31-10Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ typedef struct {
111111

112112
redis_pool_member *head;
113113
redis_session_lock_status lock_status;
114+
zend_bool session_key_missing;
114115
int buster;
115116
} redis_pool;
116117

@@ -904,12 +905,15 @@ PS_UPDATE_TIMESTAMP_FUNC(redis)
904905
if (ZSTR_LEN(key) < 1)
905906
return FAILURE;
906907

907-
/* No need to update the session timestamp if we've already done so */
908-
if (zend_ini_long_literal("redis.session.early_refresh")) {
908+
redis_pool *pool = PS_GET_MOD_DATA();
909+
910+
/* GETEX already refreshed an existing session during the read */
911+
if (zend_ini_long_literal("redis.session.early_refresh") &&
912+
!pool->session_key_missing
913+
) {
909914
return SUCCESS;
910915
}
911916

912-
redis_pool *pool = PS_GET_MOD_DATA();
913917
redis_pool_member *rpm = redis_pool_get_sock(pool, key);
914918
RedisSock *redis_sock = rpm ? rpm->redis_sock : NULL;
915919
if (!redis_sock) {
@@ -934,9 +938,14 @@ PS_UPDATE_TIMESTAMP_FUNC(redis)
934938

935939
/* Do the full check in the next major version */
936940
res = rlen == 2 && rstr[0] == ':';
941+
zend_bool missing = res && rstr[1] == '0';
937942

938943
efree(rstr);
939944

945+
if (missing) {
946+
return ps_write_redis(mod_data, key, val, maxlifetime);
947+
}
948+
940949
return res ? SUCCESS : FAILURE;
941950
}
942951
/* }}} */
@@ -1004,7 +1013,9 @@ PS_READ_FUNC(redis)
10041013
return FAILURE;
10051014
}
10061015

1007-
if (resp_len < 0) {
1016+
pool->session_key_missing = resp_len < 0;
1017+
1018+
if (pool->session_key_missing) {
10081019
*val = ZSTR_EMPTY_ALLOC();
10091020
} else {
10101021
compressed_free = session_uncompress_data(redis_sock, resp, resp_len, &compressed_buf, &compressed_len);
@@ -1400,16 +1411,18 @@ PS_UPDATE_TIMESTAMP_FUNC(rediscluster) {
14001411
RedisCmd *cmd;
14011412
short slot;
14021413

1403-
/* No need to update the session timestamp if we've already done so */
1404-
if (zend_ini_long_literal("redis.session.early_refresh")) {
1414+
/* GETEX already refreshed an existing session during the read */
1415+
if (zend_ini_long_literal("redis.session.early_refresh") &&
1416+
!c->session_key_missing
1417+
) {
14051418
return SUCCESS;
14061419
}
14071420

14081421
/* Set up command and slot info */
1409-
key = cluster_session_key(c, key, &slot);
1410-
cmd = redis_cmd_fmt(NULL, "EXPIRE", "Sd", key, session_gc_maxlifetime());
1422+
zend_string *session = cluster_session_key(c, key, &slot);
1423+
cmd = redis_cmd_fmt(NULL, "EXPIRE", "Sd", session, session_gc_maxlifetime());
14111424

1412-
zend_string_release(key);
1425+
zend_string_release(session);
14131426

14141427
/* Attempt to send EXPIRE command */
14151428
c->readonly = 0;
@@ -1429,9 +1442,15 @@ PS_UPDATE_TIMESTAMP_FUNC(rediscluster) {
14291442
return FAILURE;
14301443
}
14311444

1445+
zend_bool missing = reply->type == TYPE_INT && reply->integer == 0;
1446+
14321447
/* Clean up */
14331448
cluster_free_reply(reply, 1);
14341449

1450+
if (missing) {
1451+
return ps_write_rediscluster(mod_data, key, val, maxlifetime);
1452+
}
1453+
14351454
return SUCCESS;
14361455
}
14371456
/* }}} */
@@ -1479,7 +1498,9 @@ PS_READ_FUNC(rediscluster) {
14791498
}
14801499

14811500
/* Push reply value to caller */
1482-
if (reply->str == NULL) {
1501+
c->session_key_missing = reply->len == -1;
1502+
1503+
if (c->session_key_missing) {
14831504
*val = ZSTR_EMPTY_ALLOC();
14841505
} else {
14851506
compressed_free = session_uncompress_data(c->flags, reply->str, reply->len, &compressed_buf, &compressed_len);
Collapse file

‎tests/RedisTest.php‎

Copy file name to clipboardExpand all lines: tests/RedisTest.php
+17-4Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8561,11 +8561,24 @@ public function testSession_savedToRedis() {
85618561
$runner = $this->sessionRunner();
85628562

85638563
$this->assertSessionRunnerResult($runner);
8564-
$this->assertKeyExists($runner->getSessionKey());
8564+
if ( ! $this->assertKeyExists($runner->getSessionKey())) {
8565+
$this->externalCmdFailure($runner->getCmd(), $runner->output(),
8566+
'Failed to save session data to Redis',
8567+
$runner->getExitCode());
8568+
}
8569+
}
85658570

8566-
$this->externalCmdFailure($runner->getCmd(), $runner->output(),
8567-
'Failed to save session data to Redis',
8568-
$runner->getExitCode());
8571+
public function testSession_savedToRedisEarlyRefresh() {
8572+
$this->testRequiresMode('cli');
8573+
8574+
$runner = $this->sessionRunner()->earlyRefresh(true);
8575+
8576+
$this->assertSessionRunnerResult($runner);
8577+
if ( ! $this->assertKeyExists($runner->getSessionKey())) {
8578+
$this->externalCmdFailure($runner->getCmd(), $runner->output(),
8579+
'Failed to save session data to Redis',
8580+
$runner->getExitCode());
8581+
}
85698582
}
85708583

85718584
protected function sessionWaitUsec() {
Collapse file

‎tests/SessionHelpers.php‎

Copy file name to clipboardExpand all lines: tests/SessionHelpers.php
+10-1Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ class Runner {
7777
'data' => '',
7878
'lifetime' => 1440,
7979
'compression' => 'none',
80+
'strict-mode' => false,
81+
'early-refresh' => false,
8082
];
8183

8284
private $prefix = NULL;
@@ -174,6 +176,14 @@ public function compression(string $compression): self {
174176
return $this->set('compression', $compression);
175177
}
176178

179+
public function strictMode(bool $enabled): self {
180+
return $this->set('strict-mode', $enabled);
181+
}
182+
183+
public function earlyRefresh(bool $enabled): self {
184+
return $this->set('early-refresh', $enabled);
185+
}
186+
177187
protected function validateArgs(array $required) {
178188
foreach ($required as $req) {
179189
if ( ! isset($this->args[$req]) || $this->args[$req] === null)
@@ -242,7 +252,6 @@ private function startSessionCmd(): string {
242252

243253
public function output(?int $timeout = NULL): ?string {
244254
if ($this->output) {
245-
var_dump("early return");
246255
return $this->output;
247256
}
248257

Collapse file

‎tests/startSession.php‎

Copy file name to clipboardExpand all lines: tests/startSession.php
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
$opt = getopt('', [
55
'handler:', 'save-path:', 'id:', 'sleep:', 'max-execution-time:' ,
66
'locking-enabled:', 'lock-wait-time:', 'lock-retries:', 'lock-expires:',
7-
'data:', 'lifetime:', 'compression:'
7+
'data:', 'lifetime:', 'compression:', 'strict-mode:', 'early-refresh:'
88
]);
99

1010
$handler = $opt['handler'] ?? NULL;
@@ -19,6 +19,8 @@
1919
$locking_enabled = $opt['locking-enabled'] ?? NULL;
2020
$lock_wait_time = $opt['lock-wait-time'] ?? 0;
2121
$compression = $opt['compression'] ?? NULL;
22+
$strict_mode = !!($opt['strict-mode'] ?? false);
23+
$early_refresh = !!($opt['early-refresh'] ?? false);
2224

2325
if ( ! $handler) {
2426
fprintf(STDERR, "--handler is required\n");
@@ -37,6 +39,8 @@
3739
ini_set("{$handler}.session.locking_enabled", $locking_enabled);
3840
ini_set("{$handler}.session.lock_wait_time", $lock_wait_time);
3941
ini_set('redis.session.compression', $compression);
42+
ini_set('session.use_strict_mode', $strict_mode);
43+
ini_set('redis.session.early_refresh', $early_refresh);
4044

4145
session_id($id);
4246
$status = session_start();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.