diff --git a/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php b/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php index 9557135bcf..24280e38fc 100644 --- a/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php +++ b/src/Symfony/Component/HttpFoundation/Resources/stubs/SessionHandlerInterface.php @@ -12,14 +12,6 @@ /** * SessionHandlerInterface for PHP < 5.4 * - * The order in which these methods are invoked by PHP are: - * 1. open [session_start] - * 2. read - * 3. gc [optional depending on probability settings: gc_probability / gc_divisor] - * 4. destroy [optional when session_regenerate_id(true) is used] - * 5. write [session_write_close] or destroy [session_destroy] - * 6. close - * * Extensive documentation can be found at php.net, see links: * * @see http://php.net/sessionhandlerinterface @@ -27,7 +19,6 @@ * @see http://php.net/session-set-save-handler * * @author Drak - * @author Tobias Schultze */ interface SessionHandlerInterface { @@ -66,9 +57,6 @@ interface SessionHandlerInterface /** * Writes the session data to the storage. * - * Care, the session ID passed to write() can be different from the one previously - * received in read() when the session ID changed due to session_regenerate_id(). - * * @see http://php.net/sessionhandlerinterface.write * * @param string $sessionId Session ID , see http://php.net/function.session-id diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php index 4cdf3a8985..b831383a24 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php @@ -12,19 +12,7 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Handler; /** - * Session handler using a PDO connection to read and write data. - * - * It works with MySQL, PostgreSQL, Oracle, SQL Server and SQLite and implements - * locking of sessions to prevent loss of data by concurrent access to the same session. - * This means requests for the same session will wait until the other one finished. - * PHPs internal files session handler also works this way. - * - * Attention: Since SQLite does not support row level locks but locks the whole database, - * it means only one session can be accessed at a time. Even different sessions would wait - * for another to finish. So saving session in SQLite should only be considered for - * development or prototypes. - * - * @see http://php.net/sessionhandlerinterface + * PdoSessionHandler. * * @author Fabien Potencier * @author Michael Williams @@ -37,11 +25,6 @@ class PdoSessionHandler implements \SessionHandlerInterface */ private $pdo; - /** - * @var string Database driver - */ - private $driver; - /** * @var string Table name */ @@ -62,50 +45,39 @@ class PdoSessionHandler implements \SessionHandlerInterface */ private $timeCol; - /** - * @var bool Whether a transaction is active - */ - private $inTransaction = false; - - /** - * @var bool Whether gc() has been called - */ - private $gcCalled = false; - /** * Constructor. * * List of available options: - * * db_table: The name of the table [default: sessions] + * * db_table: The name of the table [required] * * db_id_col: The column where to store the session id [default: sess_id] * * db_data_col: The column where to store the session data [default: sess_data] * * db_time_col: The column where to store the timestamp [default: sess_time] * - * @param \PDO $pdo A \PDO instance - * @param array $options An associative array of DB options + * @param \PDO $pdo A \PDO instance + * @param array $dbOptions An associative array of DB options * - * @throws \InvalidArgumentException When PDO error mode is not PDO::ERRMODE_EXCEPTION + * @throws \InvalidArgumentException When "db_table" option is not provided */ - public function __construct(\PDO $pdo, array $options = array()) + public function __construct(\PDO $pdo, array $dbOptions = array()) { + if (!array_key_exists('db_table', $dbOptions)) { + throw new \InvalidArgumentException('You must provide the "db_table" option for a PdoSessionStorage.'); + } if (\PDO::ERRMODE_EXCEPTION !== $pdo->getAttribute(\PDO::ATTR_ERRMODE)) { throw new \InvalidArgumentException(sprintf('"%s" requires PDO error mode attribute be set to throw Exceptions (i.e. $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION))', __CLASS__)); } - $this->pdo = $pdo; - $this->driver = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME); - - $options = array_replace(array( - 'db_table' => 'sessions', + $dbOptions = array_merge(array( 'db_id_col' => 'sess_id', 'db_data_col' => 'sess_data', 'db_time_col' => 'sess_time', - ), $options); + ), $dbOptions); - $this->table = $options['db_table']; - $this->idCol = $options['db_id_col']; - $this->dataCol = $options['db_data_col']; - $this->timeCol = $options['db_time_col']; + $this->table = $dbOptions['db_table']; + $this->idCol = $dbOptions['db_id_col']; + $this->dataCol = $dbOptions['db_data_col']; + $this->timeCol = $dbOptions['db_time_col']; } /** @@ -113,56 +85,14 @@ class PdoSessionHandler implements \SessionHandlerInterface */ public function open($savePath, $sessionName) { - $this->gcCalled = false; - return true; } /** * {@inheritdoc} */ - public function read($sessionId) + public function close() { - $this->beginTransaction(); - - try { - $this->lockSession($sessionId); - - // We need to make sure we do not return session data that is already considered garbage according - // to the session.gc_maxlifetime setting because gc() is called after read() and only sometimes. - $maxlifetime = (int) ini_get('session.gc_maxlifetime'); - - $sql = "SELECT $this->dataCol FROM $this->table WHERE $this->idCol = :id AND $this->timeCol > :time"; - - $stmt = $this->pdo->prepare($sql); - $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); - $stmt->bindValue(':time', time() - $maxlifetime, \PDO::PARAM_INT); - $stmt->execute(); - - // We use fetchAll instead of fetchColumn to make sure the DB cursor gets closed - $sessionRows = $stmt->fetchAll(\PDO::FETCH_NUM); - - if ($sessionRows) { - return base64_decode($sessionRows[0][0]); - } - - return ''; - } catch (\PDOException $e) { - $this->rollback(); - - throw $e; - } - } - - /** - * {@inheritdoc} - */ - public function gc($maxlifetime) - { - // We delay gc() to close() so that it is executed outside the transactional and blocking read-write process. - // This way, pruning expired sessions does not block them from being started while the current session is used. - $this->gcCalled = true; - return true; } @@ -179,14 +109,56 @@ class PdoSessionHandler implements \SessionHandlerInterface $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); $stmt->execute(); } catch (\PDOException $e) { - $this->rollback(); - - throw $e; + throw new \RuntimeException(sprintf('PDOException was thrown when trying to delete a session: %s', $e->getMessage()), 0, $e); } return true; } + /** + * {@inheritdoc} + */ + public function gc($maxlifetime) + { + // delete the session records that have expired + $sql = "DELETE FROM $this->table WHERE $this->timeCol < :time"; + + try { + $stmt = $this->pdo->prepare($sql); + $stmt->bindValue(':time', time() - $maxlifetime, \PDO::PARAM_INT); + $stmt->execute(); + } catch (\PDOException $e) { + throw new \RuntimeException(sprintf('PDOException was thrown when trying to delete expired sessions: %s', $e->getMessage()), 0, $e); + } + + return true; + } + + /** + * {@inheritdoc} + */ + public function read($sessionId) + { + $sql = "SELECT $this->dataCol FROM $this->table WHERE $this->idCol = :id"; + + try { + $stmt = $this->pdo->prepare($sql); + $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); + $stmt->execute(); + + // We use fetchAll instead of fetchColumn to make sure the DB cursor gets closed + $sessionRows = $stmt->fetchAll(\PDO::FETCH_NUM); + + if ($sessionRows) { + return base64_decode($sessionRows[0][0]); + } + + return ''; + } catch (\PDOException $e) { + throw new \RuntimeException(sprintf('PDOException was thrown when trying to read the session data: %s', $e->getMessage()), 0, $e); + } + } + /** * {@inheritdoc} */ @@ -195,10 +167,8 @@ class PdoSessionHandler implements \SessionHandlerInterface // Session data can contain non binary safe characters so we need to encode it. $encoded = base64_encode($data); - // The session ID can be different from the one previously received in read() - // when the session ID changed due to session_regenerate_id(). So we have to - // do an insert or update even if we created a row in read() for locking. // We use a MERGE SQL query when supported by the database. + // Otherwise we have to use a transactional DELETE followed by INSERT to prevent duplicate entries under high concurrency. try { $mergeSql = $this->getMergeSql(); @@ -213,18 +183,15 @@ class PdoSessionHandler implements \SessionHandlerInterface return true; } - $updateStmt = $this->pdo->prepare( - "UPDATE $this->table SET $this->dataCol = :data, $this->timeCol = :time WHERE $this->idCol = :id" - ); - $updateStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); - $updateStmt->bindParam(':data', $encoded, \PDO::PARAM_STR); - $updateStmt->bindValue(':time', time(), \PDO::PARAM_INT); - $updateStmt->execute(); + $this->pdo->beginTransaction(); + + try { + $deleteStmt = $this->pdo->prepare( + "DELETE FROM $this->table WHERE $this->idCol = :id" + ); + $deleteStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); + $deleteStmt->execute(); - // Since we have a lock on the session, this is safe to do. Otherwise it would be prone to - // race conditions in high concurrency. And if it's a regenerated session ID it should be - // unique anyway. - if (!$updateStmt->rowCount()) { $insertStmt = $this->pdo->prepare( "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)" ); @@ -232,153 +199,18 @@ class PdoSessionHandler implements \SessionHandlerInterface $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR); $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT); $insertStmt->execute(); - } - } catch (\PDOException $e) { - $this->rollback(); - throw $e; - } - - return true; - } - - /** - * {@inheritdoc} - */ - public function close() - { - $this->commit(); - - if ($this->gcCalled) { - $maxlifetime = (int) ini_get('session.gc_maxlifetime'); - - // delete the session records that have expired - $sql = "DELETE FROM $this->table WHERE $this->timeCol <= :time"; - - $stmt = $this->pdo->prepare($sql); - $stmt->bindValue(':time', time() - $maxlifetime, \PDO::PARAM_INT); - $stmt->execute(); - } - - return true; - } - - /** - * Helper method to begin a transaction. - * - * Since SQLite does not support row level locks, we have to acquire a reserved lock - * on the database immediately. Because of https://bugs.php.net/42766 we have to create - * such a transaction manually which also means we cannot use PDO::commit or - * PDO::rollback or PDO::inTransaction for SQLite. - */ - private function beginTransaction() - { - if ($this->inTransaction) { - $this->rollback(); - - throw new \BadMethodCallException( - 'Session handler methods have been invoked in wrong sequence. ' . - 'Expected sequence: open() -> read() -> destroy() / write() -> close()'); - } - - if ('sqlite' === $this->driver) { - $this->pdo->exec('BEGIN IMMEDIATE TRANSACTION'); - } else { - $this->pdo->beginTransaction(); - } - $this->inTransaction = true; - } - - /** - * Helper method to commit a transaction. - */ - private function commit() - { - if ($this->inTransaction) { - try { - // commit read-write transaction which also releases the lock - if ('sqlite' === $this->driver) { - $this->pdo->exec('COMMIT'); - } else { - $this->pdo->commit(); - } - $this->inTransaction = false; + $this->pdo->commit(); } catch (\PDOException $e) { - $this->rollback(); + $this->pdo->rollback(); throw $e; } - } - } - - /** - * Helper method to rollback a transaction. - */ - private function rollback() - { - // We only need to rollback if we are in a transaction. Otherwise the resulting - // error would hide the real problem why rollback was called. We might not be - // in a transaction when two callbacks (e.g. destroy and write) are invoked that - // both fail. - if ($this->inTransaction) { - if ('sqlite' === $this->driver) { - $this->pdo->exec('ROLLBACK'); - } else { - $this->pdo->rollback(); - } - $this->inTransaction = false; - } - } - - /** - * Exclusively locks the row so other concurrent requests on the same session will block. - * - * This prevents loss of data by keeping the data consistent between read() and write(). - * We do not use SELECT FOR UPDATE because it does not lock non-existent rows. And a following - * INSERT when not found can result in a deadlock for one connection. - * - * @param string $sessionId Session ID - */ - private function lockSession($sessionId) - { - switch ($this->driver) { - case 'mysql': - // will also lock the row when actually nothing got updated (id = id) - $sql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . - "ON DUPLICATE KEY UPDATE $this->idCol = $this->idCol"; - break; - case 'oci': - // DUAL is Oracle specific dummy table - $sql = "MERGE INTO $this->table USING DUAL ON ($this->idCol = :id) " . - "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . - "WHEN MATCHED THEN UPDATE SET $this->idCol = $this->idCol"; - break; - case 'sqlsrv': - // MS SQL Server requires MERGE be terminated by semicolon - $sql = "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " . - "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . - "WHEN MATCHED THEN UPDATE SET $this->idCol = $this->idCol;"; - break; - case 'pgsql': - // obtain an exclusive transaction level advisory lock - $sql = 'SELECT pg_advisory_xact_lock(:lock_id)'; - $stmt = $this->pdo->prepare($sql); - $stmt->bindValue(':lock_id', hexdec(substr($sessionId, 0, 15)), \PDO::PARAM_INT); - $stmt->execute(); - - return; - default: - return; + } catch (\PDOException $e) { + throw new \RuntimeException(sprintf('PDOException was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e); } - // We create a DML lock for the session by inserting empty data or updating the row. - // This is safer than an application level advisory lock because it also prevents concurrent modification - // of the session from other parts of the application. - $stmt = $this->pdo->prepare($sql); - $stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR); - $stmt->bindValue(':data', '', \PDO::PARAM_STR); - $stmt->bindValue(':time', time(), \PDO::PARAM_INT); - $stmt->execute(); + return true; } /** @@ -388,7 +220,9 @@ class PdoSessionHandler implements \SessionHandlerInterface */ private function getMergeSql() { - switch ($this->driver) { + $driver = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME); + + switch ($driver) { case 'mysql': return "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . "ON DUPLICATE KEY UPDATE $this->dataCol = VALUES($this->dataCol), $this->timeCol = VALUES($this->timeCol)"; @@ -396,12 +230,12 @@ class PdoSessionHandler implements \SessionHandlerInterface // DUAL is Oracle specific dummy table return "MERGE INTO $this->table USING DUAL ON ($this->idCol = :id) " . "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . - "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time"; + "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data"; case 'sqlsrv': // MS SQL Server requires MERGE be terminated by semicolon return "MERGE INTO $this->table USING (SELECT 'x' AS dummy) AS src ON ($this->idCol = :id) " . "WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time) " . - "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time;"; + "WHEN MATCHED THEN UPDATE SET $this->dataCol = :data;"; case 'sqlite': return "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"; } diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php index 109addbcbd..e465f39898 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php @@ -29,108 +29,74 @@ class PdoSessionHandlerTest extends \PHPUnit_Framework_TestCase $this->pdo->exec($sql); } - /** - * @expectedException \InvalidArgumentException - */ + public function testIncompleteOptions() + { + $this->setExpectedException('InvalidArgumentException'); + $storage = new PdoSessionHandler($this->pdo, array()); + } + public function testWrongPdoErrMode() { - $this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_SILENT); + $pdo = new \PDO("sqlite::memory:"); + $pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_SILENT); + $pdo->exec("CREATE TABLE sessions (sess_id VARCHAR(255) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)"); - $storage = new PdoSessionHandler($this->pdo); + $this->setExpectedException('InvalidArgumentException'); + $storage = new PdoSessionHandler($pdo, array('db_table' => 'sessions')); } - /** - * @expectedException \RuntimeException - */ - public function testInexistentTable() + public function testWrongTableOptionsWrite() { - $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'inexistent_table')); - $storage->open('', 'sid'); - $storage->read('id'); - $storage->write('id', 'data'); - $storage->close(); + $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'bad_name')); + $this->setExpectedException('RuntimeException'); + $storage->write('foo', 'bar'); } - public function testReadWriteRead() + public function testWrongTableOptionsRead() { - $storage = new PdoSessionHandler($this->pdo); - $storage->open('', 'sid'); - $this->assertSame('', $storage->read('id'), 'New session returns empty string data'); - $storage->write('id', 'data'); - $storage->close(); - - $storage->open('', 'sid'); - $this->assertSame('data', $storage->read('id'), 'Written value can be read back correctly'); - $storage->close(); + $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'bad_name')); + $this->setExpectedException('RuntimeException'); + $storage->read('foo', 'bar'); } - /** - * Simulates session_regenerate_id(true) which will require an INSERT or UPDATE (replace) - */ - public function testWriteDifferentSessionIdThanRead() + public function testWriteRead() { - $storage = new PdoSessionHandler($this->pdo); - $storage->open('', 'sid'); - $storage->read('id'); - $storage->destroy('id'); - $storage->write('new_id', 'data_of_new_session_id'); - $storage->close(); - - $storage->open('', 'sid'); - $this->assertSame('data_of_new_session_id', $storage->read('new_id'), 'Data of regenerated session id is available'); - $storage->close(); + $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); + $storage->write('foo', 'bar'); + $this->assertEquals('bar', $storage->read('foo'), 'written value can be read back correctly'); } - /** - * @expectedException \BadMethodCallException - */ - public function testWrongUsage() + public function testMultipleInstances() { - $storage = new PdoSessionHandler($this->pdo); - $storage->open('', 'sid'); - $storage->read('id'); - $storage->read('id'); + $storage1 = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); + $storage1->write('foo', 'bar'); + + $storage2 = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); + $this->assertEquals('bar', $storage2->read('foo'), 'values persist between instances'); } public function testSessionDestroy() { - $storage = new PdoSessionHandler($this->pdo); + $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); + $storage->write('foo', 'bar'); + $this->assertCount(1, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); - $storage->open('', 'sid'); - $storage->read('id'); - $storage->write('id', 'data'); - $storage->close(); - $this->assertEquals(1, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); + $storage->destroy('foo'); - $storage->open('', 'sid'); - $storage->read('id'); - $storage->destroy('id'); - $storage->close(); - $this->assertEquals(0, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); - - $storage->open('', 'sid'); - $this->assertSame('', $storage->read('id'), 'Destroyed session returns empty string'); - $storage->close(); + $this->assertCount(0, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); } public function testSessionGC() { - $previousLifeTime = ini_set('session.gc_maxlifetime', 0); - $storage = new PdoSessionHandler($this->pdo); + $storage = new PdoSessionHandler($this->pdo, array('db_table' => 'sessions')); - $storage->open('', 'sid'); - $storage->read('id'); - $storage->write('id', 'data'); - $storage->close(); - $this->assertEquals(1, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); + $storage->write('foo', 'bar'); + $storage->write('baz', 'bar'); - $storage->open('', 'sid'); - $this->assertSame('', $storage->read('id'), 'Session already considered garbage, so not returning data even if it is not pruned yet'); - $storage->gc(0); - $storage->close(); - $this->assertEquals(0, $this->pdo->query('SELECT COUNT(*) FROM sessions')->fetchColumn()); + $this->assertCount(2, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); - ini_set('session.gc_maxlifetime', $previousLifeTime); + $storage->gc(-1); + $this->assertCount(0, $this->pdo->query('SELECT * FROM sessions')->fetchAll()); } public function testGetConnection()