Skip to content

Commit

Permalink
[HttpFoundation] Precalculate expiry timestamp
Browse files Browse the repository at this point in the history
Removed timestamp and lifetime columns in favor of expiry column.
Based on the work done in symfony#14625.

Fix symfony#13955.
  • Loading branch information
bcremer committed Jan 26, 2017
1 parent b9b6ebd commit 5780201
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 34 deletions.
24 changes: 24 additions & 0 deletions UPGRADE-3.3.md
Expand Up @@ -39,6 +39,30 @@ FrameworkBundle
---------------

* The `Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddConsoleCommandPass` has been deprecated. Use `Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass` instead.

HttpFoundation
--------------

* The schema of the `PdoSessionHandler` to store sessions in a database changed.
The following changes must be made to your `session` table:

- Create a new `sess_expiry` column. In MySQL this would be:

```sql
ALTER TABLE `session` ADD `sess_expiry` INTEGER UNSIGNED NOT NULL;
```

- Migrate the old data to the `sess_expiry` column. In MySQL this would be:

```sql
UPDATE `session` SET `sess_expiry` = `sess_time` + `sess_lifetime`;
```

- Remove the `sess_time` and `sess_lifetime` columns. In MySQL this would be:

```sql
ALTER TABLE `session` DROP COLUMN `sess_time`, DROP COLUMN `sess_lifetime`;
```

HttpKernel
-----------
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/HttpFoundation/CHANGELOG.md
Expand Up @@ -8,7 +8,8 @@ CHANGELOG
disabling `Range` and `Content-Length` handling, switching to chunked encoding instead
* added the `Cookie::fromString()` method that allows to create a cookie from a
raw header string

* PdoSessionHandler: [BC BREAK] removed the `lifetime` and `timestamp` columns in favor of `expiry` column

3.1.0
-----

Expand Down
Expand Up @@ -98,7 +98,7 @@ class PdoSessionHandler implements \SessionHandlerInterface
/**
* @var string Column for lifetime
*/
private $lifetimeCol = 'sess_lifetime';
private $expiryCol = 'sess_expiry';

/**
* @var string Column for timestamp
Expand Down Expand Up @@ -159,7 +159,7 @@ class PdoSessionHandler implements \SessionHandlerInterface
* * db_table: The name of the table [default: sessions]
* * 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_lifetime_col: The column where to store the lifetime [default: sess_lifetime]
* * db_expiry_col: The column where to store the expirytime [default: sess_expiry]
* * db_time_col: The column where to store the timestamp [default: sess_time]
* * db_username: The username when lazy-connect [default: '']
* * db_password: The password when lazy-connect [default: '']
Expand Down Expand Up @@ -187,7 +187,7 @@ public function __construct($pdoOrDsn = null, array $options = array())
$this->table = isset($options['db_table']) ? $options['db_table'] : $this->table;
$this->idCol = isset($options['db_id_col']) ? $options['db_id_col'] : $this->idCol;
$this->dataCol = isset($options['db_data_col']) ? $options['db_data_col'] : $this->dataCol;
$this->lifetimeCol = isset($options['db_lifetime_col']) ? $options['db_lifetime_col'] : $this->lifetimeCol;
$this->expiryCol = isset($options['db_expiry_col']) ? $options['db_expiry_col'] : $this->expiryCol;
$this->timeCol = isset($options['db_time_col']) ? $options['db_time_col'] : $this->timeCol;
$this->username = isset($options['db_username']) ? $options['db_username'] : $this->username;
$this->password = isset($options['db_password']) ? $options['db_password'] : $this->password;
Expand Down Expand Up @@ -218,19 +218,19 @@ public function createTable()
// - trailing space removal
// - case-insensitivity
// - language processing like é == e
$sql = "CREATE TABLE $this->table ($this->idCol VARBINARY(128) NOT NULL PRIMARY KEY, $this->dataCol BLOB NOT NULL, $this->lifetimeCol MEDIUMINT NOT NULL, $this->timeCol INTEGER UNSIGNED NOT NULL) COLLATE utf8_bin, ENGINE = InnoDB";
$sql = "CREATE TABLE $this->table ($this->idCol VARBINARY(128) NOT NULL PRIMARY KEY, $this->dataCol BLOB NOT NULL, $this->expiryCol MEDIUMINT NOT NULL, $this->timeCol INTEGER UNSIGNED NOT NULL) COLLATE utf8_bin, ENGINE = InnoDB";
break;
case 'sqlite':
$sql = "CREATE TABLE $this->table ($this->idCol TEXT NOT NULL PRIMARY KEY, $this->dataCol BLOB NOT NULL, $this->lifetimeCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
$sql = "CREATE TABLE $this->table ($this->idCol TEXT NOT NULL PRIMARY KEY, $this->dataCol BLOB NOT NULL, $this->expiryCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
break;
case 'pgsql':
$sql = "CREATE TABLE $this->table ($this->idCol VARCHAR(128) NOT NULL PRIMARY KEY, $this->dataCol BYTEA NOT NULL, $this->lifetimeCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
$sql = "CREATE TABLE $this->table ($this->idCol VARCHAR(128) NOT NULL PRIMARY KEY, $this->dataCol BYTEA NOT NULL, $this->expiryCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
break;
case 'oci':
$sql = "CREATE TABLE $this->table ($this->idCol VARCHAR2(128) NOT NULL PRIMARY KEY, $this->dataCol BLOB NOT NULL, $this->lifetimeCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
$sql = "CREATE TABLE $this->table ($this->idCol VARCHAR2(128) NOT NULL PRIMARY KEY, $this->dataCol BLOB NOT NULL, $this->expiryCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
break;
case 'sqlsrv':
$sql = "CREATE TABLE $this->table ($this->idCol VARCHAR(128) NOT NULL PRIMARY KEY, $this->dataCol VARBINARY(MAX) NOT NULL, $this->lifetimeCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
$sql = "CREATE TABLE $this->table ($this->idCol VARCHAR(128) NOT NULL PRIMARY KEY, $this->dataCol VARBINARY(MAX) NOT NULL, $this->expiryCol INTEGER NOT NULL, $this->timeCol INTEGER NOT NULL)";
break;
default:
throw new \DomainException(sprintf('Creating the session table is currently not implemented for PDO driver "%s".', $this->driver));
Expand Down Expand Up @@ -333,11 +333,11 @@ public function write($sessionId, $data)
}

$updateStmt = $this->pdo->prepare(
"UPDATE $this->table SET $this->dataCol = :data, $this->lifetimeCol = :lifetime, $this->timeCol = :time WHERE $this->idCol = :id"
"UPDATE $this->table SET $this->dataCol = :data, $this->expiryCol = :expiry, $this->timeCol = :time WHERE $this->idCol = :id"
);
$updateStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$updateStmt->bindParam(':data', $data, \PDO::PARAM_LOB);
$updateStmt->bindParam(':lifetime', $maxlifetime, \PDO::PARAM_INT);
$updateStmt->bindValue(':expiry', time() + $maxlifetime, \PDO::PARAM_INT);
$updateStmt->bindValue(':time', time(), \PDO::PARAM_INT);
$updateStmt->execute();

Expand All @@ -349,11 +349,11 @@ public function write($sessionId, $data)
if (!$updateStmt->rowCount()) {
try {
$insertStmt = $this->pdo->prepare(
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :lifetime, :time)"
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->expiryCol, $this->timeCol) VALUES (:id, :data, :expiry, :time)"
);
$insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$insertStmt->bindParam(':data', $data, \PDO::PARAM_LOB);
$insertStmt->bindParam(':lifetime', $maxlifetime, \PDO::PARAM_INT);
$insertStmt->bindValue(':expiry', time() + $maxlifetime, \PDO::PARAM_INT);
$insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
$insertStmt->execute();
} catch (\PDOException $e) {
Expand Down Expand Up @@ -389,7 +389,7 @@ public function close()
$this->gcCalled = false;

// delete the session records that have expired
$sql = "DELETE FROM $this->table WHERE $this->lifetimeCol + $this->timeCol < :time";
$sql = "DELETE FROM $this->table WHERE $this->expiryCol < :time";

$stmt = $this->pdo->prepare($sql);
$stmt->bindValue(':time', time(), \PDO::PARAM_INT);
Expand Down Expand Up @@ -510,7 +510,7 @@ private function doRead($sessionId)
$sessionRows = $selectStmt->fetchAll(\PDO::FETCH_NUM);

if ($sessionRows) {
if ($sessionRows[0][1] + $sessionRows[0][2] < time()) {
if ($sessionRows[0][1] < time()) {
$this->sessionExpired = true;

return '';
Expand All @@ -524,11 +524,11 @@ private function doRead($sessionId)
// until other connections to the session are committed.
try {
$insertStmt = $this->pdo->prepare(
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :lifetime, :time)"
"INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->expiryCol, $this->timeCol) VALUES (:id, :data, :expiry, :time)"
);
$insertStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$insertStmt->bindValue(':data', '', \PDO::PARAM_LOB);
$insertStmt->bindValue(':lifetime', 0, \PDO::PARAM_INT);
$insertStmt->bindValue(':expiry', 0, \PDO::PARAM_INT);
$insertStmt->bindValue(':time', time(), \PDO::PARAM_INT);
$insertStmt->execute();
} catch (\PDOException $e) {
Expand Down Expand Up @@ -629,9 +629,9 @@ private function getSelectSql()
case 'mysql':
case 'oci':
case 'pgsql':
return "SELECT $this->dataCol, $this->lifetimeCol, $this->timeCol FROM $this->table WHERE $this->idCol = :id FOR UPDATE";
return "SELECT $this->dataCol, $this->expiryCol, $this->timeCol FROM $this->table WHERE $this->idCol = :id FOR UPDATE";
case 'sqlsrv':
return "SELECT $this->dataCol, $this->lifetimeCol, $this->timeCol FROM $this->table WITH (UPDLOCK, ROWLOCK) WHERE $this->idCol = :id";
return "SELECT $this->dataCol, $this->expiryCol, $this->timeCol FROM $this->table WITH (UPDLOCK, ROWLOCK) WHERE $this->idCol = :id";
case 'sqlite':
// we already locked when starting transaction
break;
Expand All @@ -640,7 +640,7 @@ private function getSelectSql()
}
}

return "SELECT $this->dataCol, $this->lifetimeCol, $this->timeCol FROM $this->table WHERE $this->idCol = :id";
return "SELECT $this->dataCol, $this->expiryCol, $this->timeCol FROM $this->table WHERE $this->idCol = :id";
}

/**
Expand All @@ -657,28 +657,28 @@ private function getMergeStatement($sessionId, $data, $maxlifetime)
$mergeSql = null;
switch (true) {
case 'mysql' === $this->driver:
$mergeSql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :lifetime, :time) ".
"ON DUPLICATE KEY UPDATE $this->dataCol = VALUES($this->dataCol), $this->lifetimeCol = VALUES($this->lifetimeCol), $this->timeCol = VALUES($this->timeCol)";
$mergeSql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->expiryCol, $this->timeCol) VALUES (:id, :data, :expiry, :time) ".
"ON DUPLICATE KEY UPDATE $this->dataCol = VALUES($this->dataCol), $this->expiryCol = VALUES($this->expiryCol), $this->timeCol = VALUES($this->timeCol)";
break;
case 'oci' === $this->driver:
// DUAL is Oracle specific dummy table
$mergeSql = "MERGE INTO $this->table USING DUAL ON ($this->idCol = ?) ".
"WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (?, ?, ?, ?) ".
"WHEN MATCHED THEN UPDATE SET $this->dataCol = ?, $this->lifetimeCol = ?, $this->timeCol = ?";
"WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->expiryCol, $this->timeCol) VALUES (?, ?, ?, ?) ".
"WHEN MATCHED THEN UPDATE SET $this->dataCol = ?, $this->expiryCol = ?, $this->timeCol = ?";
break;
case 'sqlsrv' === $this->driver && 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
$mergeSql = "MERGE INTO $this->table WITH (HOLDLOCK) USING (SELECT 1 AS dummy) AS src ON ($this->idCol = ?) ".
"WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (?, ?, ?, ?) ".
"WHEN MATCHED THEN UPDATE SET $this->dataCol = ?, $this->lifetimeCol = ?, $this->timeCol = ?;";
"WHEN NOT MATCHED THEN INSERT ($this->idCol, $this->dataCol, $this->expiryCol, $this->timeCol) VALUES (?, ?, ?, ?) ".
"WHEN MATCHED THEN UPDATE SET $this->dataCol = ?, $this->expiryCol = ?, $this->timeCol = ?;";
break;
case 'sqlite' === $this->driver:
$mergeSql = "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :lifetime, :time)";
$mergeSql = "INSERT OR REPLACE INTO $this->table ($this->idCol, $this->dataCol, $this->expiryCol, $this->timeCol) VALUES (:id, :data, :expiry, :time)";
break;
case 'pgsql' === $this->driver && version_compare($this->pdo->getAttribute(\PDO::ATTR_SERVER_VERSION), '9.5', '>='):
$mergeSql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :lifetime, :time) ".
"ON CONFLICT ($this->idCol) DO UPDATE SET ($this->dataCol, $this->lifetimeCol, $this->timeCol) = (EXCLUDED.$this->dataCol, EXCLUDED.$this->lifetimeCol, EXCLUDED.$this->timeCol)";
$mergeSql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->expiryCol, $this->timeCol) VALUES (:id, :data, :expiry, :time) ".
"ON CONFLICT ($this->idCol) DO UPDATE SET ($this->dataCol, $this->expiryCol, $this->timeCol) = (EXCLUDED.$this->dataCol, EXCLUDED.$this->expiryCol, EXCLUDED.$this->timeCol)";
break;
}

Expand All @@ -689,15 +689,15 @@ private function getMergeStatement($sessionId, $data, $maxlifetime)
$mergeStmt->bindParam(1, $sessionId, \PDO::PARAM_STR);
$mergeStmt->bindParam(2, $sessionId, \PDO::PARAM_STR);
$mergeStmt->bindParam(3, $data, \PDO::PARAM_LOB);
$mergeStmt->bindParam(4, $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(4, time() + $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(5, time(), \PDO::PARAM_INT);
$mergeStmt->bindParam(6, $data, \PDO::PARAM_LOB);
$mergeStmt->bindParam(7, $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(7, time() + $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(8, time(), \PDO::PARAM_INT);
} else {
$mergeStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$mergeStmt->bindParam(':data', $data, \PDO::PARAM_LOB);
$mergeStmt->bindParam(':lifetime', $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(':expiry', time() + $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(':time', time(), \PDO::PARAM_INT);
}

Expand Down
Expand Up @@ -146,7 +146,7 @@ public function testReadConvertsStreamToString()
$stream = $this->createStream($content);

$pdo->prepareResult->expects($this->once())->method('fetchAll')
->will($this->returnValue(array(array($stream, 42, time()))));
->will($this->returnValue(array(array($stream, 42 + time()))));

$storage = new PdoSessionHandler($pdo);
$result = $storage->read('foo');
Expand Down Expand Up @@ -174,7 +174,7 @@ public function testReadLockedConvertsStreamToString()

$selectStmt->expects($this->atLeast(2))->method('fetchAll')
->will($this->returnCallback(function () use (&$exception, $stream) {
return $exception ? array(array($stream, 42, time())) : array();
return $exception ? array(array($stream, 42 + time())) : array();
}));

$insertStmt->expects($this->once())->method('execute')
Expand Down

0 comments on commit 5780201

Please sign in to comment.