#3073 (feature/session): Fix session_regenerate_id() issues
diff --git a/system/libraries/Session/drivers/Session_database_driver.php b/system/libraries/Session/drivers/Session_database_driver.php
index e3a3c50..9e74605 100644
--- a/system/libraries/Session/drivers/Session_database_driver.php
+++ b/system/libraries/Session/drivers/Session_database_driver.php
@@ -111,6 +111,9 @@
{
if ($this->_get_lock($session_id) !== FALSE)
{
+ // Needed by write() to detect session_regenerate_id() calls
+ $this->_session_id = $session_id;
+
$this->_db
->select('data')
->from($this->_config['save_path'])
@@ -142,6 +145,17 @@
{
return FALSE;
}
+ // Was the ID regenerated?
+ elseif ($session_id !== $this->_session_id)
+ {
+ if ( ! $this->_release_lock() OR ! $this->_get_lock($session_id))
+ {
+ return FALSE;
+ }
+
+ $this->_row_exists = FALSE;
+ $this->_session_id = $session_id;
+ }
if ($this->_row_exists === FALSE)
{
diff --git a/system/libraries/Session/drivers/Session_files_driver.php b/system/libraries/Session/drivers/Session_files_driver.php
index ff1553f..3d6fa63 100644
--- a/system/libraries/Session/drivers/Session_files_driver.php
+++ b/system/libraries/Session/drivers/Session_files_driver.php
@@ -114,36 +114,37 @@
// which re-reads session data
if ($this->_file_handle === NULL)
{
- $this->_file_path .= $session_id;
-
// Just using fopen() with 'c+b' mode would be perfect, but it is only
// available since PHP 5.2.6 and we have to set permissions for new files,
// so we'd have to hack around this ...
- if (($this->_file_new = ! file_exists($this->_file_path)) === TRUE)
+ if (($this->_file_new = ! file_exists($this->_file_path.$session_id)) === TRUE)
{
- if (($this->_file_handle = fopen($this->_file_path, 'w+b')) === FALSE)
+ if (($this->_file_handle = fopen($this->_file_path.$session_id, 'w+b')) === FALSE)
{
- log_message('error', "Session: File '".$this->_file_path."' doesn't exist and cannot be created.");
+ log_message('error', "Session: File '".$this->_file_path.$session_id."' doesn't exist and cannot be created.");
return FALSE;
}
}
- elseif (($this->_file_handle = fopen($this->_file_path, 'r+b')) === FALSE)
+ elseif (($this->_file_handle = fopen($this->_file_path.$session_id, 'r+b')) === FALSE)
{
- log_message('error', "Session: Unable to open file '".$this->_file_path."'.");
+ log_message('error', "Session: Unable to open file '".$this->_file_path.$session_id."'.");
return FALSE;
}
if (flock($this->_file_handle, LOCK_EX) === FALSE)
{
- log_message('error', "Session: Unable to obtain lock for file '".$this->_file_path."'.");
+ log_message('error', "Session: Unable to obtain lock for file '".$this->_file_path.$session_id."'.");
fclose($this->_file_handle);
$this->_file_handle = NULL;
return FALSE;
}
+ // Needed by write() to detect session_regenerate_id() calls
+ $this->_session_id = $session_id;
+
if ($this->_file_new)
{
- chmod($this->_file_path, 0600);
+ chmod($this->_file_path.$session_id, 0600);
$this->_fingerprint = md5('');
return '';
}
@@ -154,7 +155,7 @@
}
$session_data = '';
- for ($read = 0, $length = filesize($this->_file_path); $read < $length; $read += strlen($buffer))
+ for ($read = 0, $length = filesize($this->_file_path.$session_id); $read < $length; $read += strlen($buffer))
{
if (($buffer = fread($this->_file_handle, $length - $read)) === FALSE)
{
@@ -170,6 +171,13 @@
public function write($session_id, $session_data)
{
+ // If the two IDs don't match, we have a session_regenerate_id() call
+ // and we need to close the old handle and open a new one
+ if ($session_id !== $this->_session_id && ( ! $this->close() OR $this->read($session_id) === FALSE))
+ {
+ return FALSE;
+ }
+
if ( ! is_resource($this->_file_handle))
{
return FALSE;
@@ -178,7 +186,7 @@
{
return ($this->_file_new)
? TRUE
- : touch($this->_file_path);
+ : touch($this->_file_path.$session_id);
}
if ( ! $this->_file_new)
@@ -218,11 +226,11 @@
flock($this->_file_handle, LOCK_UN);
fclose($this->_file_handle);
- $this->_file_handle = $this->_file_new = NULL;
+ $this->_file_handle = $this->_file_new = $this->_session_id = NULL;
return TRUE;
}
- return FALSE;
+ return TRUE;
}
// ------------------------------------------------------------------------
@@ -231,13 +239,13 @@
{
if ($this->close())
{
- return unlink($this->_file_path) && $this->_cookie_destroy();
+ return unlink($this->_file_path.$session_id) && $this->_cookie_destroy();
}
elseif ($this->_file_path !== NULL)
{
clearstatcache();
- return file_exists($this->_file_path)
- ? (unlink($this->_file_path) && $this->_cookie_destroy())
+ return file_exists($this->_file_path.$session_id)
+ ? (unlink($this->_file_path.$session_id) && $this->_cookie_destroy())
: TRUE;
}
diff --git a/system/libraries/Session/drivers/Session_memcached_driver.php b/system/libraries/Session/drivers/Session_memcached_driver.php
index 318c11a..8905e8d 100644
--- a/system/libraries/Session/drivers/Session_memcached_driver.php
+++ b/system/libraries/Session/drivers/Session_memcached_driver.php
@@ -133,6 +133,9 @@
{
if (isset($this->_memcached) && $this->_get_lock($session_id))
{
+ // Needed by write() to detect session_regenerate_id() calls
+ $this->_session_id = $session_id;
+
$session_data = (string) $this->_memcached->get($this->_key_prefix.$session_id);
$this->_fingerprint = md5($session_data);
return $session_data;
@@ -143,7 +146,23 @@
public function write($session_id, $session_data)
{
- if (isset($this->_memcached, $this->_lock_key))
+ if ( ! isset($this->_memcached))
+ {
+ return FALSE;
+ }
+ // Was the ID regenerated?
+ elseif ($session_id !== $this->_session_id)
+ {
+ if ( ! $this->_release_lock() OR ! $this->_get_lock($session_id))
+ {
+ return FALSE;
+ }
+
+ $this->_fingerprint = md5('');
+ $this->_session_id = $session_id;
+ }
+
+ if (isset($this->_lock_key))
{
$this->_memcached->replace($this->_lock_key, time(), 5);
if ($this->_fingerprint !== ($fingerprint = md5($session_data)))
@@ -189,16 +208,17 @@
if (isset($this->_memcached, $this->_lock_key))
{
$this->_memcached->delete($this->_key_prefix.$session_id);
- return ($this->_cookie_destroy() && $this->close());
+ return $this->_cookie_destroy();
}
- return $this->close();
+ return FALSE;
}
// ------------------------------------------------------------------------
public function gc($maxlifetime)
{
+ // Not necessary, Memcached takes care of that.
return TRUE;
}
diff --git a/system/libraries/Session/drivers/Session_redis_driver.php b/system/libraries/Session/drivers/Session_redis_driver.php
index ef18def..bc6150d 100644
--- a/system/libraries/Session/drivers/Session_redis_driver.php
+++ b/system/libraries/Session/drivers/Session_redis_driver.php
@@ -135,6 +135,9 @@
{
if (isset($this->_redis) && $this->_get_lock($session_id))
{
+ // Needed by write() to detect session_regenerate_id() calls
+ $this->_session_id = $session_id;
+
$session_data = (string) $this->_redis->get($this->_key_prefix.$session_id);
$this->_fingerprint = md5($session_data);
return $session_data;
@@ -145,7 +148,23 @@
public function write($session_id, $session_data)
{
- if (isset($this->_redis, $this->_lock_key))
+ if ( ! isset($this->_redis))
+ {
+ return FALSE;
+ }
+ // Was the ID regenerated?
+ elseif ($session_id !== $this->_session_id)
+ {
+ if ( ! $this->_release_lock() OR ! $this->_get_lock($session_id))
+ {
+ return FALSE;
+ }
+
+ $this->_fingerprint = md5('');
+ $this->_session_id = $session_id;
+ }
+
+ if (isset($this->_lock_key))
{
$this->_redis->setTimeout($this->_lock_key, 5);
if ($this->_fingerprint !== ($fingerprint = md5($session_data)))
@@ -190,7 +209,7 @@
return TRUE;
}
- return FALSE;
+ return TRUE;
}
// ------------------------------------------------------------------------
@@ -204,20 +223,17 @@
log_message('debug', 'Session: Redis::delete() expected to return 1, got '.var_export($result, TRUE).' instead.');
}
- return ($this->_cookie_destroy() && $this->close());
+ return $this->_cookie_destroy();
}
- return $this->close();
+ return FALSE;
}
// ------------------------------------------------------------------------
public function gc($maxlifetime)
{
- // TODO: keys()/getKeys() is said to be performance-intensive,
- // although it supports patterns (*, [charlist] at the very least).
- // scan() seems to be recommended, but requires redis 2.8
- // Not sure if we need any of these though, as we set keys with expire times
+ // Not necessary, Redis takes care of that.
return TRUE;
}