Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[HttpFoundation] smaller fixes for PdoSessionHandler #11009

Merged
merged 2 commits into from
Jun 4, 2014
Merged

[HttpFoundation] smaller fixes for PdoSessionHandler #11009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
00d707f
[HttpFoundation] use different approach for duplicate keys in postgre…
Tobion May 28, 2014
a0e1d4d
[Doctrine Bridge] fix DBAL session handler according to PdoSessionHan…
Tobion May 28, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 44 additions & 31 deletions src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
namespace Symfony\Bridge\Doctrine\HttpFoundation;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Driver\DriverException;
use Doctrine\DBAL\Platforms\SQLServer2008Platform;

/**
* DBAL based session storage.
Expand Down Expand Up @@ -146,13 +148,10 @@ public function read($sessionId)
*/
public function write($sessionId, $data)
{
// 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 {
// We use a single MERGE SQL query when supported by the database.
$mergeSql = $this->getMergeSql();

if (null !== $mergeSql) {
Expand All @@ -165,28 +164,41 @@ public function write($sessionId, $data)
return true;
}

$this->con->beginTransaction();

try {
$deleteStmt = $this->con->prepare(
"DELETE FROM $this->table WHERE $this->idCol = :id"
);
$deleteStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$deleteStmt->execute();

$insertStmt = $this->con->prepare(
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
);
$insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
$insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
$insertStmt->execute();

$this->con->commit();
} catch (\Exception $e) {
$this->con->rollback();

throw $e;
$updateStmt = $this->con->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();

// When MERGE is not supported, like in Postgres, we have to use this approach that can result in
// duplicate key errors when the same session is written simultaneously. We can just catch such an
// error and re-execute the update. This is similar to a serializable transaction with retry logic
// on serialization failures but without the overhead and without possible false positives due to
// longer gap locking.
if (!$updateStmt->rowCount()) {
try {
$insertStmt = $this->con->prepare(
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
);
$insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
$insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
$insertStmt->execute();
} catch (\Exception $e) {
$driverException = $e->getPrevious();
// Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys
// DriverException only available since DBAL 2.5
if (
($driverException instanceof DriverException && 0 === strpos($driverException->getSQLState(), '23')) ||
($driverException instanceof \PDOException && 0 === strpos($driverException->getCode(), '23'))
) {
$updateStmt->execute();
} else {
throw $e;
}
}
}
} catch (\Exception $e) {
throw new \RuntimeException(sprintf('Exception was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e);
Expand All @@ -212,12 +224,13 @@ private function getMergeSql()
// 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";
case 'mssql':
// 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 MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time";
case $this->con->getDatabasePlatform() instanceof SQLServer2008Platform:
// MERGE is only available since SQL Server 2008 and must be terminated by semicolon
// It also requires HOLDLOCK according to http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx
return "MERGE INTO $this->table WITH (HOLDLOCK) USING (SELECT 1 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;";
"WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time;";
case 'sqlite':
return "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)";
}
Expand Down
9 changes: 3 additions & 6 deletions src/Symfony/Bridge/Doctrine/HttpFoundation/DbalSessionHandlerSchema.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,11 @@
*/
final class DbalSessionHandlerSchema extends Schema
{
private $tableName;

public function __construct($tableName = 'sessions')
{
parent::__construct();

$this->tableName = $tableName;
$this->addSessionTable();
$this->addSessionTable($tableName);
}

public function addToSchema(Schema $schema)
Expand All @@ -37,9 +34,9 @@ public function addToSchema(Schema $schema)
}
}

private function addSessionTable()
private function addSessionTable($tableName)
{
$table = $this->createTable($this->tableName);
$table = $this->createTable($tableName);
$table->addColumn('sess_id', 'string');
$table->addColumn('sess_data', 'text')->setNotNull(true);
$table->addColumn('sess_time', 'integer')->setNotNull(true)->setUnsigned(true);
Expand Down
76 changes: 44 additions & 32 deletions src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,13 @@
namespace Symfony\Component\HttpFoundation\Session\Storage\Handler;

/**
* PdoSessionHandler.
* Session handler using a PDO connection to read and write data.
*
* Session data is a binary string that can contain non-printable characters like the null byte.
* For this reason this handler base64 encodes the data to be able to save it in a character column.
*
* This version of the PdoSessionHandler does NOT implement locking. So concurrent requests to the
* same session can result in data loss due to race conditions.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Michael Williams <michael.williams@funsational.com>
Expand Down Expand Up @@ -164,13 +170,10 @@ public function read($sessionId)
*/
public function write($sessionId, $data)
{
// 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 {
// We use a single MERGE SQL query when supported by the database.
$mergeSql = $this->getMergeSql();

if (null !== $mergeSql) {
Expand All @@ -183,28 +186,36 @@ public function write($sessionId, $data)
return true;
}

$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();

$insertStmt = $this->pdo->prepare(
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
);
$insertStmt->bindParam(':id', $sessionId, \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;
$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();

// When MERGE is not supported, like in Postgres, we have to use this approach that can result in
// duplicate key errors when the same session is written simultaneously. We can just catch such an
// error and re-execute the update. This is similar to a serializable transaction with retry logic
// on serialization failures but without the overhead and without possible false positives due to
// longer gap locking.
if (!$updateStmt->rowCount()) {
try {
$insertStmt = $this->pdo->prepare(
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)"
);
$insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$insertStmt->bindParam(':data', $encoded, \PDO::PARAM_STR);
$insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
$insertStmt->execute();
} catch (\PDOException $e) {
// Handle integrity violation SQLSTATE 23000 (or a subclass like 23505 in Postgres) for duplicate keys
if (0 === strpos($e->getCode(), '23')) {
$updateStmt->execute();
} else {
throw $e;
}
}
}
} catch (\PDOException $e) {
throw new \RuntimeException(sprintf('PDOException was thrown when trying to write the session data: %s', $e->getMessage()), 0, $e);
Expand All @@ -230,12 +241,13 @@ private function getMergeSql()
// 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";
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 MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time";
case 'sqlsrv' && version_compare($this->pdo->getAttribute(\PDO::ATTR_SERVER_VERSION), '10', '>='):
// MERGE is only available since SQL Server 2008 and must be terminated by semicolon
// It also requires HOLDLOCK according to http://weblogs.sqlteam.com/dang/archive/2009/01/31/UPSERT-Race-Condition-With-MERGE.aspx
return "MERGE INTO $this->table WITH (HOLDLOCK) USING (SELECT 1 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;";
"WHEN MATCHED THEN UPDATE SET $this->dataCol = :data, $this->timeCol = :time;";
case 'sqlite':
return "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->timeCol) VALUES (:id, :data, :time)";
}
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/PdoSessionHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ protected function setUp()
$this->markTestSkipped('This test requires SQLite support in your environment');
}

$this->pdo = new \PDO("sqlite::memory:");
$this->pdo = new \PDO('sqlite::memory:');
$this->pdo->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION);
$sql = "CREATE TABLE sessions (sess_id VARCHAR(255) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)";
$sql = 'CREATE TABLE sessions (sess_id VARCHAR(128) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)';
$this->pdo->exec($sql);
}

Expand All @@ -37,9 +37,9 @@ public function testIncompleteOptions()

public function testWrongPdoErrMode()
{
$pdo = new \PDO("sqlite::memory:");
$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)");
$pdo->exec('CREATE TABLE sessions (sess_id VARCHAR(128) PRIMARY KEY, sess_data TEXT, sess_time INTEGER)');

$this->setExpectedException('InvalidArgumentException');
$storage = new PdoSessionHandler($pdo, array('db_table' => 'sessions'));
Expand Down