diff --git a/phpstan-baseline.neon.dist b/phpstan-baseline.neon.dist index 7c0704286696..1021eb445899 100644 --- a/phpstan-baseline.neon.dist +++ b/phpstan-baseline.neon.dist @@ -745,26 +745,6 @@ parameters: count: 1 path: system/Session/Handlers/DatabaseHandler.php - - - message: "#^Property CodeIgniter\\\\Session\\\\Handlers\\\\BaseHandler\\:\\:\\$sessionID \\(string\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: system/Session/Handlers/DatabaseHandler.php - - - - message: "#^Property CodeIgniter\\\\Session\\\\Handlers\\\\BaseHandler\\:\\:\\$sessionID \\(string\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: system/Session/Handlers/FileHandler.php - - - - message: "#^Property CodeIgniter\\\\Session\\\\Handlers\\\\BaseHandler\\:\\:\\$sessionID \\(string\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: system/Session/Handlers/MemcachedHandler.php - - - - message: "#^Property CodeIgniter\\\\Session\\\\Handlers\\\\BaseHandler\\:\\:\\$sessionID \\(string\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: system/Session/Handlers/RedisHandler.php - - message: "#^Strict comparison using \\=\\=\\= between string and true will always evaluate to false\\.$#" count: 1 diff --git a/system/Config/Services.php b/system/Config/Services.php index 2d44e3ccfd78..9dca50115f51 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -47,6 +47,9 @@ use CodeIgniter\Router\RouteCollectionInterface; use CodeIgniter\Router\Router; use CodeIgniter\Security\Security; +use CodeIgniter\Session\Handlers\Database\MySQLiHandler; +use CodeIgniter\Session\Handlers\Database\PostgreHandler; +use CodeIgniter\Session\Handlers\DatabaseHandler; use CodeIgniter\Session\Session; use CodeIgniter\Throttle\Throttler; use CodeIgniter\Typography\Typography; @@ -58,6 +61,7 @@ use Config\App; use Config\Cache; use Config\ContentSecurityPolicy as CSPConfig; +use Config\Database; use Config\Email as EmailConfig; use Config\Encryption as EncryptionConfig; use Config\Exceptions as ExceptionsConfig; @@ -585,7 +589,21 @@ public static function session(?App $config = null, bool $getShared = true) $logger = AppServices::logger(); $driverName = $config->sessionDriver; - $driver = new $driverName($config, AppServices::request()->getIPAddress()); + + if ($driverName === DatabaseHandler::class) { + $DBGroup = $config->sessionDBGroup ?? config(Database::class)->defaultGroup; + $db = Database::connect($DBGroup); + + $driver = $db->getPlatform(); + + if ($driver === 'MySQLi') { + $driverName = MySQLiHandler::class; + } elseif ($driver === 'Postgre') { + $driverName = PostgreHandler::class; + } + } + + $driver = new $driverName($config, AppServices::request()->getIPAddress()); $driver->setLogger($logger); $session = new Session($driver, $config); diff --git a/system/Database/BaseConnection.php b/system/Database/BaseConnection.php index d2ce3fdd390a..23a78d423c47 100644 --- a/system/Database/BaseConnection.php +++ b/system/Database/BaseConnection.php @@ -508,7 +508,7 @@ public function getPrefix(): string } /** - * The name of the platform in use (MySQLi, mssql, etc) + * The name of the platform in use (MySQLi, Postgre, SQLite3, OCI8, etc) */ public function getPlatform(): string { diff --git a/system/Session/Handlers/BaseHandler.php b/system/Session/Handlers/BaseHandler.php index f6fa57b23b68..008cae369e80 100644 --- a/system/Session/Handlers/BaseHandler.php +++ b/system/Session/Handlers/BaseHandler.php @@ -81,7 +81,7 @@ abstract class BaseHandler implements SessionHandlerInterface /** * Current session ID * - * @var string + * @var string|null */ protected $sessionID; diff --git a/system/Session/Handlers/Database/MySQLiHandler.php b/system/Session/Handlers/Database/MySQLiHandler.php new file mode 100644 index 000000000000..fabaae451e6a --- /dev/null +++ b/system/Session/Handlers/Database/MySQLiHandler.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Session\Handlers\Database; + +use CodeIgniter\Session\Handlers\DatabaseHandler; + +/** + * Session handler for MySQLi + */ +class MySQLiHandler extends DatabaseHandler +{ + /** + * Lock the session. + */ + protected function lockSession(string $sessionID): bool + { + $arg = md5($sessionID . ($this->matchIP ? '_' . $this->ipAddress : '')); + if ($this->db->query("SELECT GET_LOCK('{$arg}', 300) AS ci_session_lock")->getRow()->ci_session_lock) { + $this->lock = $arg; + + return true; + } + + return $this->fail(); + } + + /** + * Releases the lock, if any. + */ + protected function releaseLock(): bool + { + if (! $this->lock) { + return true; + } + + if ($this->db->query("SELECT RELEASE_LOCK('{$this->lock}') AS ci_session_lock")->getRow()->ci_session_lock) { + $this->lock = false; + + return true; + } + + return $this->fail(); + } +} diff --git a/system/Session/Handlers/Database/PostgreHandler.php b/system/Session/Handlers/Database/PostgreHandler.php new file mode 100644 index 000000000000..2b730d831c89 --- /dev/null +++ b/system/Session/Handlers/Database/PostgreHandler.php @@ -0,0 +1,100 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Session\Handlers\Database; + +use CodeIgniter\Database\BaseBuilder; +use CodeIgniter\Session\Handlers\DatabaseHandler; +use ReturnTypeWillChange; + +/** + * Session handler for Postgre + */ +class PostgreHandler extends DatabaseHandler +{ + /** + * Sets SELECT clause + */ + protected function setSelect(BaseBuilder $builder) + { + $builder->select("encode(data, 'base64') AS data"); + } + + /** + * Decodes column data + * + * @param mixed $data + * + * @return false|string + */ + protected function decodeData($data) + { + return base64_decode(rtrim($data), true); + } + + /** + * Prepare data to insert/update + */ + protected function prepareData(string $data): string + { + return '\x' . bin2hex($data); + } + + /** + * Cleans up expired sessions. + * + * @param int $max_lifetime Sessions that have not updated + * for the last max_lifetime seconds will be removed. + * + * @return false|int Returns the number of deleted sessions on success, or false on failure. + */ + #[ReturnTypeWillChange] + public function gc($max_lifetime) + { + $separator = '\''; + $interval = implode($separator, ['', "{$max_lifetime} second", '']); + + return $this->db->table($this->table)->where('timestamp <', "now() - INTERVAL {$interval}", false)->delete() ? 1 : $this->fail(); + } + + /** + * Lock the session. + */ + protected function lockSession(string $sessionID): bool + { + $arg = "hashtext('{$sessionID}')" . ($this->matchIP ? ", hashtext('{$this->ipAddress}')" : ''); + if ($this->db->simpleQuery("SELECT pg_advisory_lock({$arg})")) { + $this->lock = $arg; + + return true; + } + + return $this->fail(); + } + + /** + * Releases the lock, if any. + */ + protected function releaseLock(): bool + { + if (! $this->lock) { + return true; + } + + if ($this->db->simpleQuery("SELECT pg_advisory_unlock({$this->lock})")) { + $this->lock = false; + + return true; + } + + return $this->fail(); + } +} diff --git a/system/Session/Handlers/DatabaseHandler.php b/system/Session/Handlers/DatabaseHandler.php index dfaa472029fd..8cbf43ca280a 100644 --- a/system/Session/Handlers/DatabaseHandler.php +++ b/system/Session/Handlers/DatabaseHandler.php @@ -11,6 +11,7 @@ namespace CodeIgniter\Session\Handlers; +use CodeIgniter\Database\BaseBuilder; use CodeIgniter\Database\BaseConnection; use CodeIgniter\Session\Exceptions\SessionException; use Config\App as AppConfig; @@ -18,7 +19,9 @@ use ReturnTypeWillChange; /** - * Session handler using current Database for storage + * Base database session handler + * + * Do not use this class. Use database specific handler class. */ class DatabaseHandler extends BaseHandler { @@ -44,7 +47,7 @@ class DatabaseHandler extends BaseHandler protected $db; /** - * The database type, for locking purposes. + * The database type * * @var string */ @@ -73,13 +76,7 @@ public function __construct(AppConfig $config, string $ipAddress) $this->db = Database::connect($this->DBGroup); - $driver = strtolower(get_class($this->db)); - - if (strpos($driver, 'mysql') !== false) { - $this->platform = 'mysql'; - } elseif (strpos($driver, 'postgre') !== false) { - $this->platform = 'postgre'; - } + $this->platform = $this->db->getPlatform(); } /** @@ -118,14 +115,14 @@ public function read($id) $this->sessionID = $id; } - $builder = $this->db->table($this->table) - ->select($this->platform === 'postgre' ? "encode(data, 'base64') AS data" : 'data') - ->where('id', $id); + $builder = $this->db->table($this->table)->where('id', $id); if ($this->matchIP) { $builder = $builder->where('ip_address', $this->ipAddress); } + $this->setSelect($builder); + $result = $builder->get()->getRow(); if ($result === null) { @@ -138,11 +135,7 @@ public function read($id) return ''; } - if (is_bool($result)) { - $result = ''; - } else { - $result = ($this->platform === 'postgre') ? base64_decode(rtrim($result->data), true) : $result->data; - } + $result = is_bool($result) ? '' : $this->decodeData($result->data); $this->fingerprint = md5($result); $this->rowExists = true; @@ -150,6 +143,26 @@ public function read($id) return $result; } + /** + * Sets SELECT clause + */ + protected function setSelect(BaseBuilder $builder) + { + $builder->select('data'); + } + + /** + * Decodes column data + * + * @param mixed $data + * + * @return false|string + */ + protected function decodeData($data) + { + return $data; + } + /** * Writes the session data to the session storage. * @@ -171,7 +184,7 @@ public function write($id, $data): bool $insertData = [ 'id' => $id, 'ip_address' => $this->ipAddress, - 'data' => $this->platform === 'postgre' ? '\x' . bin2hex($data) : $data, + 'data' => $this->prepareData($data), ]; if (! $this->db->table($this->table)->set('timestamp', 'now()', false)->insert($insertData)) { @@ -193,7 +206,7 @@ public function write($id, $data): bool $updateData = []; if ($this->fingerprint !== md5($data)) { - $updateData['data'] = ($this->platform === 'postgre') ? '\x' . bin2hex($data) : $data; + $updateData['data'] = $this->prepareData($data); } if (! $builder->set('timestamp', 'now()', false)->update($updateData)) { @@ -205,6 +218,14 @@ public function write($id, $data): bool return true; } + /** + * Prepare data to insert/update + */ + protected function prepareData(string $data): string + { + return $data; + } + /** * Closes the current session. */ @@ -252,43 +273,12 @@ public function destroy($id): bool #[ReturnTypeWillChange] public function gc($max_lifetime) { - $separator = $this->platform === 'postgre' ? '\'' : ' '; + $separator = ' '; $interval = implode($separator, ['', "{$max_lifetime} second", '']); return $this->db->table($this->table)->where('timestamp <', "now() - INTERVAL {$interval}", false)->delete() ? 1 : $this->fail(); } - /** - * Lock the session. - */ - protected function lockSession(string $sessionID): bool - { - if ($this->platform === 'mysql') { - $arg = md5($sessionID . ($this->matchIP ? '_' . $this->ipAddress : '')); - if ($this->db->query("SELECT GET_LOCK('{$arg}', 300) AS ci_session_lock")->getRow()->ci_session_lock) { - $this->lock = $arg; - - return true; - } - - return $this->fail(); - } - - if ($this->platform === 'postgre') { - $arg = "hashtext('{$sessionID}')" . ($this->matchIP ? ", hashtext('{$this->ipAddress}')" : ''); - if ($this->db->simpleQuery("SELECT pg_advisory_lock({$arg})")) { - $this->lock = $arg; - - return true; - } - - return $this->fail(); - } - - // Unsupported DB? Let the parent handle the simplified version. - return parent::lockSession($sessionID); - } - /** * Releases the lock, if any. */ @@ -298,26 +288,6 @@ protected function releaseLock(): bool return true; } - if ($this->platform === 'mysql') { - if ($this->db->query("SELECT RELEASE_LOCK('{$this->lock}') AS ci_session_lock")->getRow()->ci_session_lock) { - $this->lock = false; - - return true; - } - - return $this->fail(); - } - - if ($this->platform === 'postgre') { - if ($this->db->simpleQuery("SELECT pg_advisory_unlock({$this->lock})")) { - $this->lock = false; - - return true; - } - - return $this->fail(); - } - // Unsupported DB? Let the parent handle the simple version. return parent::releaseLock(); } diff --git a/tests/system/Session/Handlers/DatabaseHandlerTest.php b/tests/system/Session/Handlers/Database/AbstactHandlerTestCase.php similarity index 78% rename from tests/system/Session/Handlers/DatabaseHandlerTest.php rename to tests/system/Session/Handlers/Database/AbstactHandlerTestCase.php index 7af077606cba..7434301f40cc 100644 --- a/tests/system/Session/Handlers/DatabaseHandlerTest.php +++ b/tests/system/Session/Handlers/Database/AbstactHandlerTestCase.php @@ -9,18 +9,17 @@ * the LICENSE file that was distributed with this source code. */ -namespace CodeIgniter\Session\Handlers; +namespace CodeIgniter\Session\Handlers\Database; use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\DatabaseTestTrait; use CodeIgniter\Test\ReflectionHelper; -use Config\App as AppConfig; use Config\Database as DatabaseConfig; /** * @internal */ -final class DatabaseHandlerTest extends CIUnitTestCase +abstract class AbstactHandlerTestCase extends CIUnitTestCase { use DatabaseTestTrait; use ReflectionHelper; @@ -37,32 +36,7 @@ protected function setUp(): void } } - protected function getInstance($options = []) - { - $defaults = [ - 'sessionDriver' => DatabaseHandler::class, - 'sessionCookieName' => 'ci_session', - 'sessionExpiration' => 7200, - 'sessionSavePath' => 'ci_sessions', - 'sessionMatchIP' => false, - 'sessionTimeToUpdate' => 300, - 'sessionRegenerateDestroy' => false, - 'cookieDomain' => '', - 'cookiePrefix' => '', - 'cookiePath' => '/', - 'cookieSecure' => false, - 'cookieSameSite' => 'Lax', - ]; - - $config = array_merge($defaults, $options); - $appConfig = new AppConfig(); - - foreach ($config as $key => $c) { - $appConfig->{$key} = $c; - } - - return new DatabaseHandler($appConfig, '127.0.0.1'); - } + abstract protected function getInstance($options = []); public function testOpen() { diff --git a/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php b/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php new file mode 100644 index 000000000000..469b0c8e1f7e --- /dev/null +++ b/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php @@ -0,0 +1,58 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Session\Handlers\Database; + +use CodeIgniter\Session\Handlers\DatabaseHandler; +use Config\App as AppConfig; +use Config\Database as DatabaseConfig; + +/** + * @internal + */ +final class MySQLiHandlerTest extends AbstactHandlerTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + if (config(DatabaseConfig::class)->tests['DBDriver'] !== 'MySQLi') { + $this->markTestSkipped('This test case needs MySQLi'); + } + } + + protected function getInstance($options = []) + { + $defaults = [ + 'sessionDriver' => DatabaseHandler::class, + 'sessionCookieName' => 'ci_session', + 'sessionExpiration' => 7200, + 'sessionSavePath' => 'ci_sessions', + 'sessionMatchIP' => false, + 'sessionTimeToUpdate' => 300, + 'sessionRegenerateDestroy' => false, + 'cookieDomain' => '', + 'cookiePrefix' => '', + 'cookiePath' => '/', + 'cookieSecure' => false, + 'cookieSameSite' => 'Lax', + ]; + + $config = array_merge($defaults, $options); + $appConfig = new AppConfig(); + + foreach ($config as $key => $c) { + $appConfig->{$key} = $c; + } + + return new MySQLiHandler($appConfig, '127.0.0.1'); + } +} diff --git a/tests/system/Session/Handlers/Database/PostgreHandlerTest.php b/tests/system/Session/Handlers/Database/PostgreHandlerTest.php new file mode 100644 index 000000000000..9575b68e499f --- /dev/null +++ b/tests/system/Session/Handlers/Database/PostgreHandlerTest.php @@ -0,0 +1,58 @@ + + * + * For the full copyright and license information, please view + * the LICENSE file that was distributed with this source code. + */ + +namespace CodeIgniter\Session\Handlers\Database; + +use CodeIgniter\Session\Handlers\DatabaseHandler; +use Config\App as AppConfig; +use Config\Database as DatabaseConfig; + +/** + * @internal + */ +final class PostgreHandlerTest extends AbstactHandlerTestCase +{ + protected function setUp(): void + { + parent::setUp(); + + if (config(DatabaseConfig::class)->tests['DBDriver'] !== 'Postgre') { + $this->markTestSkipped('This test case needs Postgre'); + } + } + + protected function getInstance($options = []) + { + $defaults = [ + 'sessionDriver' => DatabaseHandler::class, + 'sessionCookieName' => 'ci_session', + 'sessionExpiration' => 7200, + 'sessionSavePath' => 'ci_sessions', + 'sessionMatchIP' => false, + 'sessionTimeToUpdate' => 300, + 'sessionRegenerateDestroy' => false, + 'cookieDomain' => '', + 'cookiePrefix' => '', + 'cookiePath' => '/', + 'cookieSecure' => false, + 'cookieSameSite' => 'Lax', + ]; + + $config = array_merge($defaults, $options); + $appConfig = new AppConfig(); + + foreach ($config as $key => $c) { + $appConfig->{$key} = $c; + } + + return new PostgreHandler($appConfig, '127.0.0.1'); + } +}