Permalink
Browse files

[HttpFoundation] fix PDO session handler under high concurrency

  • Loading branch information...
1 parent a18ee42 commit e58d7cf6c6da343b3e9259425ad8f11c9e05ca51 @Tobion Tobion committed Apr 8, 2014
Showing with 86 additions and 92 deletions.
  1. +86 −92 src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
@@ -16,18 +16,34 @@
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Michael Williams <michael.williams@funsational.com>
+ * @author Tobias Schultze <http://tobion.de>
*/
class PdoSessionHandler implements \SessionHandlerInterface
{
/**
- * @var \PDO PDO instance.
+ * @var \PDO PDO instance
*/
private $pdo;
/**
- * @var array Database options.
+ * @var string Table name
*/
- private $dbOptions;
+ private $table;
+
+ /**
+ * @var string Column for session id
+ */
+ private $idCol;
+
+ /**
+ * @var string Column for session data
+ */
+ private $dataCol;
+
+ /**
+ * @var string Column for timestamp
+ */
+ private $timeCol;
/**
* Constructor.
@@ -52,11 +68,16 @@ public function __construct(\PDO $pdo, array $dbOptions = array())
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->dbOptions = array_merge(array(
+ $dbOptions = array_merge(array(
'db_id_col' => 'sess_id',
'db_data_col' => 'sess_data',
'db_time_col' => 'sess_time',
), $dbOptions);
+
+ $this->table = $dbOptions['db_table'];
+ $this->idCol = $dbOptions['db_id_col'];
+ $this->dataCol = $dbOptions['db_data_col'];
+ $this->timeCol = $dbOptions['db_time_col'];
}
/**
@@ -80,19 +101,15 @@ public function close()
*/
public function destroy($id)
{
- // get table/column
- $dbTable = $this->dbOptions['db_table'];
- $dbIdCol = $this->dbOptions['db_id_col'];
-
// delete the record associated with this id
- $sql = "DELETE FROM $dbTable WHERE $dbIdCol = :id";
+ $sql = "DELETE FROM $this->table WHERE $this->idCol = :id";
try {
$stmt = $this->pdo->prepare($sql);
$stmt->bindParam(':id', $id, \PDO::PARAM_STR);
$stmt->execute();
} catch (\PDOException $e) {
- throw new \RuntimeException(sprintf('PDOException was thrown when trying to manipulate session data: %s', $e->getMessage()), 0, $e);
+ throw new \RuntimeException(sprintf('PDOException was thrown when trying to delete a session: %s', $e->getMessage()), 0, $e);
}
return true;
@@ -103,19 +120,15 @@ public function destroy($id)
*/
public function gc($lifetime)
{
- // get table/column
- $dbTable = $this->dbOptions['db_table'];
- $dbTimeCol = $this->dbOptions['db_time_col'];
-
// delete the session records that have expired
- $sql = "DELETE FROM $dbTable WHERE $dbTimeCol < :time";
+ $sql = "DELETE FROM $this->table WHERE $this->timeCol < :time";
try {
$stmt = $this->pdo->prepare($sql);
$stmt->bindValue(':time', time() - $lifetime, \PDO::PARAM_INT);
$stmt->execute();
} catch (\PDOException $e) {
- throw new \RuntimeException(sprintf('PDOException was thrown when trying to manipulate session data: %s', $e->getMessage()), 0, $e);
+ throw new \RuntimeException(sprintf('PDOException was thrown when trying to delete expired sessions: %s', $e->getMessage()), 0, $e);
}
return true;
@@ -126,29 +139,20 @@ public function gc($lifetime)
*/
public function read($id)
{
- // get table/columns
- $dbTable = $this->dbOptions['db_table'];
- $dbDataCol = $this->dbOptions['db_data_col'];
- $dbIdCol = $this->dbOptions['db_id_col'];
+ $sql = "SELECT $this->dataCol FROM $this->table WHERE $this->idCol = :id";
try {
- $sql = "SELECT $dbDataCol FROM $dbTable WHERE $dbIdCol = :id";
-
$stmt = $this->pdo->prepare($sql);
$stmt->bindParam(':id', $id, \PDO::PARAM_STR);
-
$stmt->execute();
- // it is recommended to use fetchAll so that PDO can close the DB cursor
- // we anyway expect either no rows, or one row with one column. fetchColumn, seems to be buggy #4777
+
+ // We use fetchAll instead of fetchColumn to make sure the DB cursor gets closed
$sessionRows = $stmt->fetchAll(\PDO::FETCH_NUM);
- if (count($sessionRows) == 1) {
+ if ($sessionRows) {
return base64_decode($sessionRows[0][0]);
}
- // session does not exist, create it
- $this->createNewSession($id);
-
return '';
} catch (\PDOException $e) {
throw new \RuntimeException(sprintf('PDOException was thrown when trying to read the session data: %s', $e->getMessage()), 0, $e);
@@ -160,84 +164,74 @@ public function read($id)
*/
public function write($id, $data)
{
- // get table/column
- $dbTable = $this->dbOptions['db_table'];
- $dbDataCol = $this->dbOptions['db_data_col'];
- $dbIdCol = $this->dbOptions['db_id_col'];
- $dbTimeCol = $this->dbOptions['db_time_col'];
-
- //session data can contain non binary safe characters so we need to encode it
+ // Session data can contain non binary safe characters so we need to encode it.
$encoded = base64_encode($data);
+ // 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 {
- $driver = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME);
-
- if ('mysql' === $driver) {
- // MySQL would report $stmt->rowCount() = 0 on UPDATE when the data is left unchanged
- // it could result in calling createNewSession() whereas the session already exists in
- // the DB which would fail as the id is unique
- $stmt = $this->pdo->prepare(
- "INSERT INTO $dbTable ($dbIdCol, $dbDataCol, $dbTimeCol) VALUES (:id, :data, :time) " .
- "ON DUPLICATE KEY UPDATE $dbDataCol = VALUES($dbDataCol), $dbTimeCol = VALUES($dbTimeCol)"
+ $mergeSql = $this->getMergeSql();
+
+ if (null !== $mergeSql) {
+ $mergeStmt = $this->pdo->prepare($mergeSql);
+ $mergeStmt->bindParam(':id', $id, \PDO::PARAM_STR);
+ $mergeStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
+ $mergeStmt->bindValue(':time', time(), \PDO::PARAM_INT);
+ $mergeStmt->execute();
+
+ return true;
+ }
+
+ $this->pdo->beginTransaction();
+
+ try {
+ $deleteStmt = $this->pdo->prepare(
+ "DELETE FROM $this->table WHERE $this->idCol = :id"
+ );
+ $deleteStmt->bindParam(':id', $id, \PDO::PARAM_STR);
+ $deleteStmt->execute();
+
+ $insertStmt = $this->pdo->prepare(
+ "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
);
- $stmt->bindParam(':id', $id, \PDO::PARAM_STR);
- $stmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
- $stmt->bindValue(':time', time(), \PDO::PARAM_INT);
- $stmt->execute();
- } elseif ('oci' === $driver) {
- $stmt = $this->pdo->prepare("MERGE INTO $dbTable USING DUAL ON($dbIdCol = :id) ".
- "WHEN NOT MATCHED THEN INSERT ($dbIdCol, $dbDataCol, $dbTimeCol) VALUES (:id, :data, sysdate) " .
- "WHEN MATCHED THEN UPDATE SET $dbDataCol = :data WHERE $dbIdCol = :id");
-
- $stmt->bindParam(':id', $id, \PDO::PARAM_STR);
- $stmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
- $stmt->execute();
- } else {
- $stmt = $this->pdo->prepare("UPDATE $dbTable SET $dbDataCol = :data, $dbTimeCol = :time WHERE $dbIdCol = :id");
- $stmt->bindParam(':id', $id, \PDO::PARAM_STR);
- $stmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
- $stmt->bindValue(':time', time(), \PDO::PARAM_INT);
- $stmt->execute();
-
- if (!$stmt->rowCount()) {
- // No session exists in the database to update. This happens when we have called
- // session_regenerate_id()
- $this->createNewSession($id, $data);
- }
+ $insertStmt->bindParam(':id', $id, \PDO::PARAM_STR);
+ $insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
+ $insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
+ $insertStmt->execute();
+
+ $this->pdo->commit();
+ } catch (\PDOException $e) {
+ $this->pdo->rollback();
+
+ throw $e;
}
} catch (\PDOException $e) {
- throw new \RuntimeException(sprintf('PDOException was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e);
+ throw new \RuntimeException(sprintf('PDOException was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e);
}
return true;
}
/**
- * Creates a new session with the given $id and $data
+ * Returns a merge/upsert (i.e. insert or update) SQL query when supported by the database.
*
- * @param string $id
- * @param string $data
- *
- * @return boolean True.
+ * @return string|null The SQL string or null when not supported
*/
- private function createNewSession($id, $data = '')
+ private function getMergeSql()
{
- // get table/column
- $dbTable = $this->dbOptions['db_table'];
- $dbDataCol = $this->dbOptions['db_data_col'];
- $dbIdCol = $this->dbOptions['db_id_col'];
- $dbTimeCol = $this->dbOptions['db_time_col'];
-
- $sql = "INSERT INTO $dbTable ($dbIdCol, $dbDataCol, $dbTimeCol) VALUES (:id, :data, :time)";
-
- //session data can contain non binary safe characters so we need to encode it
- $encoded = base64_encode($data);
- $stmt = $this->pdo->prepare($sql);
- $stmt->bindParam(':id', $id, \PDO::PARAM_STR);
- $stmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
- $stmt->bindValue(':time', time(), \PDO::PARAM_INT);
- $stmt->execute();
+ $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) " .
@srinikumar11
srinikumar11 Apr 28, 2014

In reference to this url http://dev.mysql.com/doc/refman/5.6/en/insert-on-duplicate.html. Existing user session will be lost by updating existing unique column. Kindly confirm how this is being handled.

@Tobion
Tobion Apr 28, 2014 Symfony member

What you mean?

+ "ON DUPLICATE KEY UPDATE $this->dataCol = VALUES($this->dataCol), $this->timeCol = VALUES($this->timeCol)";
+ case 'oci':
+ 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";
+ }
- return true;
+ return null;
}
}

0 comments on commit e58d7cf

Please sign in to comment.