From 49be3f1d23b470c40e0ea29867ea046bb01d8060 Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 14 Feb 2023 19:40:01 +0900 Subject: [PATCH 1/9] fix: remove Session config items in Config\App --- app/Config/App.php | 107 ------------------ .../Generators/MigrationGenerator.php | 6 +- system/Config/Services.php | 4 +- system/Session/Handlers/BaseHandler.php | 13 +-- system/Session/Handlers/DatabaseHandler.php | 13 +-- system/Session/Handlers/MemcachedHandler.php | 8 +- system/Session/Handlers/RedisHandler.php | 19 +--- system/Session/Session.php | 25 ++-- 8 files changed, 24 insertions(+), 171 deletions(-) diff --git a/app/Config/App.php b/app/Config/App.php index 322526f06756..3eeb1573bae9 100644 --- a/app/Config/App.php +++ b/app/Config/App.php @@ -3,7 +3,6 @@ namespace Config; use CodeIgniter\Config\BaseConfig; -use CodeIgniter\Session\Handlers\FileHandler; class App extends BaseConfig { @@ -136,112 +135,6 @@ class App extends BaseConfig */ public bool $forceGlobalSecureRequests = false; - /** - * -------------------------------------------------------------------------- - * Session Driver - * -------------------------------------------------------------------------- - * - * The session storage driver to use: - * - `CodeIgniter\Session\Handlers\FileHandler` - * - `CodeIgniter\Session\Handlers\DatabaseHandler` - * - `CodeIgniter\Session\Handlers\MemcachedHandler` - * - `CodeIgniter\Session\Handlers\RedisHandler` - * - * @deprecated use Config\Session::$driver instead. - */ - public string $sessionDriver = FileHandler::class; - - /** - * -------------------------------------------------------------------------- - * Session Cookie Name - * -------------------------------------------------------------------------- - * - * The session cookie name, must contain only [0-9a-z_-] characters - * - * @deprecated use Config\Session::$cookieName instead. - */ - public string $sessionCookieName = 'ci_session'; - - /** - * -------------------------------------------------------------------------- - * Session Expiration - * -------------------------------------------------------------------------- - * - * The number of SECONDS you want the session to last. - * Setting to 0 (zero) means expire when the browser is closed. - * - * @deprecated use Config\Session::$expiration instead. - */ - public int $sessionExpiration = 7200; - - /** - * -------------------------------------------------------------------------- - * Session Save Path - * -------------------------------------------------------------------------- - * - * The location to save sessions to and is driver dependent. - * - * For the 'files' driver, it's a path to a writable directory. - * WARNING: Only absolute paths are supported! - * - * For the 'database' driver, it's a table name. - * Please read up the manual for the format with other session drivers. - * - * IMPORTANT: You are REQUIRED to set a valid save path! - * - * @deprecated use Config\Session::$savePath instead. - */ - public string $sessionSavePath = WRITEPATH . 'session'; - - /** - * -------------------------------------------------------------------------- - * Session Match IP - * -------------------------------------------------------------------------- - * - * Whether to match the user's IP address when reading the session data. - * - * WARNING: If you're using the database driver, don't forget to update - * your session table's PRIMARY KEY when changing this setting. - * - * @deprecated use Config\Session::$matchIP instead. - */ - public bool $sessionMatchIP = false; - - /** - * -------------------------------------------------------------------------- - * Session Time to Update - * -------------------------------------------------------------------------- - * - * How many seconds between CI regenerating the session ID. - * - * @deprecated use Config\Session::$timeToUpdate instead. - */ - public int $sessionTimeToUpdate = 300; - - /** - * -------------------------------------------------------------------------- - * Session Regenerate Destroy - * -------------------------------------------------------------------------- - * - * Whether to destroy session data associated with the old session ID - * when auto-regenerating the session ID. When set to FALSE, the data - * will be later deleted by the garbage collector. - * - * @deprecated use Config\Session::$regenerateDestroy instead. - */ - public bool $sessionRegenerateDestroy = false; - - /** - * -------------------------------------------------------------------------- - * Session Database Group - * -------------------------------------------------------------------------- - * - * DB Group for the database session. - * - * @deprecated use Config\Session::$DBGroup instead. - */ - public ?string $sessionDBGroup = null; - /** * -------------------------------------------------------------------------- * Reverse Proxy IPs diff --git a/system/Commands/Generators/MigrationGenerator.php b/system/Commands/Generators/MigrationGenerator.php index 817c16885205..5ebab33c3708 100644 --- a/system/Commands/Generators/MigrationGenerator.php +++ b/system/Commands/Generators/MigrationGenerator.php @@ -14,7 +14,6 @@ use CodeIgniter\CLI\BaseCommand; use CodeIgniter\CLI\CLI; use CodeIgniter\CLI\GeneratorTrait; -use Config\App as AppConfig; use Config\Session as SessionConfig; /** @@ -109,13 +108,10 @@ protected function prepare(string $class): string $data['DBGroup'] = is_string($DBGroup) ? $DBGroup : 'default'; $data['DBDriver'] = config('Database')->{$data['DBGroup']}['DBDriver']; - /** @var AppConfig $config */ - $config = config('App'); /** @var SessionConfig|null $session */ $session = config('Session'); - $data['matchIP'] = ($session instanceof SessionConfig) - ? $session->matchIP : $config->sessionMatchIP; + $data['matchIP'] = $session->matchIP; } return $this->parseTemplate($class, [], [], $data); diff --git a/system/Config/Services.php b/system/Config/Services.php index 161a6b112757..1640c4f04bd8 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -653,10 +653,10 @@ public static function session(?App $config = null, bool $getShared = true) /** @var SessionConfig|null $sessionConfig */ $sessionConfig = config('Session'); - $driverName = $sessionConfig->driver ?? $config->sessionDriver; + $driverName = $sessionConfig->driver; if ($driverName === DatabaseHandler::class) { - $DBGroup = $sessionConfig->DBGroup ?? $config->sessionDBGroup ?? config(Database::class)->defaultGroup; + $DBGroup = $sessionConfig->DBGroup ?? config(Database::class)->defaultGroup; $db = Database::connect($DBGroup); $driver = $db->getPlatform(); diff --git a/system/Session/Handlers/BaseHandler.php b/system/Session/Handlers/BaseHandler.php index ab6d7e17bf73..00cf1413139c 100644 --- a/system/Session/Handlers/BaseHandler.php +++ b/system/Session/Handlers/BaseHandler.php @@ -111,16 +111,9 @@ public function __construct(AppConfig $config, string $ipAddress) $session = config('Session'); // Store Session configurations - if ($session instanceof SessionConfig) { - $this->cookieName = $session->cookieName; - $this->matchIP = $session->matchIP; - $this->savePath = $session->savePath; - } else { - // `Config/Session.php` is absence - $this->cookieName = $config->sessionCookieName; - $this->matchIP = $config->sessionMatchIP; - $this->savePath = $config->sessionSavePath; - } + $this->cookieName = $session->cookieName; + $this->matchIP = $session->matchIP; + $this->savePath = $session->savePath; /** @var CookieConfig $cookie */ $cookie = config('Cookie'); diff --git a/system/Session/Handlers/DatabaseHandler.php b/system/Session/Handlers/DatabaseHandler.php index b566a3b04d9e..2cc8917b5f27 100644 --- a/system/Session/Handlers/DatabaseHandler.php +++ b/system/Session/Handlers/DatabaseHandler.php @@ -77,16 +77,9 @@ public function __construct(AppConfig $config, string $ipAddress) $session = config('Session'); // Store Session configurations - if ($session instanceof SessionConfig) { - $this->DBGroup = $session->DBGroup ?? config(Database::class)->defaultGroup; - // Add sessionCookieName for multiple session cookies. - $this->idPrefix = $session->cookieName . ':'; - } else { - // `Config/Session.php` is absence - $this->DBGroup = $config->sessionDBGroup ?? config(Database::class)->defaultGroup; - // Add sessionCookieName for multiple session cookies. - $this->idPrefix = $config->sessionCookieName . ':'; - } + $this->DBGroup = $session->DBGroup ?? config(Database::class)->defaultGroup; + // Add sessionCookieName for multiple session cookies. + $this->idPrefix = $session->cookieName . ':'; $this->table = $this->savePath; if (empty($this->table)) { diff --git a/system/Session/Handlers/MemcachedHandler.php b/system/Session/Handlers/MemcachedHandler.php index e4c8c6e8c673..f7d6210a9293 100644 --- a/system/Session/Handlers/MemcachedHandler.php +++ b/system/Session/Handlers/MemcachedHandler.php @@ -58,19 +58,17 @@ public function __construct(AppConfig $config, string $ipAddress) { parent::__construct($config, $ipAddress); - /** @var SessionConfig|null $session */ + /** @var SessionConfig $session */ $session = config('Session'); - $this->sessionExpiration = ($session instanceof SessionConfig) - ? $session->expiration : $config->sessionExpiration; + $this->sessionExpiration = $session->expiration; if (empty($this->savePath)) { throw SessionException::forEmptySavepath(); } // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= ($session instanceof SessionConfig) - ? $session->cookieName : $config->sessionCookieName . ':'; + $this->keyPrefix .= $session->cookieName . ':'; if ($this->matchIP === true) { $this->keyPrefix .= $this->ipAddress . ':'; diff --git a/system/Session/Handlers/RedisHandler.php b/system/Session/Handlers/RedisHandler.php index 9910637663f5..d2cdb0337eae 100644 --- a/system/Session/Handlers/RedisHandler.php +++ b/system/Session/Handlers/RedisHandler.php @@ -75,20 +75,11 @@ public function __construct(AppConfig $config, string $ipAddress) $session = config('Session'); // Store Session configurations - if ($session instanceof SessionConfig) { - $this->sessionExpiration = empty($session->expiration) - ? (int) ini_get('session.gc_maxlifetime') - : (int) $session->expiration; - // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= $session->cookieName . ':'; - } else { - // `Config/Session.php` is absence - $this->sessionExpiration = empty($config->sessionExpiration) - ? (int) ini_get('session.gc_maxlifetime') - : (int) $config->sessionExpiration; - // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= $config->sessionCookieName . ':'; - } + $this->sessionExpiration = empty($session->expiration) + ? (int) ini_get('session.gc_maxlifetime') + : (int) $session->expiration; + // Add sessionCookieName for multiple session cookies. + $this->keyPrefix .= $session->cookieName . ':'; $this->setSavePath(); diff --git a/system/Session/Session.php b/system/Session/Session.php index 75fba76e478f..e4b4a5fb2519 100644 --- a/system/Session/Session.php +++ b/system/Session/Session.php @@ -169,24 +169,13 @@ public function __construct(SessionHandlerInterface $driver, App $config) $session = config('Session'); // Store Session configurations - if ($session instanceof SessionConfig) { - $this->sessionDriverName = $session->driver; - $this->sessionCookieName = $session->cookieName ?? $this->sessionCookieName; - $this->sessionExpiration = $session->expiration ?? $this->sessionExpiration; - $this->sessionSavePath = $session->savePath; - $this->sessionMatchIP = $session->matchIP ?? $this->sessionMatchIP; - $this->sessionTimeToUpdate = $session->timeToUpdate ?? $this->sessionTimeToUpdate; - $this->sessionRegenerateDestroy = $session->regenerateDestroy ?? $this->sessionRegenerateDestroy; - } else { - // `Config/Session.php` is absence - $this->sessionDriverName = $config->sessionDriver; - $this->sessionCookieName = $config->sessionCookieName ?? $this->sessionCookieName; - $this->sessionExpiration = $config->sessionExpiration ?? $this->sessionExpiration; - $this->sessionSavePath = $config->sessionSavePath; - $this->sessionMatchIP = $config->sessionMatchIP ?? $this->sessionMatchIP; - $this->sessionTimeToUpdate = $config->sessionTimeToUpdate ?? $this->sessionTimeToUpdate; - $this->sessionRegenerateDestroy = $config->sessionRegenerateDestroy ?? $this->sessionRegenerateDestroy; - } + $this->sessionDriverName = $session->driver; + $this->sessionCookieName = $session->cookieName ?? $this->sessionCookieName; + $this->sessionExpiration = $session->expiration ?? $this->sessionExpiration; + $this->sessionSavePath = $session->savePath; + $this->sessionMatchIP = $session->matchIP ?? $this->sessionMatchIP; + $this->sessionTimeToUpdate = $session->timeToUpdate ?? $this->sessionTimeToUpdate; + $this->sessionRegenerateDestroy = $session->regenerateDestroy ?? $this->sessionRegenerateDestroy; /** @var CookieConfig $cookie */ $cookie = config('Cookie'); From 75fbf5650f0737e4c1a01fab9cf7e4a0f48e66af Mon Sep 17 00:00:00 2001 From: kenjis Date: Tue, 14 Feb 2023 20:04:09 +0900 Subject: [PATCH 2/9] fix: remove Config\App from constructors in Session and Session Handlers --- system/Config/Services.php | 14 ++++++------ system/Session/Handlers/BaseHandler.php | 12 ++++------ system/Session/Handlers/DatabaseHandler.php | 10 +++------ system/Session/Handlers/FileHandler.php | 4 ++-- system/Session/Handlers/MemcachedHandler.php | 10 +++------ system/Session/Handlers/RedisHandler.php | 12 ++++------ system/Session/Session.php | 7 ++---- system/Test/CIUnitTestCase.php | 2 +- tests/system/CommonFunctionsTest.php | 21 +++++++++--------- .../SecurityCSRFSessionRandomizeTokenTest.php | 22 +++++++++---------- .../Security/SecurityCSRFSessionTest.php | 22 +++++++++---------- tests/system/Session/SessionTest.php | 2 +- 12 files changed, 60 insertions(+), 78 deletions(-) diff --git a/system/Config/Services.php b/system/Config/Services.php index 1640c4f04bd8..fafeb3ade954 100644 --- a/system/Config/Services.php +++ b/system/Config/Services.php @@ -638,6 +638,8 @@ public static function security(?App $config = null, bool $getShared = true) * Return the session manager. * * @return Session + * + * @TODO replace the first parameter type `?App` with `?SessionConfig` */ public static function session(?App $config = null, bool $getShared = true) { @@ -645,18 +647,16 @@ public static function session(?App $config = null, bool $getShared = true) return static::getSharedInstance('session', $config); } - $config ??= config('App'); - assert($config instanceof App); - $logger = AppServices::logger(); - /** @var SessionConfig|null $sessionConfig */ - $sessionConfig = config('Session'); + /** @var SessionConfig $config */ + $config = config('Session'); + assert($config instanceof SessionConfig, 'Missing "Config/Session.php".'); - $driverName = $sessionConfig->driver; + $driverName = $config->driver; if ($driverName === DatabaseHandler::class) { - $DBGroup = $sessionConfig->DBGroup ?? config(Database::class)->defaultGroup; + $DBGroup = $config->DBGroup ?? config(Database::class)->defaultGroup; $db = Database::connect($DBGroup); $driver = $db->getPlatform(); diff --git a/system/Session/Handlers/BaseHandler.php b/system/Session/Handlers/BaseHandler.php index 00cf1413139c..df62f72303ba 100644 --- a/system/Session/Handlers/BaseHandler.php +++ b/system/Session/Handlers/BaseHandler.php @@ -11,7 +11,6 @@ namespace CodeIgniter\Session\Handlers; -use Config\App as AppConfig; use Config\Cookie as CookieConfig; use Config\Session as SessionConfig; use Psr\Log\LoggerAwareTrait; @@ -105,15 +104,12 @@ abstract class BaseHandler implements SessionHandlerInterface */ protected $ipAddress; - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $config, string $ipAddress) { - /** @var SessionConfig|null $session */ - $session = config('Session'); - // Store Session configurations - $this->cookieName = $session->cookieName; - $this->matchIP = $session->matchIP; - $this->savePath = $session->savePath; + $this->cookieName = $config->cookieName; + $this->matchIP = $config->matchIP; + $this->savePath = $config->savePath; /** @var CookieConfig $cookie */ $cookie = config('Cookie'); diff --git a/system/Session/Handlers/DatabaseHandler.php b/system/Session/Handlers/DatabaseHandler.php index 2cc8917b5f27..edde55ca34c6 100644 --- a/system/Session/Handlers/DatabaseHandler.php +++ b/system/Session/Handlers/DatabaseHandler.php @@ -14,7 +14,6 @@ use CodeIgniter\Database\BaseBuilder; use CodeIgniter\Database\BaseConnection; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; use Config\Database; use Config\Session as SessionConfig; use ReturnTypeWillChange; @@ -69,17 +68,14 @@ class DatabaseHandler extends BaseHandler /** * @throws SessionException */ - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $config, string $ipAddress) { parent::__construct($config, $ipAddress); - /** @var SessionConfig|null $session */ - $session = config('Session'); - // Store Session configurations - $this->DBGroup = $session->DBGroup ?? config(Database::class)->defaultGroup; + $this->DBGroup = $config->DBGroup ?? config(Database::class)->defaultGroup; // Add sessionCookieName for multiple session cookies. - $this->idPrefix = $session->cookieName . ':'; + $this->idPrefix = $config->cookieName . ':'; $this->table = $this->savePath; if (empty($this->table)) { diff --git a/system/Session/Handlers/FileHandler.php b/system/Session/Handlers/FileHandler.php index cc8a6694f692..9eb642408381 100644 --- a/system/Session/Handlers/FileHandler.php +++ b/system/Session/Handlers/FileHandler.php @@ -13,7 +13,7 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; +use Config\Session as SessionConfig; use ReturnTypeWillChange; /** @@ -63,7 +63,7 @@ class FileHandler extends BaseHandler */ protected $sessionIDRegex = ''; - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $config, string $ipAddress) { parent::__construct($config, $ipAddress); diff --git a/system/Session/Handlers/MemcachedHandler.php b/system/Session/Handlers/MemcachedHandler.php index f7d6210a9293..abccfafb5ba4 100644 --- a/system/Session/Handlers/MemcachedHandler.php +++ b/system/Session/Handlers/MemcachedHandler.php @@ -13,7 +13,6 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; use Config\Session as SessionConfig; use Memcached; use ReturnTypeWillChange; @@ -54,21 +53,18 @@ class MemcachedHandler extends BaseHandler /** * @throws SessionException */ - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $config, string $ipAddress) { parent::__construct($config, $ipAddress); - /** @var SessionConfig $session */ - $session = config('Session'); - - $this->sessionExpiration = $session->expiration; + $this->sessionExpiration = $config->expiration; if (empty($this->savePath)) { throw SessionException::forEmptySavepath(); } // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= $session->cookieName . ':'; + $this->keyPrefix .= $config->cookieName . ':'; if ($this->matchIP === true) { $this->keyPrefix .= $this->ipAddress . ':'; diff --git a/system/Session/Handlers/RedisHandler.php b/system/Session/Handlers/RedisHandler.php index d2cdb0337eae..f5360e96c48a 100644 --- a/system/Session/Handlers/RedisHandler.php +++ b/system/Session/Handlers/RedisHandler.php @@ -13,7 +13,6 @@ use CodeIgniter\I18n\Time; use CodeIgniter\Session\Exceptions\SessionException; -use Config\App as AppConfig; use Config\Session as SessionConfig; use Redis; use RedisException; @@ -67,19 +66,16 @@ class RedisHandler extends BaseHandler * * @throws SessionException */ - public function __construct(AppConfig $config, string $ipAddress) + public function __construct(SessionConfig $config, string $ipAddress) { parent::__construct($config, $ipAddress); - /** @var SessionConfig|null $session */ - $session = config('Session'); - // Store Session configurations - $this->sessionExpiration = empty($session->expiration) + $this->sessionExpiration = empty($config->expiration) ? (int) ini_get('session.gc_maxlifetime') - : (int) $session->expiration; + : (int) $config->expiration; // Add sessionCookieName for multiple session cookies. - $this->keyPrefix .= $session->cookieName . ':'; + $this->keyPrefix .= $config->cookieName . ':'; $this->setSavePath(); diff --git a/system/Session/Session.php b/system/Session/Session.php index e4b4a5fb2519..6d77ac3e82c7 100644 --- a/system/Session/Session.php +++ b/system/Session/Session.php @@ -62,7 +62,7 @@ class Session implements SessionInterface protected $sessionExpiration = 7200; /** - * The location to save sessions to, driver dependent.. + * The location to save sessions to, driver dependent. * * For the 'files' driver, it's a path to a writable directory. * WARNING: Only absolute paths are supported! @@ -161,13 +161,10 @@ class Session implements SessionInterface * * Extract configuration settings and save them here. */ - public function __construct(SessionHandlerInterface $driver, App $config) + public function __construct(SessionHandlerInterface $driver, SessionConfig $session) { $this->driver = $driver; - /** @var SessionConfig|null $session */ - $session = config('Session'); - // Store Session configurations $this->sessionDriverName = $session->driver; $this->sessionCookieName = $session->cookieName ?? $this->sessionCookieName; diff --git a/system/Test/CIUnitTestCase.php b/system/Test/CIUnitTestCase.php index bfde56faf6ff..32d782f69ec2 100644 --- a/system/Test/CIUnitTestCase.php +++ b/system/Test/CIUnitTestCase.php @@ -333,7 +333,7 @@ protected function mockSession() { $_SESSION = []; - $config = config('App'); + $config = config('Session'); $session = new MockSession(new ArrayHandler($config, '0.0.0.0'), $config); Services::injectMock('session', $session); diff --git a/tests/system/CommonFunctionsTest.php b/tests/system/CommonFunctionsTest.php index a40e4cd8d438..4c174ecd12f4 100644 --- a/tests/system/CommonFunctionsTest.php +++ b/tests/system/CommonFunctionsTest.php @@ -34,6 +34,7 @@ use Config\Modules; use Config\Routing; use Config\Services; +use Config\Session as SessionConfig; use Kint; use RuntimeException; use stdClass; @@ -517,20 +518,20 @@ public function testSlashItemThrowsErrorOnNonStringableItem() protected function injectSessionMock() { - $appConfig = new App(); + $sessionConfig = new SessionConfig(); $defaults = [ - 'sessionDriver' => FileHandler::class, - 'sessionCookieName' => 'ci_session', - 'sessionExpiration' => 7200, - 'sessionSavePath' => '', - 'sessionMatchIP' => false, - 'sessionTimeToUpdate' => 300, - 'sessionRegenerateDestroy' => false, + 'driver' => FileHandler::class, + 'cookieName' => 'ci_session', + 'expiration' => 7200, + 'savePath' => '', + 'matchIP' => false, + 'timeToUpdate' => 300, + 'regenerateDestroy' => false, ]; foreach ($defaults as $key => $config) { - $appConfig->{$key} = $config; + $sessionConfig->{$key} = $config; } $cookie = new Cookie(); @@ -546,7 +547,7 @@ protected function injectSessionMock() } Factories::injectMock('config', 'Cookie', $cookie); - $session = new MockSession(new FileHandler($appConfig, '127.0.0.1'), $appConfig); + $session = new MockSession(new FileHandler($sessionConfig, '127.0.0.1'), $sessionConfig); $session->setLogger(new TestLogger(new Logger())); BaseService::injectMock('session', $session); } diff --git a/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php b/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php index f8f3fc01fbaa..36bd5ea97f1c 100644 --- a/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php +++ b/tests/system/Security/SecurityCSRFSessionRandomizeTokenTest.php @@ -25,10 +25,10 @@ use CodeIgniter\Test\Mock\MockSecurity; use CodeIgniter\Test\Mock\MockSession; use CodeIgniter\Test\TestLogger; -use Config\App as AppConfig; use Config\Cookie; use Config\Logger as LoggerConfig; use Config\Security as SecurityConfig; +use Config\Session as SessionConfig; /** * @runTestsInSeparateProcesses @@ -69,20 +69,20 @@ protected function setUp(): void private function createSession($options = []): Session { $defaults = [ - 'sessionDriver' => FileHandler::class, - 'sessionCookieName' => 'ci_session', - 'sessionExpiration' => 7200, - 'sessionSavePath' => '', - 'sessionMatchIP' => false, - 'sessionTimeToUpdate' => 300, - 'sessionRegenerateDestroy' => false, + 'driver' => FileHandler::class, + 'cookieName' => 'ci_session', + 'expiration' => 7200, + 'savePath' => '', + 'matchIP' => false, + 'timeToUpdate' => 300, + 'regenerateDestroy' => false, ]; $config = array_merge($defaults, $options); - $appConfig = new AppConfig(); + $sessionConfig = new SessionConfig(); foreach ($config as $key => $c) { - $appConfig->{$key} = $c; + $sessionConfig->{$key} = $c; } $cookie = new Cookie(); @@ -98,7 +98,7 @@ private function createSession($options = []): Session } Factories::injectMock('config', 'Cookie', $cookie); - $session = new MockSession(new ArrayHandler($appConfig, '127.0.0.1'), $appConfig); + $session = new MockSession(new ArrayHandler($sessionConfig, '127.0.0.1'), $sessionConfig); $session->setLogger(new TestLogger(new LoggerConfig())); return $session; diff --git a/tests/system/Security/SecurityCSRFSessionTest.php b/tests/system/Security/SecurityCSRFSessionTest.php index 495f9e58b06d..d6f2216e29f6 100644 --- a/tests/system/Security/SecurityCSRFSessionTest.php +++ b/tests/system/Security/SecurityCSRFSessionTest.php @@ -24,10 +24,10 @@ use CodeIgniter\Test\Mock\MockAppConfig; use CodeIgniter\Test\Mock\MockSession; use CodeIgniter\Test\TestLogger; -use Config\App as AppConfig; use Config\Cookie; use Config\Logger as LoggerConfig; use Config\Security as SecurityConfig; +use Config\Session as SessionConfig; /** * @runTestsInSeparateProcesses @@ -62,20 +62,20 @@ protected function setUp(): void private function createSession($options = []): Session { $defaults = [ - 'sessionDriver' => FileHandler::class, - 'sessionCookieName' => 'ci_session', - 'sessionExpiration' => 7200, - 'sessionSavePath' => '', - 'sessionMatchIP' => false, - 'sessionTimeToUpdate' => 300, - 'sessionRegenerateDestroy' => false, + 'driver' => FileHandler::class, + 'cookieName' => 'ci_session', + 'expiration' => 7200, + 'savePath' => '', + 'matchIP' => false, + 'timeToUpdate' => 300, + 'regenerateDestroy' => false, ]; $config = array_merge($defaults, $options); - $appConfig = new AppConfig(); + $sessionConfig = new SessionConfig(); foreach ($config as $key => $c) { - $appConfig->{$key} = $c; + $sessionConfig->{$key} = $c; } $cookie = new Cookie(); @@ -91,7 +91,7 @@ private function createSession($options = []): Session } Factories::injectMock('config', 'Cookie', $cookie); - $session = new MockSession(new ArrayHandler($appConfig, '127.0.0.1'), $appConfig); + $session = new MockSession(new ArrayHandler($sessionConfig, '127.0.0.1'), $sessionConfig); $session->setLogger(new TestLogger(new LoggerConfig())); return $session; diff --git a/tests/system/Session/SessionTest.php b/tests/system/Session/SessionTest.php index 8e0170d0fee1..de093e18a8e1 100644 --- a/tests/system/Session/SessionTest.php +++ b/tests/system/Session/SessionTest.php @@ -62,7 +62,7 @@ protected function getInstance($options = []) } Factories::injectMock('config', 'Session', $sessionConfig); - $session = new MockSession(new FileHandler($appConfig, '127.0.0.1'), $appConfig); + $session = new MockSession(new FileHandler($sessionConfig, '127.0.0.1'), $sessionConfig); $session->setLogger(new TestLogger(new LoggerConfig())); return $session; From 01e1e51c28cf11b9424b7ebdf2b7359db1ab1328 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 15 Feb 2023 10:34:18 +0900 Subject: [PATCH 3/9] refactor: add property for SessionConfig and use it --- system/Session/Session.php | 70 +++++++++++++++++--------------- system/Test/Mock/MockSession.php | 2 +- 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/system/Session/Session.php b/system/Session/Session.php index 6d77ac3e82c7..8c5a9d96ecbc 100644 --- a/system/Session/Session.php +++ b/system/Session/Session.php @@ -43,6 +43,8 @@ class Session implements SessionInterface * The storage driver to use: files, database, redis, memcached * * @var string + * + * @deprecated Use $this->config->driver. */ protected $sessionDriverName; @@ -50,6 +52,8 @@ class Session implements SessionInterface * The session cookie name, must contain only [0-9a-z_-] characters. * * @var string + * + * @deprecated Use $this->config->cookieName. */ protected $sessionCookieName = 'ci_session'; @@ -58,6 +62,8 @@ class Session implements SessionInterface * Setting it to 0 (zero) means expire when the browser is closed. * * @var int + * + * @deprecated Use $this->config->expiration. */ protected $sessionExpiration = 7200; @@ -74,6 +80,8 @@ class Session implements SessionInterface * IMPORTANT: You are REQUIRED to set a valid save path! * * @var string + * + * @deprecated Use $this->config->savePath. */ protected $sessionSavePath; @@ -84,6 +92,8 @@ class Session implements SessionInterface * your session table's PRIMARY KEY when changing this setting. * * @var bool + * + * @deprecated Use $this->config->matchIP. */ protected $sessionMatchIP = false; @@ -91,6 +101,8 @@ class Session implements SessionInterface * How many seconds between CI regenerating the session ID. * * @var int + * + * @deprecated Use $this->config->timeToUpdate. */ protected $sessionTimeToUpdate = 300; @@ -100,6 +112,8 @@ class Session implements SessionInterface * will be later deleted by the garbage collector. * * @var bool + * + * @deprecated Use $this->config->regenerateDestroy. */ protected $sessionRegenerateDestroy = false; @@ -156,6 +170,11 @@ class Session implements SessionInterface */ protected $sidRegexp; + /** + * Session Config + */ + protected SessionConfig $config; + /** * Constructor. * @@ -165,20 +184,13 @@ public function __construct(SessionHandlerInterface $driver, SessionConfig $sess { $this->driver = $driver; - // Store Session configurations - $this->sessionDriverName = $session->driver; - $this->sessionCookieName = $session->cookieName ?? $this->sessionCookieName; - $this->sessionExpiration = $session->expiration ?? $this->sessionExpiration; - $this->sessionSavePath = $session->savePath; - $this->sessionMatchIP = $session->matchIP ?? $this->sessionMatchIP; - $this->sessionTimeToUpdate = $session->timeToUpdate ?? $this->sessionTimeToUpdate; - $this->sessionRegenerateDestroy = $session->regenerateDestroy ?? $this->sessionRegenerateDestroy; + $this->config = $session; /** @var CookieConfig $cookie */ $cookie = config('Cookie'); - $this->cookie = (new Cookie($this->sessionCookieName, '', [ - 'expires' => $this->sessionExpiration === 0 ? 0 : Time::now()->getTimestamp() + $this->sessionExpiration, + $this->cookie = (new Cookie($this->config->cookieName, '', [ + 'expires' => $this->config->expiration === 0 ? 0 : Time::now()->getTimestamp() + $this->config->expiration, 'path' => $cookie->path, 'domain' => $cookie->domain, 'secure' => $cookie->secure, @@ -221,32 +233,32 @@ public function start() $this->setSaveHandler(); // Sanitize the cookie, because apparently PHP doesn't do that for userspace handlers - if (isset($_COOKIE[$this->sessionCookieName]) - && (! is_string($_COOKIE[$this->sessionCookieName]) || ! preg_match('#\A' . $this->sidRegexp . '\z#', $_COOKIE[$this->sessionCookieName])) + if (isset($_COOKIE[$this->config->cookieName]) + && (! is_string($_COOKIE[$this->config->cookieName]) || ! preg_match('#\A' . $this->sidRegexp . '\z#', $_COOKIE[$this->config->cookieName])) ) { - unset($_COOKIE[$this->sessionCookieName]); + unset($_COOKIE[$this->config->cookieName]); } $this->startSession(); // Is session ID auto-regeneration configured? (ignoring ajax requests) if ((empty($_SERVER['HTTP_X_REQUESTED_WITH']) || strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) !== 'xmlhttprequest') - && ($regenerateTime = $this->sessionTimeToUpdate) > 0 + && ($regenerateTime = $this->config->timeToUpdate) > 0 ) { if (! isset($_SESSION['__ci_last_regenerate'])) { $_SESSION['__ci_last_regenerate'] = Time::now()->getTimestamp(); } elseif ($_SESSION['__ci_last_regenerate'] < (Time::now()->getTimestamp() - $regenerateTime)) { - $this->regenerate((bool) $this->sessionRegenerateDestroy); + $this->regenerate((bool) $this->config->regenerateDestroy); } } // Another work-around ... PHP doesn't seem to send the session cookie // unless it is being currently created or regenerated - elseif (isset($_COOKIE[$this->sessionCookieName]) && $_COOKIE[$this->sessionCookieName] === session_id()) { + elseif (isset($_COOKIE[$this->config->cookieName]) && $_COOKIE[$this->config->cookieName] === session_id()) { $this->setCookie(); } $this->initVars(); - $this->logger->info("Session: Class initialized using '" . $this->sessionDriverName . "' driver."); + $this->logger->info("Session: Class initialized using '" . $this->config->driver . "' driver."); return $this; } @@ -268,16 +280,12 @@ public function stop() */ protected function configure() { - if (empty($this->sessionCookieName)) { - $this->sessionCookieName = ini_get('session.name'); - } else { - ini_set('session.name', $this->sessionCookieName); - } + ini_set('session.name', $this->config->cookieName); $sameSite = $this->cookie->getSameSite() ?: ucfirst(Cookie::SAMESITE_LAX); $params = [ - 'lifetime' => $this->sessionExpiration, + 'lifetime' => $this->config->expiration, 'path' => $this->cookie->getPath(), 'domain' => $this->cookie->getDomain(), 'secure' => $this->cookie->isSecure(), @@ -288,14 +296,12 @@ protected function configure() ini_set('session.cookie_samesite', $sameSite); session_set_cookie_params($params); - if (! isset($this->sessionExpiration)) { - $this->sessionExpiration = (int) ini_get('session.gc_maxlifetime'); - } elseif ($this->sessionExpiration > 0) { - ini_set('session.gc_maxlifetime', (string) $this->sessionExpiration); + if ($this->config->expiration > 0) { + ini_set('session.gc_maxlifetime', (string) $this->config->expiration); } - if (! empty($this->sessionSavePath)) { - ini_set('session.save_path', $this->sessionSavePath); + if (! empty($this->config->savePath)) { + ini_set('session.save_path', $this->config->savePath); } // Security is king @@ -402,12 +408,12 @@ private function removeOldSessionCookie(): void $response = Services::response(); $cookieStoreInResponse = $response->getCookieStore(); - if (! $cookieStoreInResponse->has($this->sessionCookieName)) { + if (! $cookieStoreInResponse->has($this->config->cookieName)) { return; } // CookieStore is immutable. - $newCookieStore = $cookieStoreInResponse->remove($this->sessionCookieName); + $newCookieStore = $cookieStoreInResponse->remove($this->config->cookieName); // But clear() method clears cookies in the object (not immutable). $cookieStoreInResponse->clear(); @@ -921,7 +927,7 @@ protected function startSession() */ protected function setCookie() { - $expiration = $this->sessionExpiration === 0 ? 0 : Time::now()->getTimestamp() + $this->sessionExpiration; + $expiration = $this->config->expiration === 0 ? 0 : Time::now()->getTimestamp() + $this->config->expiration; $this->cookie = $this->cookie->withValue(session_id())->withExpires($expiration); $response = Services::response(); diff --git a/system/Test/Mock/MockSession.php b/system/Test/Mock/MockSession.php index f5290f26525d..9f558e1034ad 100644 --- a/system/Test/Mock/MockSession.php +++ b/system/Test/Mock/MockSession.php @@ -57,7 +57,7 @@ protected function startSession() */ protected function setCookie() { - $expiration = $this->sessionExpiration === 0 ? 0 : Time::now()->getTimestamp() + $this->sessionExpiration; + $expiration = $this->config->expiration === 0 ? 0 : Time::now()->getTimestamp() + $this->config->expiration; $this->cookie = $this->cookie->withValue(session_id())->withExpires($expiration); $this->cookies[] = $this->cookie; From 6a80edddeca1e01ca384c79afee952888443ee0c Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 15 Feb 2023 10:43:27 +0900 Subject: [PATCH 4/9] test: remove unneeded variable --- tests/system/Session/SessionTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/system/Session/SessionTest.php b/tests/system/Session/SessionTest.php index de093e18a8e1..921f9b80568d 100644 --- a/tests/system/Session/SessionTest.php +++ b/tests/system/Session/SessionTest.php @@ -17,7 +17,6 @@ use CodeIgniter\Test\CIUnitTestCase; use CodeIgniter\Test\Mock\MockSession; use CodeIgniter\Test\TestLogger; -use Config\App as AppConfig; use Config\Cookie as CookieConfig; use Config\Logger as LoggerConfig; use Config\Session as SessionConfig; @@ -43,8 +42,6 @@ protected function setUp(): void protected function getInstance($options = []) { - $appConfig = new AppConfig(); - $defaults = [ 'driver' => FileHandler::class, 'cookieName' => 'ci_session', From 9db8130f94caa5a93c027e80514acf443367462a Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 15 Feb 2023 10:59:21 +0900 Subject: [PATCH 5/9] docs: add changelog and upgrading guide --- user_guide_src/source/changelogs/v4.4.0.rst | 10 ++++++++++ user_guide_src/source/installation/upgrade_440.rst | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index dc1630e05a8f..a90187940cf1 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -47,6 +47,11 @@ Method Signature Changes ``RouteCollection::__construct()``. - **Validation:** The method signature of ``Validation::check()`` has been changed. The ``string`` typehint on the ``$rule`` parameter was removed. +- **Session:** The second parameter of ``Session::__construct()`` has been + changed from ``Config\App`` to ``Config\Session``. +- **Session:** The first parameter of ``__construct()`` in ``BaseHandler``, + ``DatabaseHandler``, ``FileHandler``, ``MemcachedHandler``, and ``RedisHandler`` + has been changed from ``Config\App`` to ``Config\Session``. Enhancements ************ @@ -127,6 +132,7 @@ Changes - **Images:** The default quality for WebP in ``GDHandler`` has been changed from 80 to 90. - **Config:** The deprecated Cookie items in **app/Config/App.php** has been removed. +- **Config:** The deprecated Session items in **app/Config/App.php** has been removed. - **Config:** Routing settings have been moved to **app/Config/Routing.php** config file. See :ref:`Upgrading Guide `. - **DownloadResponse:** When generating response headers, does not replace the ``Content-Disposition`` header if it was previously specified. @@ -147,6 +153,10 @@ Deprecations - **Autoloader:** ``Autoloader::sanitizeFilename()`` is deprecated. - **CodeIgniter:** ``CodeIgniter::$returnResponse`` property is deprecated. No longer used. - **RedirectException:** ``\CodeIgniter\Router\Exceptions\RedirectException`` is deprecated. Use \CodeIgniter\HTTP\Exceptions\RedirectException instead. +- **Session:** The property ``$sessionDriverName``, ``$sessionCookieName``, + ``$sessionExpiration``, ``$sessionSavePath``, ``$sessionMatchIP``, + ``$sessionTimeToUpdate``, and ``$sessionRegenerateDestroy`` in ``Session`` are + deprecated, and no longer used. Use ``$config`` instead. Bugs Fixed ********** diff --git a/user_guide_src/source/installation/upgrade_440.rst b/user_guide_src/source/installation/upgrade_440.rst index 5e7ab6856ce9..9c765f24434b 100644 --- a/user_guide_src/source/installation/upgrade_440.rst +++ b/user_guide_src/source/installation/upgrade_440.rst @@ -100,6 +100,16 @@ The Cookie config items in **app/Config/App.php** are no longer used. 2. Remove the properties (from ``$cookiePrefix`` to ``$cookieSameSite``) in **app/Config/App.php**. +app/Config/Session.php +---------------------- + +The Session config items in **app/Config/App.php** are no longer used. + +1. Copy **app/Config/Session.php** from the new framework to your **app/Config** + directory, and configure it. +2. Remove the properties (from ``$sessionDriver`` to ``$sessionDBGroup``) in + **app/Config/App.php**. + Breaking Enhancements ********************* From 3a84ef4f36e01a217f5d669dc9d9737532f1883b Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 15 Feb 2023 11:02:59 +0900 Subject: [PATCH 6/9] test: update parameter --- tests/system/Session/Handlers/Database/MySQLiHandlerTest.php | 5 +---- .../system/Session/Handlers/Database/PostgreHandlerTest.php | 5 +---- tests/system/Session/Handlers/Database/RedisHandlerTest.php | 5 +---- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php b/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php index b18f1b047eef..19266f4462b5 100644 --- a/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php +++ b/tests/system/Session/Handlers/Database/MySQLiHandlerTest.php @@ -11,8 +11,6 @@ namespace CodeIgniter\Session\Handlers\Database; -use CodeIgniter\Config\Factories; -use Config\App as AppConfig; use Config\Database as DatabaseConfig; use Config\Session as SessionConfig; @@ -49,8 +47,7 @@ protected function getInstance($options = []) foreach ($config as $key => $value) { $sessionConfig->{$key} = $value; } - Factories::injectMock('config', 'Session', $sessionConfig); - return new MySQLiHandler(new AppConfig(), $this->userIpAddress); + return new MySQLiHandler($sessionConfig, $this->userIpAddress); } } diff --git a/tests/system/Session/Handlers/Database/PostgreHandlerTest.php b/tests/system/Session/Handlers/Database/PostgreHandlerTest.php index f9db847faab0..3e5fe5a3b10b 100644 --- a/tests/system/Session/Handlers/Database/PostgreHandlerTest.php +++ b/tests/system/Session/Handlers/Database/PostgreHandlerTest.php @@ -11,8 +11,6 @@ namespace CodeIgniter\Session\Handlers\Database; -use CodeIgniter\Config\Factories; -use Config\App as AppConfig; use Config\Database as DatabaseConfig; use Config\Session as SessionConfig; @@ -49,8 +47,7 @@ protected function getInstance($options = []) foreach ($config as $key => $value) { $sessionConfig->{$key} = $value; } - Factories::injectMock('config', 'Session', $sessionConfig); - return new PostgreHandler(new AppConfig(), $this->userIpAddress); + return new PostgreHandler($sessionConfig, $this->userIpAddress); } } diff --git a/tests/system/Session/Handlers/Database/RedisHandlerTest.php b/tests/system/Session/Handlers/Database/RedisHandlerTest.php index b5d67a768c47..893f249ee22c 100644 --- a/tests/system/Session/Handlers/Database/RedisHandlerTest.php +++ b/tests/system/Session/Handlers/Database/RedisHandlerTest.php @@ -11,10 +11,8 @@ namespace CodeIgniter\Session\Handlers\Database; -use CodeIgniter\Config\Factories; use CodeIgniter\Session\Handlers\RedisHandler; use CodeIgniter\Test\CIUnitTestCase; -use Config\App as AppConfig; use Config\Session as SessionConfig; use Redis; @@ -49,9 +47,8 @@ protected function getInstance($options = []) foreach ($config as $key => $value) { $sessionConfig->{$key} = $value; } - Factories::injectMock('config', 'Session', $sessionConfig); - return new RedisHandler(new AppConfig(), $this->userIpAddress); + return new RedisHandler($sessionConfig, $this->userIpAddress); } public function testSavePathWithoutProtocol() From 33829746efd96b2b51dc9307ff643569b3f14836 Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 15 Feb 2023 11:07:32 +0900 Subject: [PATCH 7/9] refactor: revert parameter variable name --- system/Session/Session.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/Session/Session.php b/system/Session/Session.php index 8c5a9d96ecbc..4d139c18aa02 100644 --- a/system/Session/Session.php +++ b/system/Session/Session.php @@ -180,11 +180,11 @@ class Session implements SessionInterface * * Extract configuration settings and save them here. */ - public function __construct(SessionHandlerInterface $driver, SessionConfig $session) + public function __construct(SessionHandlerInterface $driver, SessionConfig $config) { $this->driver = $driver; - $this->config = $session; + $this->config = $config; /** @var CookieConfig $cookie */ $cookie = config('Cookie'); From 24fc2553d582efb5ac40e7c0124c09240b23855e Mon Sep 17 00:00:00 2001 From: kenjis Date: Wed, 15 Feb 2023 11:47:14 +0900 Subject: [PATCH 8/9] test: update out of dated test --- tests/system/CommonFunctionsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/CommonFunctionsTest.php b/tests/system/CommonFunctionsTest.php index 4c174ecd12f4..884b2819f912 100644 --- a/tests/system/CommonFunctionsTest.php +++ b/tests/system/CommonFunctionsTest.php @@ -496,7 +496,7 @@ public function testReallyWritable() public function testSlashItem() { $this->assertSame('en/', slash_item('defaultLocale')); // en - $this->assertSame('7200/', slash_item('sessionExpiration')); // int 7200 + $this->assertSame('7200/', slash_item('CSRFExpire')); // int 7200 $this->assertSame('', slash_item('negotiateLocale')); // false } From 7dda3b12e0ce69256c8d4342a6e1025e58c4f636 Mon Sep 17 00:00:00 2001 From: kenjis Date: Fri, 24 Feb 2023 11:29:05 +0900 Subject: [PATCH 9/9] chore: remove PHPStan ignored error pattern --- phpstan-baseline.neon.dist | 5 ----- 1 file changed, 5 deletions(-) diff --git a/phpstan-baseline.neon.dist b/phpstan-baseline.neon.dist index a32d4fc11a79..2d60b2b599d2 100644 --- a/phpstan-baseline.neon.dist +++ b/phpstan-baseline.neon.dist @@ -230,11 +230,6 @@ parameters: count: 1 path: system/Router/Router.php - - - message: "#^Property CodeIgniter\\\\Session\\\\Session\\:\\:\\$sessionExpiration \\(int\\) in isset\\(\\) is not nullable\\.$#" - count: 1 - path: system/Session/Session.php - - message: "#^Access to an undefined property object\\:\\:\\$createdField\\.$#" count: 1