Browse Source

refine timeout and gclifetime code

Instead of special casing 0 with a hardcoded default, we can leave the
gc_maxlifetime alone and only define it when a timeout has been set.
Mark Story 2 years ago
parent
commit
ba64677b7f
2 changed files with 32 additions and 12 deletions
  1. 9 10
      src/Http/Session.php
  2. 23 2
      tests/TestCase/Http/SessionTest.php

+ 9 - 10
src/Http/Session.php

@@ -200,7 +200,8 @@ class Session
      *
      * ### Configuration:
      *
-     * - timeout: The time in minutes the session should be valid for.
+     * - timeout: The time in minutes that a session can be idle and remain valid.
+     *  If set to 0, no server side timeout will be applied.
      * - cookiePath: The url path for which session cookie is set. Maps to the
      *   `session.cookie_path` php.ini config. Defaults to base path of app.
      * - ini: A list of php.ini directives to change before the session start.
@@ -220,14 +221,12 @@ class Session
             'handler' => [],
         ];
 
-        if (isset($config['timeout']) && !isset($config['ini']['session.gc_maxlifetime'])) {
-            $maxlifetime = $config['timeout'] * 60;
-            if ($maxlifetime === 0) {
-                // If sessions are set to have no idle timeout, extend the
-                // gc_maxlifetime to 30 days so that sessions don't get reaped immediately
-                $maxlifetime = 60 * 60 * 24 * 30;
-            }
-            $config['ini']['session.gc_maxlifetime'] = $maxlifetime;
+        $lifetime = 0;
+        if (isset($config['timeout'])) {
+            $lifetime = $config['timeout'] * 60;
+        }
+        if ($lifetime !== 0) {
+            $config['ini']['session.gc_maxlifetime'] = $lifetime;
         }
 
         if ($config['cookie']) {
@@ -247,7 +246,7 @@ class Session
             $this->engine($class, $config['handler']);
         }
 
-        $this->_lifetime = (int)ini_get('session.gc_maxlifetime');
+        $this->_lifetime = $lifetime;
         $this->_isCLI = (PHP_SAPI === 'cli' || PHP_SAPI === 'phpdbg');
         session_register_shutdown();
     }

+ 23 - 2
tests/TestCase/Http/SessionTest.php

@@ -70,17 +70,38 @@ class SessionTest extends TestCase
      * @preserveGlobalState disabled
      * @runInSeparateProcess
      */
-    public function testSessionConfigTimeout(): void
+    public function testSessionConfigTimeoutZero(): void
     {
         $_SESSION = null;
 
+        ini_set('session.gc_maxlifetime', 86400);
         $config = [
             'defaults' => 'php',
             'timeout' => 0,
         ];
 
         Session::create($config);
-        $this->assertEquals(60 * 60 * 24 * 30, ini_get('session.gc_maxlifetime'), 'Ini value is incorrect');
+        $this->assertEquals(86400, ini_get('session.gc_maxlifetime'), 'ini value unchanged when timeout disabled');
+    }
+
+    /**
+     * test setting ini properties with Session configuration.
+     *
+     * @preserveGlobalState disabled
+     * @runInSeparateProcess
+     */
+    public function testSessionConfigTimeout(): void
+    {
+        $_SESSION = null;
+
+        ini_set('session.gc_maxlifetime', 86400);
+        $config = [
+            'defaults' => 'php',
+            'timeout' => 30,
+        ];
+
+        Session::create($config);
+        $this->assertEquals(30 * 60, ini_get('session.gc_maxlifetime'), 'timeout should set gc maxlifetime');
     }
 
     /**