From da2e2ce2ae859df89b9cc99cc98df4832caef4f6 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Thu, 21 Mar 2024 08:15:21 +0000
Subject: [PATCH] Issue #3428565 by catch, phenaproxima, alexpott: Implement
lazy database creation for sessions
---
.../Core/Installer/InstallerRedirectTrait.php | 2 +-
.../Drupal/Core/Session/SessionHandler.php | 148 ++++++++++++++++--
.../Drupal/Core/Session/SessionManager.php | 11 +-
.../tests/src/Kernel/mysql/DbDumpTest.php | 4 +-
core/modules/system/system.install | 51 ------
.../Installer/InstallerRedirectTraitTest.php | 8 +-
6 files changed, 148 insertions(+), 76 deletions(-)
diff --git a/core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php b/core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php
index 4dbb485a93a3..45ee1cdd5070 100644
--- a/core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php
+++ b/core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php
@@ -73,7 +73,7 @@ protected function shouldRedirectToInstaller(\Throwable $exception, Connection $
// Redirect if the database is empty.
if ($connection) {
try {
- return !$connection->schema()->tableExists('sessions');
+ return !$connection->schema()->tableExists('sequences');
}
catch (\Exception $e) {
// If we still have an exception at this point, we need to be careful
diff --git a/core/lib/Drupal/Core/Session/SessionHandler.php b/core/lib/Drupal/Core/Session/SessionHandler.php
index e243c425c6db..f6fe3b28f58e 100644
--- a/core/lib/Drupal/Core/Session/SessionHandler.php
+++ b/core/lib/Drupal/Core/Session/SessionHandler.php
@@ -5,6 +5,7 @@
use Drupal\Component\Datetime\TimeInterface;
use Drupal\Component\Utility\Crypt;
use Drupal\Core\Database\Connection;
+use Drupal\Core\Database\DatabaseException;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpFoundation\Session\Storage\Proxy\AbstractProxy;
@@ -62,10 +63,15 @@ public function open(string $save_path, string $name): bool {
public function read(#[\SensitiveParameter] string $sid): string|false {
$data = '';
if (!empty($sid)) {
- // Read the session data from the database.
- $query = $this->connection
- ->queryRange('SELECT [session] FROM {sessions} WHERE [sid] = :sid', 0, 1, [':sid' => Crypt::hashBase64($sid)]);
- $data = (string) $query->fetchField();
+ try {
+ // Read the session data from the database.
+ $query = $this->connection
+ ->queryRange('SELECT [session] FROM {sessions} WHERE [sid] = :sid', 0, 1, [':sid' => Crypt::hashBase64($sid)]);
+ $data = (string) $query->fetchField();
+ }
+ // Swallow the error if the table hasn't been created yet.
+ catch (\Exception) {
+ }
}
return $data;
}
@@ -74,6 +80,7 @@ public function read(#[\SensitiveParameter] string $sid): string|false {
* {@inheritdoc}
*/
public function write(#[\SensitiveParameter] string $sid, string $value): bool {
+ $try_again = FALSE;
$request = $this->requestStack->getCurrentRequest();
$fields = [
'uid' => $request->getSession()->get('uid', 0),
@@ -81,10 +88,27 @@ public function write(#[\SensitiveParameter] string $sid, string $value): bool {
'session' => $value,
'timestamp' => $this->time->getRequestTime(),
];
- $this->connection->merge('sessions')
- ->keys(['sid' => Crypt::hashBase64($sid)])
- ->fields($fields)
- ->execute();
+ $doWrite = fn() =>
+ $this->connection->merge('sessions')
+ ->keys(['sid' => Crypt::hashBase64($sid)])
+ ->fields($fields)
+ ->execute();
+ try {
+ $doWrite();
+ }
+ catch (\Exception $e) {
+ // If there was an exception, try to create the table.
+ if (!$try_again = $this->ensureTableExists()) {
+ // If the exception happened for other reason than the missing
+ // table, propagate the exception.
+ throw $e;
+ }
+ }
+ // Now that the bin has been created, try again if necessary.
+ if ($try_again) {
+ $doWrite();
+ }
+
return TRUE;
}
@@ -99,10 +123,15 @@ public function close(): bool {
* {@inheritdoc}
*/
public function destroy(#[\SensitiveParameter] string $sid): bool {
- // Delete session data.
- $this->connection->delete('sessions')
- ->condition('sid', Crypt::hashBase64($sid))
- ->execute();
+ try {
+ // Delete session data.
+ $this->connection->delete('sessions')
+ ->condition('sid', Crypt::hashBase64($sid))
+ ->execute();
+ }
+ // Swallow the error if the table hasn't been created yet.
+ catch (\Exception) {
+ }
return TRUE;
}
@@ -116,9 +145,98 @@ public function gc(int $lifetime): int|false {
// for three weeks before deleting them, you need to set gc_maxlifetime
// to '1814400'. At that value, only after a user doesn't log in after
// three weeks (1814400 seconds) will their session be removed.
- return $this->connection->delete('sessions')
- ->condition('timestamp', $this->time->getRequestTime() - $lifetime, '<')
- ->execute();
+ try {
+ return $this->connection->delete('sessions')
+ ->condition('timestamp', $this->time->getRequestTime() - $lifetime, '<')
+ ->execute();
+ }
+ // Swallow the error if the table hasn't been created yet.
+ catch (\Exception) {
+ }
+ return FALSE;
+ }
+
+ /**
+ * Defines the schema for the session table.
+ *
+ * @internal
+ */
+ protected function schemaDefinition(): array {
+ $schema = [
+ 'description' => "Drupal's session handlers read and write into the sessions table. Each record represents a user session, either anonymous or authenticated.",
+ 'fields' => [
+ 'uid' => [
+ 'description' => 'The {users}.uid corresponding to a session, or 0 for anonymous user.',
+ 'type' => 'int',
+ 'unsigned' => TRUE,
+ 'not null' => TRUE,
+ ],
+ 'sid' => [
+ 'description' => "A session ID (hashed). The value is generated by Drupal's session handlers.",
+ 'type' => 'varchar_ascii',
+ 'length' => 128,
+ 'not null' => TRUE,
+ ],
+ 'hostname' => [
+ 'description' => 'The IP address that last used this session ID (sid).',
+ 'type' => 'varchar_ascii',
+ 'length' => 128,
+ 'not null' => TRUE,
+ 'default' => '',
+ ],
+ 'timestamp' => [
+ 'description' => 'The Unix timestamp when this session last requested a page. Old records are purged by PHP automatically.',
+ 'type' => 'int',
+ 'not null' => TRUE,
+ 'default' => 0,
+ 'size' => 'big',
+ ],
+ 'session' => [
+ 'description' => 'The serialized contents of the user\'s session, an array of name/value pairs that persists across page requests by this session ID. Drupal loads the user\'s session from here at the start of each request and saves it at the end.',
+ 'type' => 'blob',
+ 'not null' => FALSE,
+ 'size' => 'big',
+ ],
+ ],
+ 'primary key' => [
+ 'sid',
+ ],
+ 'indexes' => [
+ 'timestamp' => ['timestamp'],
+ 'uid' => ['uid'],
+ ],
+ 'foreign keys' => [
+ 'session_user' => [
+ 'table' => 'users',
+ 'columns' => ['uid' => 'uid'],
+ ],
+ ],
+ ];
+
+ return $schema;
+ }
+
+ /**
+ * Check if the session table exists and create it if not.
+ *
+ * @return bool
+ * TRUE if the table already exists or was created, FALSE if creation fails.
+ */
+ protected function ensureTableExists(): bool {
+ try {
+ $database_schema = $this->connection->schema();
+ $schema_definition = $this->schemaDefinition();
+ $database_schema->createTable('sessions', $schema_definition);
+ }
+ // If another process has already created the table, attempting to create
+ // it will throw an exception. In this case just catch the exception and do
+ // nothing.
+ catch (DatabaseException $e) {
+ }
+ catch (\Exception $e) {
+ return FALSE;
+ }
+ return TRUE;
}
}
diff --git a/core/lib/Drupal/Core/Session/SessionManager.php b/core/lib/Drupal/Core/Session/SessionManager.php
index cf794ebe4b6b..f43db06b29bf 100644
--- a/core/lib/Drupal/Core/Session/SessionManager.php
+++ b/core/lib/Drupal/Core/Session/SessionManager.php
@@ -234,9 +234,14 @@ public function delete($uid) {
if (!$this->writeSafeHandler->isSessionWritable() || $this->isCli()) {
return;
}
- $this->connection->delete('sessions')
- ->condition('uid', $uid)
- ->execute();
+ // The sessions table may not have been created yet.
+ try {
+ $this->connection->delete('sessions')
+ ->condition('uid', $uid)
+ ->execute();
+ }
+ catch (\Exception) {
+ }
}
/**
diff --git a/core/modules/mysql/tests/src/Kernel/mysql/DbDumpTest.php b/core/modules/mysql/tests/src/Kernel/mysql/DbDumpTest.php
index bc509a13b208..787095110f73 100644
--- a/core/modules/mysql/tests/src/Kernel/mysql/DbDumpTest.php
+++ b/core/modules/mysql/tests/src/Kernel/mysql/DbDumpTest.php
@@ -26,6 +26,8 @@ class DbDumpTest extends DriverSpecificKernelTestBase {
* {@inheritdoc}
*/
protected static $modules = [
+ // @todo system can be removed from this test once
+ // https://www.drupal.org/project/drupal/issues/2851705 is committed.
'system',
'config',
'dblog',
@@ -87,7 +89,6 @@ protected function setUp(): void {
parent::setUp();
// Create some schemas so our export contains tables.
- $this->installSchema('system', ['sessions']);
$this->installSchema('dblog', ['watchdog']);
$this->installEntitySchema('block_content');
$this->installEntitySchema('user');
@@ -131,7 +132,6 @@ protected function setUp(): void {
'menu_link_content_data',
'menu_link_content_revision',
'menu_link_content_field_revision',
- 'sessions',
'path_alias',
'path_alias_revision',
'user__roles',
diff --git a/core/modules/system/system.install b/core/modules/system/system.install
index 7474ae9846cf..09ea1af714f8 100644
--- a/core/modules/system/system.install
+++ b/core/modules/system/system.install
@@ -1621,57 +1621,6 @@ function system_schema() {
'primary key' => ['value'],
];
- $schema['sessions'] = [
- 'description' => "Drupal's session handlers read and write into the sessions table. Each record represents a user session, either anonymous or authenticated.",
- 'fields' => [
- 'uid' => [
- 'description' => 'The {users}.uid corresponding to a session, or 0 for anonymous user.',
- 'type' => 'int',
- 'unsigned' => TRUE,
- 'not null' => TRUE,
- ],
- 'sid' => [
- 'description' => "A session ID (hashed). The value is generated by Drupal's session handlers.",
- 'type' => 'varchar_ascii',
- 'length' => 128,
- 'not null' => TRUE,
- ],
- 'hostname' => [
- 'description' => 'The IP address that last used this session ID (sid).',
- 'type' => 'varchar_ascii',
- 'length' => 128,
- 'not null' => TRUE,
- 'default' => '',
- ],
- 'timestamp' => [
- 'description' => 'The Unix timestamp when this session last requested a page. Old records are purged by PHP automatically.',
- 'type' => 'int',
- 'not null' => TRUE,
- 'default' => 0,
- 'size' => 'big',
- ],
- 'session' => [
- 'description' => 'The serialized contents of the user\'s session, an array of name/value pairs that persists across page requests by this session ID. Drupal loads the user\'s session from here at the start of each request and saves it at the end.',
- 'type' => 'blob',
- 'not null' => FALSE,
- 'size' => 'big',
- ],
- ],
- 'primary key' => [
- 'sid',
- ],
- 'indexes' => [
- 'timestamp' => ['timestamp'],
- 'uid' => ['uid'],
- ],
- 'foreign keys' => [
- 'session_user' => [
- 'table' => 'users',
- 'columns' => ['uid' => 'uid'],
- ],
- ],
- ];
-
return $schema;
}
diff --git a/core/tests/Drupal/KernelTests/Core/Installer/InstallerRedirectTraitTest.php b/core/tests/Drupal/KernelTests/Core/Installer/InstallerRedirectTraitTest.php
index 41b2e770e307..b26b69147fb8 100644
--- a/core/tests/Drupal/KernelTests/Core/Installer/InstallerRedirectTraitTest.php
+++ b/core/tests/Drupal/KernelTests/Core/Installer/InstallerRedirectTraitTest.php
@@ -27,7 +27,7 @@ class InstallerRedirectTraitTest extends KernelTestBase {
* - Exceptions to be handled by shouldRedirectToInstaller()
* - Whether or not there is a database connection.
* - Whether or not there is database connection info.
- * - Whether or not there exists a sessions table in the database.
+ * - Whether or not there exists a sequences table in the database.
*/
public static function providerShouldRedirectToInstaller() {
return [
@@ -67,7 +67,7 @@ public static function providerShouldRedirectToInstaller() {
* @covers ::shouldRedirectToInstaller
* @dataProvider providerShouldRedirectToInstaller
*/
- public function testShouldRedirectToInstaller($expected, $exception, $connection, $connection_info, $session_table_exists = TRUE) {
+ public function testShouldRedirectToInstaller($expected, $exception, $connection, $connection_info, $sequences_table_exists = TRUE) {
try {
throw new $exception();
}
@@ -106,8 +106,8 @@ public function testShouldRedirectToInstaller($expected, $exception, $connection
$schema->expects($this->any())
->method('tableExists')
- ->with('sessions')
- ->willReturn($session_table_exists);
+ ->with('sequences')
+ ->willReturn($sequences_table_exists);
$connection->expects($this->any())
->method('schema')
--
GitLab