Skip to content

Commit 15fa3f1

Browse files
kenjiskenjis
authored
Merge pull request #6614 from kenjis/fix-redis-session-php81
fix: default values for Session Redis Handler
2 parents b764570 + 2a6ff86 commit 15fa3f1

File tree

2 files changed

+150
-13
lines changed

2 files changed

+150
-13
lines changed

‎system/Session/Handlers/RedisHandler.php

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
*/
2323
class RedisHandler extends BaseHandler
2424
{
25+
private const DEFAULT_PORT = 6379;
26+
2527
/**
2628
* phpRedis instance
2729
*
@@ -58,12 +60,27 @@ class RedisHandler extends BaseHandler
5860
protected $sessionExpiration = 7200;
5961

6062
/**
63+
* @param string $ipAddress User's IP address
64+
*
6165
* @throws SessionException
6266
*/
6367
public function __construct(AppConfig $config, string $ipAddress)
6468
{
6569
parent::__construct($config, $ipAddress);
6670

71+
$this->setSavePath();
72+
73+
if ($this->matchIP === true) {
74+
$this->keyPrefix .= $this->ipAddress . ':';
75+
}
76+
77+
$this->sessionExpiration = empty($config->sessionExpiration)
78+
? (int) ini_get('session.gc_maxlifetime')
79+
: (int) $config->sessionExpiration;
80+
}
81+
82+
protected function setSavePath(): void
83+
{
6784
if (empty($this->savePath)) {
6885
throw SessionException::forEmptySavepath();
6986
}
@@ -75,24 +92,16 @@ public function __construct(AppConfig $config, string $ipAddress)
7592

7693
$this->savePath = [
7794
'host' => $matches[1],
78-
'port' => empty($matches[2]) ? null : $matches[2],
95+
'port' => empty($matches[2]) ? self::DEFAULT_PORT : $matches[2],
7996
'password' => preg_match('#auth=([^\s&]+)#', $matches[3], $match) ? $match[1] : null,
80-
'database' => preg_match('#database=(\d+)#', $matches[3], $match) ? (int) $match[1] : null,
81-
'timeout' => preg_match('#timeout=(\d+\.\d+)#', $matches[3], $match) ? (float) $match[1] : null,
97+
'database' => preg_match('#database=(\d+)#', $matches[3], $match) ? (int) $match[1] : 0,
98+
'timeout' => preg_match('#timeout=(\d+\.\d+|\d+)#', $matches[3], $match) ? (float) $match[1] : 0.0,
8299
];
83100

84101
preg_match('#prefix=([^\s&]+)#', $matches[3], $match) && $this->keyPrefix = $match[1];
85102
} else {
86103
throw SessionException::forInvalidSavePathFormat($this->savePath);
87104
}
88-
89-
if ($this->matchIP === true) {
90-
$this->keyPrefix .= $this->ipAddress . ':';
91-
}
92-
93-
$this->sessionExpiration = empty($config->sessionExpiration)
94-
? (int) ini_get('session.gc_maxlifetime')
95-
: (int) $config->sessionExpiration;
96105
}
97106

98107
/**
@@ -266,14 +275,15 @@ public function gc($max_lifetime)
266275
*/
267276
protected function lockSession(string $sessionID): bool
268277
{
278+
$lockKey = $this->keyPrefix . $sessionID . ':lock';
279+
269280
// PHP 7 reuses the SessionHandler object on regeneration,
270281
// so we need to check here if the lock key is for the
271282
// correct session ID.
272-
if ($this->lockKey === $this->keyPrefix . $sessionID . ':lock') {
283+
if ($this->lockKey === $lockKey) {
273284
return $this->redis->expire($this->lockKey, 300);
274285
}
275286

276-
$lockKey = $this->keyPrefix . $sessionID . ':lock';
277287
$attempt = 0;
278288

279289
do {

‎tests/system/Session/Handlers/Database/RedisHandlerTest.php

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
<?php
2+
3+
/**
4+
* This file is part of CodeIgniter 4 framework.
5+
*
6+
* (c) CodeIgniter Foundation <[email protected]>
7+
*
8+
* For the full copyright and license information, please view
9+
* the LICENSE file that was distributed with this source code.
10+
*/
11+
12+
namespace CodeIgniter\Session\Handlers\Database;
13+
14+
use CodeIgniter\Session\Handlers\RedisHandler;
15+
use CodeIgniter\Test\CIUnitTestCase;
16+
use Config\App as AppConfig;
17+
use Redis;
18+
19+
/**
20+
* @requires extension redis
21+
*
22+
* @internal
23+
*/
24+
final class RedisHandlerTest extends CIUnitTestCase
25+
{
26+
private string $sessionName = 'ci_session';
27+
private string $sessionSavePath = 'tcp://127.0.0.1:6379';
28+
private string $userIpAddress = '127.0.0.1';
29+
30+
private function getInstance($options = [])
31+
{
32+
$defaults = [
33+
'sessionDriver' => RedisHandler::class,
34+
'sessionCookieName' => $this->sessionName,
35+
'sessionExpiration' => 7200,
36+
'sessionSavePath' => $this->sessionSavePath,
37+
'sessionMatchIP' => false,
38+
'sessionTimeToUpdate' => 300,
39+
'sessionRegenerateDestroy' => false,
40+
'cookieDomain' => '',
41+
'cookiePrefix' => '',
42+
'cookiePath' => '/',
43+
'cookieSecure' => false,
44+
'cookieSameSite' => 'Lax',
45+
];
46+
47+
$config = array_merge($defaults, $options);
48+
$appConfig = new AppConfig();
49+
50+
foreach ($config as $key => $c) {
51+
$appConfig->{$key} = $c;
52+
}
53+
54+
return new RedisHandler($appConfig, $this->userIpAddress);
55+
}
56+
57+
public function testSavePathTimeoutFloat()
58+
{
59+
$handler = $this->getInstance(
60+
['sessionSavePath' => 'tcp://127.0.0.1:6379?timeout=2.5']
61+
);
62+
63+
$savePath = $this->getPrivateProperty($handler, 'savePath');
64+
65+
$this->assertSame(2.5, $savePath['timeout']);
66+
}
67+
68+
public function testSavePathTimeoutInt()
69+
{
70+
$handler = $this->getInstance(
71+
['sessionSavePath' => 'tcp://127.0.0.1:6379?timeout=10']
72+
);
73+
74+
$savePath = $this->getPrivateProperty($handler, 'savePath');
75+
76+
$this->assertSame(10.0, $savePath['timeout']);
77+
}
78+
79+
public function testOpen()
80+
{
81+
$handler = $this->getInstance();
82+
$this->assertTrue($handler->open($this->sessionSavePath, $this->sessionName));
83+
}
84+
85+
public function testWrite()
86+
{
87+
$handler = $this->getInstance();
88+
$handler->open($this->sessionSavePath, $this->sessionName);
89+
$handler->read('555556b43phsnnf8if6bo33b635e4447');
90+
91+
$data = <<<'DATA'
92+
__ci_last_regenerate|i:1664607454;_ci_previous_url|s:32:"http://localhost:8080/index.php/";key|s:5:"value";
93+
DATA;
94+
$this->assertTrue($handler->write('555556b43phsnnf8if6bo33b635e4447', $data));
95+
96+
$handler->close();
97+
}
98+
99+
public function testReadSuccess()
100+
{
101+
$handler = $this->getInstance();
102+
$handler->open($this->sessionSavePath, $this->sessionName);
103+
104+
$expected = <<<'DATA'
105+
__ci_last_regenerate|i:1664607454;_ci_previous_url|s:32:"http://localhost:8080/index.php/";key|s:5:"value";
106+
DATA;
107+
$this->assertSame($expected, $handler->read('555556b43phsnnf8if6bo33b635e4447'));
108+
109+
$handler->close();
110+
}
111+
112+
public function testReadFailure()
113+
{
114+
$handler = $this->getInstance();
115+
$handler->open($this->sessionSavePath, $this->sessionName);
116+
117+
$this->assertSame('', $handler->read('123456b43phsnnf8if6bo33b635e4321'));
118+
119+
$handler->close();
120+
}
121+
122+
public function testGC()
123+
{
124+
$handler = $this->getInstance();
125+
$this->assertSame(1, $handler->gc(3600));
126+
}
127+
}

0 commit comments

Comments
 (0)