Skip to content

Commit

Permalink
Misc session fixes, inspired by #318
Browse files Browse the repository at this point in the history
  • Loading branch information
@lonnieezell
lonnieezell committed Dec 1, 2016
1 parent 4c3856f commit b198b43
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 90 deletions.
14 changes: 7 additions & 7 deletions application/Config/App.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,13 @@ class App extends BaseConfig
| except for 'cookie_prefix' and 'cookie_httponly', which are ignored here.
|
*/
public $sessionDriver = 'CodeIgniter\Session\Handlers\FileHandler';
public $sessionCookieName = 'ci_session';
public $sessionExpiration = 7200;
public $sessionSavePath = NULL;
public $sessionMatchIP = FALSE;
public $sessionTimeToUpdate = 300;
public $sessionRegenerateDestroy = FALSE;
public $sessionDriver = 'CodeIgniter\Session\Handlers\FileHandler';
public $sessionCookieName = 'ci_session';
public $sessionExpiration = 7200;
public $sessionSavePath = null;
public $sessionMatchIP = false;
public $sessionTimeToUpdate = 300;
public $sessionRegenerateDestroy = false;

/*
|--------------------------------------------------------------------------
Expand Down
12 changes: 10 additions & 2 deletions system/Session/Handlers/BaseHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ abstract class BaseHandler implements \SessionHandlerInterface
*/
protected $sessionID;

/**
* The 'save path' for the session
* varies between
* @var mixed
*/
protected $savePath;

//--------------------------------------------------------------------

/**
Expand All @@ -120,6 +127,7 @@ public function __construct(BaseConfig $config)
$this->cookieSecure = $config->cookieSecure;
$this->cookieName = $config->sessionCookieName;
$this->matchIP = $config->sessionMatchIP;
$this->savePath = $config->sessionSavePath;
}

//--------------------------------------------------------------------
Expand Down Expand Up @@ -150,11 +158,11 @@ protected function destroyCookie(): bool
* (databases other than PostgreSQL and MySQL) to act as if they
* do acquire a lock.
*
* @param string $session_id
* @param string $sessionID
*
* @return bool
*/
protected function lockSession(string $session_id): bool
protected function lockSession(string $sessionID): bool
{
$this->lock = true;
return true;
Expand Down
57 changes: 29 additions & 28 deletions system/Session/Handlers/FileHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,17 @@ public function __construct(BaseConfig $config)

//--------------------------------------------------------------------

/**
* Open
*
* Sanitizes the save_path directory.
*
* @param string $savePath Path to session files' directory
* @param string $name Session cookie name
*
* @return bool
*/
/**
* Open
*
* Sanitizes the save_path directory.
*
* @param string $savePath Path to session files' directory
* @param string $name Session cookie name
*
* @return bool
* @throws \Exception
*/
public function open($savePath, $name): bool
{
if ( ! is_dir($savePath))
Expand Down Expand Up @@ -135,39 +136,39 @@ public function open($savePath, $name): bool
*
* Reads session data and acquires a lock
*
* @param string $session_id Session ID
* @param string $sessionID Session ID
*
* @return string Serialized session data
*/
public function read($session_id)
public function read($sessionID)
{
// This might seem weird, but PHP 5.6 introduced session_reset(),
// which re-reads session data
if ($this->fileHandle === null)
{

if (($this->fileHandle = fopen($this->filePath.$session_id, 'c+b')) === false)
if (($this->fileHandle = fopen($this->filePath.$sessionID, 'c+b')) === false)
{
$this->logger->error("Session: Unable to open file '".$this->filePath.$session_id."'.");
$this->logger->error("Session: Unable to open file '".$this->filePath.$sessionID."'.");

return false;
}

if (flock($this->fileHandle, LOCK_EX) === false)
{
$this->logger->error("Session: Unable to obtain lock for file '".$this->filePath.$session_id."'.");
$this->logger->error("Session: Unable to obtain lock for file '".$this->filePath.$sessionID."'.");
fclose($this->fileHandle);
$this->fileHandle = null;

return false;
}

// Needed by write() to detect session_regenerate_id() calls
$this->sessionID = $session_id;
$this->sessionID = $sessionID;

if ($this->fileNew)
{
chmod($this->filePath.$session_id, 0600);
chmod($this->filePath.$sessionID, 0600);
$this->fingerprint = md5('');

return '';
Expand All @@ -179,7 +180,7 @@ public function read($session_id)
}

$session_data = '';
for ($read = 0, $length = filesize($this->filePath.$session_id); $read < $length; $read += strlen($buffer))
for ($read = 0, $length = filesize($this->filePath.$sessionID); $read < $length; $read += strlen($buffer))
{
if (($buffer = fread($this->fileHandle, $length - $read)) === false)
{
Expand All @@ -201,16 +202,16 @@ public function read($session_id)
*
* Writes (create / update) session data
*
* @param string $session_id Session ID
* @param string $session_data Serialized session data
* @param string $sessionID Session ID
* @param string $sessionData Serialized session data
*
* @return bool
*/
public function write($session_id, $session_data): bool
public function write($sessionID, $sessionData): bool
{
// 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->sessionID && ( ! $this->close() || $this->read($session_id) === false))
if ($sessionID !== $this->sessionID && (! $this->close() || $this->read($sessionID) === false))
{
return false;
}
Expand All @@ -219,11 +220,11 @@ public function write($session_id, $session_data): bool
{
return false;
}
elseif ($this->fingerprint === md5($session_data))
elseif ($this->fingerprint === md5($sessionData))
{
return ($this->fileNew)
? true
: touch($this->filePath.$session_id);
: touch($this->filePath.$sessionID);
}

if ( ! $this->fileNew)
Expand All @@ -232,26 +233,26 @@ public function write($session_id, $session_data): bool
rewind($this->fileHandle);
}

if (($length = strlen($session_data)) > 0)
if (($length = strlen($sessionData)) > 0)
{
for ($written = 0; $written < $length; $written += $result)
{
if (($result = fwrite($this->fileHandle, substr($session_data, $written))) === false)
if (($result = fwrite($this->fileHandle, substr($sessionData, $written))) === false)
{
break;
}
}

if ( ! is_int($result))
{
$this->_fingerprint = md5(substr($session_data, 0, $written));
$this->fingerprint = md5(substr($sessionData, 0, $written));
$this->logger->error('Session: Unable to write data.');

return false;
}
}

$this->fingerprint = md5($session_data);
$this->fingerprint = md5($sessionData);

return true;
}
Expand Down
42 changes: 21 additions & 21 deletions system/Session/Handlers/MemcachedHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class MemcachedHandler extends BaseHandler implements \SessionHandlerInterface

/**
* Constructor
*
*
* @param BaseConfig $config
* @throws \Exception
*/
Expand Down Expand Up @@ -166,18 +166,18 @@ public function open($save_path, $name)
*
* Reads session data and acquires a lock
*
* @param string $session_id Session ID
* @param string $sessionID Session ID
*
* @return string Serialized session data
*/
public function read($session_id)
public function read($sessionID)
{
if (isset($this->memcached) && $this->lockSession($session_id))
if (isset($this->memcached) && $this->lockSession($sessionID))
{
// Needed by write() to detect session_regenerate_id() calls
$this->sessionID = $session_id;
$this->sessionID = $sessionID;

$session_data = (string)$this->memcached->get($this->keyPrefix.$session_id);
$session_data = (string)$this->memcached->get($this->keyPrefix.$sessionID);
$this->fingerprint = md5($session_data);

return $session_data;
Expand All @@ -193,46 +193,46 @@ public function read($session_id)
*
* Writes (create / update) session data
*
* @param string $session_id Session ID
* @param string $session_data Serialized session data
* @param string $sessionID Session ID
* @param string $sessionData Serialized session data
*
* @return bool
*/
public function write($session_id, $session_data)
public function write($sessionID, $sessionData)
{
if ( ! isset($this->memcached))
{
return false;
}
// Was the ID regenerated?
elseif ($session_id !== $this->sessionID)
elseif ($sessionID !== $this->sessionID)
{
if ( ! $this->releaseLock() || ! $this->lockSession($session_id))
if ( ! $this->releaseLock() || ! $this->lockSession($sessionID))
{
return false;
}

$this->fingerprint = md5('');
$this->sessionID = $session_id;
$this->sessionID = $sessionID;
}

if (isset($this->lockKey))
{
$this->memcached->replace($this->lockKey, time(), 300);

if ($this->fingerprint !== ($fingerprint = md5($session_data)))
if ($this->fingerprint !== ($fingerprint = md5($sessionData)))
{
if ($this->memcached->set($this->keyPrefix.$session_id, $session_data, $this->sessionExpiration))
if ($this->memcached->set($this->keyPrefix.$sessionID, $sessionData, $this->sessionExpiration))
{
$this->_fingerprint = $fingerprint;
$this->fingerprint = $fingerprint;

return true;
}

return false;
}

return $this->memcached->touch($this->keyPrefix.$session_id, $this->sessionExpiration);
return $this->memcached->touch($this->keyPrefix.$sessionID, $this->sessionExpiration);
}

return false;
Expand Down Expand Up @@ -313,19 +313,19 @@ public function gc($maxlifetime)
*
* Acquires an (emulated) lock.
*
* @param string $session_id Session ID
* @param string $sessionID Session ID
*
* @return bool
*/
protected function lockSession(string $session_id): bool
protected function lockSession(string $sessionID): bool
{
if (isset($this->lockKey))
{
return $this->memcached->replace($this->lockKey, time(), 300);
}

// 30 attempts to obtain a lock, in case another request already has it
$lock_key = $this->keyPrefix.$session_id.':lock';
$lock_key = $this->keyPrefix.$sessionID.':lock';
$attempt = 0;

do
Expand All @@ -338,7 +338,7 @@ protected function lockSession(string $session_id): bool

if ( ! $this->memcached->set($lock_key, time(), 300))
{
$this->logger->error('Session: Error while trying to obtain lock for '.$this->keyPrefix.$session_id);
$this->logger->error('Session: Error while trying to obtain lock for '.$this->keyPrefix.$sessionID);

return false;
}
Expand All @@ -350,7 +350,7 @@ protected function lockSession(string $session_id): bool

if ($attempt === 30)
{
$this->logger->error('Session: Unable to obtain lock for '.$this->keyPrefix.$session_id.' after 30 attempts, aborting.');
$this->logger->error('Session: Unable to obtain lock for '.$this->keyPrefix.$sessionID.' after 30 attempts, aborting.');

return false;
}
Expand Down
Loading

0 comments on commit b198b43

Please sign in to comment.