Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feature #33169 [HttpFoundation] Precalculate session expiry timestamp…
… (azjezz)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpFoundation] Precalculate session expiry timestamp

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #13955, #21423, #14625
| License       | MIT

Continued work from the original PR #21423 / #21857

Commits
-------

066000a [HttpFoundation] Precalculate session expiry timestamp
  • Loading branch information
nicolas-grekas committed Aug 20, 2019
2 parents 225bf41 + 066000a commit b253f25
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 23 deletions.
3 changes: 3 additions & 0 deletions UPGRADE-4.4.md
Expand Up @@ -109,6 +109,9 @@ HttpFoundation

* `ApacheRequest` is deprecated, use `Request` class instead.
* Passing a third argument to `HeaderBag::get()` is deprecated since Symfony 4.4, use method `all()` instead
* `PdoSessionHandler` now precalculates the expiry timestamp in the lifetime column,
make sure to run `CREATE INDEX EXPIRY ON sessions (sess_lifetime)` to update your database
to speed up garbage collection of expired sessions.

HttpKernel
----------
Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Component/HttpFoundation/CHANGELOG.md
Expand Up @@ -7,7 +7,10 @@ CHANGELOG
* passing arguments to `Request::isMethodSafe()` is deprecated.
* `ApacheRequest` is deprecated, use the `Request` class instead.
* passing a third argument to `HeaderBag::get()` is deprecated, use method `all()` instead

* `PdoSessionHandler` now precalculates the expiry timestamp in the lifetime column,
make sure to run `CREATE INDEX EXPIRY ON sessions (sess_lifetime)` to update your database
to speed up garbage collection of expired sessions.

4.3.0
-----

Expand Down
Expand Up @@ -65,6 +65,8 @@ class PdoSessionHandler extends AbstractSessionHandler
*/
const LOCK_TRANSACTIONAL = 2;

private const MAX_LIFETIME = 315576000;

/**
* @var \PDO|null PDO instance or null when not connected yet
*/
Expand Down Expand Up @@ -237,6 +239,7 @@ public function createTable()

try {
$this->pdo->exec($sql);
$this->pdo->exec("CREATE INDEX EXPIRY ON $this->table ($this->lifetimeCol)");
} catch (\PDOException $e) {
$this->rollback();

Expand Down Expand Up @@ -368,14 +371,14 @@ protected function doWrite($sessionId, $data)
*/
public function updateTimestamp($sessionId, $data)
{
$maxlifetime = (int) ini_get('session.gc_maxlifetime');
$expiry = time() + (int) ini_get('session.gc_maxlifetime');

try {
$updateStmt = $this->pdo->prepare(
"UPDATE $this->table SET $this->lifetimeCol = :lifetime, $this->timeCol = :time WHERE $this->idCol = :id"
"UPDATE $this->table SET $this->lifetimeCol = :expiry, $this->timeCol = :time WHERE $this->idCol = :id"
);
$updateStmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$updateStmt->bindParam(':lifetime', $maxlifetime, \PDO::PARAM_INT);
$updateStmt->bindParam(':expiry', $expiry, \PDO::PARAM_INT);
$updateStmt->bindValue(':time', time(), \PDO::PARAM_INT);
$updateStmt->execute();
} catch (\PDOException $e) {
Expand All @@ -402,14 +405,21 @@ public function close()
$this->gcCalled = false;

// delete the session records that have expired
$sql = "DELETE FROM $this->table WHERE $this->lifetimeCol < :time AND $this->lifetimeCol > :min";
$stmt = $this->pdo->prepare($sql);
$stmt->bindValue(':time', time(), \PDO::PARAM_INT);
$stmt->bindValue(':min', self::MAX_LIFETIME, \PDO::PARAM_INT);
$stmt->execute();
// to be removed in 6.0
if ('mysql' === $this->driver) {
$sql = "DELETE FROM $this->table WHERE $this->lifetimeCol + $this->timeCol < :time";
$legacySql = "DELETE FROM $this->table WHERE $this->lifetimeCol <= :min AND $this->lifetimeCol + $this->timeCol < :time";
} else {
$sql = "DELETE FROM $this->table WHERE $this->lifetimeCol < :time - $this->timeCol";
$legacySql = "DELETE FROM $this->table WHERE $this->lifetimeCol <= :min AND $this->lifetimeCol < :time - $this->timeCol";
}

$stmt = $this->pdo->prepare($sql);
$stmt = $this->pdo->prepare($legacySql);
$stmt->bindValue(':time', time(), \PDO::PARAM_INT);
$stmt->bindValue(':min', self::MAX_LIFETIME, \PDO::PARAM_INT);
$stmt->execute();
}

Expand Down Expand Up @@ -616,7 +626,12 @@ protected function doRead($sessionId)
$sessionRows = $selectStmt->fetchAll(\PDO::FETCH_NUM);

if ($sessionRows) {
if ($sessionRows[0][1] + $sessionRows[0][2] < time()) {
$expiry = (int) $sessionRows[0][1];
if ($expiry <= self::MAX_LIFETIME) {
$expiry += $sessionRows[0][2];
}

if ($expiry < time()) {
$this->sessionExpired = true;

return '';
Expand Down Expand Up @@ -747,6 +762,7 @@ private function getSelectSql(): string
if (self::LOCK_TRANSACTIONAL === $this->lockMode) {
$this->beginTransaction();

// selecting the time column should be removed in 6.0
switch ($this->driver) {
case 'mysql':
case 'oci':
Expand Down Expand Up @@ -775,18 +791,18 @@ private function getInsertStatement(string $sessionId, string $sessionData, int
$data = fopen('php://memory', 'r+');
fwrite($data, $sessionData);
rewind($data);
$sql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, EMPTY_BLOB(), :lifetime, :time) RETURNING $this->dataCol into :data";
$sql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, EMPTY_BLOB(), :expiry, :time) RETURNING $this->dataCol into :data";
break;
default:
$data = $sessionData;
$sql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :lifetime, :time)";
$sql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :expiry, :time)";
break;
}

$stmt = $this->pdo->prepare($sql);
$stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$stmt->bindParam(':data', $data, \PDO::PARAM_LOB);
$stmt->bindParam(':lifetime', $maxlifetime, \PDO::PARAM_INT);
$stmt->bindValue(':expiry', time() + $maxlifetime, \PDO::PARAM_INT);
$stmt->bindValue(':time', time(), \PDO::PARAM_INT);

return $stmt;
Expand All @@ -802,18 +818,18 @@ private function getUpdateStatement(string $sessionId, string $sessionData, int
$data = fopen('php://memory', 'r+');
fwrite($data, $sessionData);
rewind($data);
$sql = "UPDATE $this->table SET $this->dataCol = EMPTY_BLOB(), $this->lifetimeCol = :lifetime, $this->timeCol = :time WHERE $this->idCol = :id RETURNING $this->dataCol into :data";
$sql = "UPDATE $this->table SET $this->dataCol = EMPTY_BLOB(), $this->lifetimeCol = :expiry, $this->timeCol = :time WHERE $this->idCol = :id RETURNING $this->dataCol into :data";
break;
default:
$data = $sessionData;
$sql = "UPDATE $this->table SET $this->dataCol = :data, $this->lifetimeCol = :lifetime, $this->timeCol = :time WHERE $this->idCol = :id";
$sql = "UPDATE $this->table SET $this->dataCol = :data, $this->lifetimeCol = :expiry, $this->timeCol = :time WHERE $this->idCol = :id";
break;
}

$stmt = $this->pdo->prepare($sql);
$stmt->bindParam(':id', $sessionId, \PDO::PARAM_STR);
$stmt->bindParam(':data', $data, \PDO::PARAM_LOB);
$stmt->bindParam(':lifetime', $maxlifetime, \PDO::PARAM_INT);
$stmt->bindValue(':expiry', time() + $maxlifetime, \PDO::PARAM_INT);
$stmt->bindValue(':time', time(), \PDO::PARAM_INT);

return $stmt;
Expand All @@ -826,7 +842,7 @@ private function getMergeStatement(string $sessionId, string $data, int $maxlife
{
switch (true) {
case 'mysql' === $this->driver:
$mergeSql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :lifetime, :time) ".
$mergeSql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :expiry, :time) ".
"ON DUPLICATE KEY UPDATE $this->dataCol = VALUES($this->dataCol), $this->lifetimeCol = VALUES($this->lifetimeCol), $this->timeCol = VALUES($this->timeCol)";
break;
case 'sqlsrv' === $this->driver && version_compare($this->pdo->getAttribute(\PDO::ATTR_SERVER_VERSION), '10', '>='):
Expand All @@ -837,10 +853,10 @@ private function getMergeStatement(string $sessionId, string $data, int $maxlife
"WHEN MATCHED THEN UPDATE SET $this->dataCol = ?, $this->lifetimeCol = ?, $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->lifetimeCol, $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) ".
$mergeSql = "INSERT INTO $this->table ($this->idCol, $this->dataCol, $this->lifetimeCol, $this->timeCol) VALUES (:id, :data, :expiry, :time) ".
"ON CONFLICT ($this->idCol) DO UPDATE SET ($this->dataCol, $this->lifetimeCol, $this->timeCol) = (EXCLUDED.$this->dataCol, EXCLUDED.$this->lifetimeCol, EXCLUDED.$this->timeCol)";
break;
default:
Expand All @@ -854,15 +870,15 @@ private function getMergeStatement(string $sessionId, string $data, int $maxlife
$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(5, time(), \PDO::PARAM_INT);
$mergeStmt->bindParam(6, $data, \PDO::PARAM_LOB);
$mergeStmt->bindParam(7, $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(8, time(), \PDO::PARAM_INT);
$mergeStmt->bindValue(4, time() + $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(4, time(), \PDO::PARAM_INT);
$mergeStmt->bindParam(5, $data, \PDO::PARAM_LOB);
$mergeStmt->bindValue(6, time() + $maxlifetime, \PDO::PARAM_INT);
$mergeStmt->bindValue(6, 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

0 comments on commit b253f25

Please sign in to comment.