From e849cbb0905faee3fbd73bba8655cbda82efadcd Mon Sep 17 00:00:00 2001 From: azjezz Date: Wed, 14 Aug 2019 15:53:04 +0100 Subject: [PATCH] [HttpFoundation] Precalculate session expiry timestamp Co-authored-by: Benjamin Cremer Co-authored-by: Rob Frawley 2nd --- UPGRADE-4.4.md | 3 + .../Component/HttpFoundation/CHANGELOG.md | 5 +- .../Storage/Handler/PdoSessionHandler.php | 60 ++++++++++++------- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md index 46f76af7cbf73..2189f6b231120 100644 --- a/UPGRADE-4.4.md +++ b/UPGRADE-4.4.md @@ -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 ---------- diff --git a/src/Symfony/Component/HttpFoundation/CHANGELOG.md b/src/Symfony/Component/HttpFoundation/CHANGELOG.md index 62aa6be13d1b4..577f27e6c6af8 100644 --- a/src/Symfony/Component/HttpFoundation/CHANGELOG.md +++ b/src/Symfony/Component/HttpFoundation/CHANGELOG.md @@ -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 ----- diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php index f1648adaa52e4..2d50956cefd43 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php @@ -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 */ @@ -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(); @@ -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) { @@ -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(); } @@ -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 ''; @@ -747,6 +762,7 @@ private function getSelectSql(): string if (self::LOCK_TRANSACTIONAL === $this->lockMode) { $this->beginTransaction(); + // In the future, we should avoid selecting {$this->>timeCol} column switch ($this->driver) { case 'mysql': case 'oci': @@ -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; @@ -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; @@ -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', '>='): @@ -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: @@ -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); }