From 29ce04e5d0343999feacfa07779f5c33fbcdb681 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Fri, 23 Sep 2016 10:34:06 +0100 Subject: [PATCH] Issue #728702 by Crell, daffie, pillarsdotnet, pjcdawkins, Damien Tournoud, David_Rothstein, reglogge, amateescu, tim.plunkett, tstoeckler, xjm, hussainweb, alexpott, cweagans, Tor Arne Thune, catch: Visiting index.php should redirect to install.php if settings.php already has database credentials but database is empty --- core/core.services.yml | 5 + core/lib/Drupal/Core/Database/Connection.php | 20 +++ .../DatabaseAccessDeniedException.php | 8 ++ .../Database/DatabaseNotFoundException.php | 2 +- .../Core/Database/Driver/mysql/Connection.php | 19 ++- .../Core/Database/Driver/pgsql/Connection.php | 25 +++- .../Database/Driver/pgsql/Install/Tasks.php | 3 +- .../Database/Driver/sqlite/Connection.php | 14 +- core/lib/Drupal/Core/DrupalKernel.php | 13 +- .../ExceptionDetectNeedsInstallSubscriber.php | 65 +++++++++ .../Core/Installer/InstallerRedirectTrait.php | 90 ++++++++++++ .../Tests/System/UncaughtExceptionTest.php | 4 +- .../Installer/InstallerRedirectTraitTest.php | 132 ++++++++++++++++++ 13 files changed, 388 insertions(+), 12 deletions(-) create mode 100644 core/lib/Drupal/Core/Database/DatabaseAccessDeniedException.php create mode 100644 core/lib/Drupal/Core/EventSubscriber/ExceptionDetectNeedsInstallSubscriber.php create mode 100644 core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php create mode 100644 core/tests/Drupal/Tests/Core/Installer/InstallerRedirectTraitTest.php diff --git a/core/core.services.yml b/core/core.services.yml index 0b18c3b19357..d1223e9cc757 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -1197,6 +1197,11 @@ services: tags: - { name: event_subscriber } arguments: ['@config.manager', '@config.storage', '@config.storage.snapshot'] + exception.needs_installer: + class: Drupal\Core\EventSubscriber\ExceptionDetectNeedsInstallSubscriber + arguments: ['@database'] + tags: + - { name: event_subscriber } exception.default_json: class: Drupal\Core\EventSubscriber\ExceptionJsonSubscriber tags: diff --git a/core/lib/Drupal/Core/Database/Connection.php b/core/lib/Drupal/Core/Database/Connection.php index 5acd2362b7dd..38342f91005c 100644 --- a/core/lib/Drupal/Core/Database/Connection.php +++ b/core/lib/Drupal/Core/Database/Connection.php @@ -1444,6 +1444,26 @@ public function quote($string, $parameter_type = \PDO::PARAM_STR) { return $this->connection->quote($string, $parameter_type); } + /** + * Extracts the SQLSTATE error from the PDOException. + * + * @param \Exception $e + * The exception + * + * @return string + * The five character error code. + */ + protected static function getSQLState(\Exception $e) { + // The PDOException code is not always reliable, try to see whether the + // message has something usable. + if (preg_match('/^SQLSTATE\[(\w{5})\]/', $e->getMessage(), $matches)) { + return $matches[1]; + } + else { + return $e->getCode(); + } + } + /** * Prevents the database connection from being serialized. */ diff --git a/core/lib/Drupal/Core/Database/DatabaseAccessDeniedException.php b/core/lib/Drupal/Core/Database/DatabaseAccessDeniedException.php new file mode 100644 index 000000000000..85bfea9d86ac --- /dev/null +++ b/core/lib/Drupal/Core/Database/DatabaseAccessDeniedException.php @@ -0,0 +1,8 @@ +<?php + +namespace Drupal\Core\Database; + +/** + * Exception thrown if access credentials fail. + */ +class DatabaseAccessDeniedException extends \RuntimeException implements DatabaseException {} diff --git a/core/lib/Drupal/Core/Database/DatabaseNotFoundException.php b/core/lib/Drupal/Core/Database/DatabaseNotFoundException.php index 63eb9a3a0b70..1c5b5eb32cf3 100644 --- a/core/lib/Drupal/Core/Database/DatabaseNotFoundException.php +++ b/core/lib/Drupal/Core/Database/DatabaseNotFoundException.php @@ -5,4 +5,4 @@ /** * Exception thrown if specified database is not found. */ -class DatabaseNotFoundException extends \RuntimeException {} +class DatabaseNotFoundException extends \RuntimeException implements DatabaseException {} diff --git a/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php index df510672155d..fd464388a1ed 100644 --- a/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/mysql/Connection.php @@ -2,6 +2,7 @@ namespace Drupal\Core\Database\Driver\mysql; +use Drupal\Core\Database\DatabaseAccessDeniedException; use Drupal\Core\Database\DatabaseExceptionWrapper; use Drupal\Core\Database\Database; @@ -26,6 +27,11 @@ class Connection extends DatabaseConnection { */ const DATABASE_NOT_FOUND = 1049; + /** + * Error code for "Access denied" error. + */ + const ACCESS_DENIED = 1045; + /** * Error code for "Can't initialize character set" error. */ @@ -139,7 +145,18 @@ public static function open(array &$connection_options = array()) { $connection_options['pdo'] += [\PDO::MYSQL_ATTR_MULTI_STATEMENTS => FALSE]; } - $pdo = new \PDO($dsn, $connection_options['username'], $connection_options['password'], $connection_options['pdo']); + try { + $pdo = new \PDO($dsn, $connection_options['username'], $connection_options['password'], $connection_options['pdo']); + } + catch (\PDOException $e) { + if ($e->getCode() == static::DATABASE_NOT_FOUND) { + throw new DatabaseNotFoundException($e->getMessage(), $e->getCode(), $e); + } + if ($e->getCode() == static::ACCESS_DENIED) { + throw new DatabaseAccessDeniedException($e->getMessage(), $e->getCode(), $e); + } + throw $e; + } // Force MySQL to use the UTF-8 character set. Also set the collation, if a // certain one has been set; otherwise, MySQL defaults to diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php index 80497962cb24..570faee693ec 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php @@ -4,6 +4,7 @@ use Drupal\Core\Database\Database; use Drupal\Core\Database\Connection as DatabaseConnection; +use Drupal\Core\Database\DatabaseAccessDeniedException; use Drupal\Core\Database\DatabaseNotFoundException; /** @@ -26,6 +27,14 @@ class Connection extends DatabaseConnection { */ const DATABASE_NOT_FOUND = 7; + /** + * Error code for "Connection failure" errors. + * + * Technically this is an internal error code that will only be shown in the + * PDOException message. It will need to get extracted. + */ + const CONNECTION_FAILURE = '08006'; + /** * The list of PostgreSQL reserved key words. * @@ -113,7 +122,21 @@ public static function open(array &$connection_options = array()) { // Convert numeric values to strings when fetching. \PDO::ATTR_STRINGIFY_FETCHES => TRUE, ); - $pdo = new \PDO($dsn, $connection_options['username'], $connection_options['password'], $connection_options['pdo']); + + try { + $pdo = new \PDO($dsn, $connection_options['username'], $connection_options['password'], $connection_options['pdo']); + } + catch (\PDOException $e) { + if (static::getSQLState($e) == static::CONNECTION_FAILURE) { + if (strpos($e->getMessage(), 'password authentication failed for user') !== FALSE) { + throw new DatabaseAccessDeniedException($e->getMessage(), $e->getCode(), $e); + } + elseif (strpos($e->getMessage(), 'database') !== FALSE && strpos($e->getMessage(), 'does not exist') !== FALSE) { + throw new DatabaseNotFoundException($e->getMessage(), $e->getCode(), $e); + } + } + throw $e; + } return $pdo; } diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php index 7e78b1405d3e..77e0a47538e2 100644 --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Install/Tasks.php @@ -4,7 +4,6 @@ use Drupal\Core\Database\Database; use Drupal\Core\Database\Install\Tasks as InstallTasks; -use Drupal\Core\Database\Driver\pgsql\Connection; use Drupal\Core\Database\DatabaseNotFoundException; /** @@ -66,7 +65,7 @@ protected function connect() { } catch (\Exception $e) { // Attempt to create the database if it is not found. - if ($e->getCode() == Connection::DATABASE_NOT_FOUND) { + if ($e instanceof DatabaseNotFoundException) { // Remove the database string from connection info. $connection_info = Database::getConnectionInfo(); $database = $connection_info['default']['database']; diff --git a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php index 23dd26132ca8..f43f04de9cef 100644 --- a/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php +++ b/core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php @@ -99,7 +99,19 @@ public static function open(array &$connection_options = array()) { // Convert numeric values to strings when fetching. \PDO::ATTR_STRINGIFY_FETCHES => TRUE, ); - $pdo = new \PDO('sqlite:' . $connection_options['database'], '', '', $connection_options['pdo']); + + try { + $pdo = new \PDO('sqlite:' . $connection_options['database'], '', '', $connection_options['pdo']); + } + catch (\PDOException $e) { + if ($e->getCode() == static::DATABASE_NOT_FOUND) { + throw new DatabaseNotFoundException($e->getMessage(), $e->getCode(), $e); + } + // SQLite doesn't have a distinct error code for access denied, so don't + // deal with that case. + throw $e; + } + // Create functions needed by SQLite. $pdo->sqliteCreateFunction('if', array(__CLASS__, 'sqlFunctionIf')); diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index a132ff1714e7..ab2f645c87d0 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -16,6 +16,7 @@ use Drupal\Core\Extension\ExtensionDiscovery; use Drupal\Core\File\MimeType\MimeTypeGuesser; use Drupal\Core\Http\TrustedHostsRequestFactory; +use Drupal\Core\Installer\InstallerRedirectTrait; use Drupal\Core\Language\Language; use Drupal\Core\Site\Settings; use Drupal\Core\Test\TestDatabase; @@ -45,6 +46,7 @@ * container, or modify existing services. */ class DrupalKernel implements DrupalKernelInterface, TerminableInterface { + use InstallerRedirectTrait; /** * Holds the class used for dumping the container to a PHP array. @@ -644,7 +646,7 @@ public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = // installed yet (i.e., if no $databases array has been defined in the // settings.php file) and we are not already installing. if (!Database::getConnectionInfo() && !drupal_installation_attempted() && PHP_SAPI !== 'cli') { - $response = new RedirectResponse($request->getBasePath() . '/core/install.php'); + $response = new RedirectResponse($request->getBasePath() . '/core/install.php', 302, ['Cache-Control' => 'no-cache']); } else { $this->boot(); @@ -683,14 +685,17 @@ public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = * If the passed in exception cannot be turned into a response. */ protected function handleException(\Exception $e, $request, $type) { + if ($this->shouldRedirectToInstaller($e, $this->container ? $this->container->get('database') : NULL)) { + return new RedirectResponse($request->getBasePath() . '/core/install.php', 302, ['Cache-Control' => 'no-cache']); + } + if ($e instanceof HttpExceptionInterface) { $response = new Response($e->getMessage(), $e->getStatusCode()); $response->headers->add($e->getHeaders()); return $response; } - else { - throw $e; - } + + throw $e; } /** diff --git a/core/lib/Drupal/Core/EventSubscriber/ExceptionDetectNeedsInstallSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ExceptionDetectNeedsInstallSubscriber.php new file mode 100644 index 000000000000..9b974f28e3f7 --- /dev/null +++ b/core/lib/Drupal/Core/EventSubscriber/ExceptionDetectNeedsInstallSubscriber.php @@ -0,0 +1,65 @@ +<?php + +namespace Drupal\Core\EventSubscriber; + +use Drupal\Core\Database\Connection; +use Drupal\Core\Installer\InstallerRedirectTrait; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent; +use Symfony\Component\HttpKernel\KernelEvents; + +/** + * Exception handler to determine if an exception indicates an uninstalled site. + */ +class ExceptionDetectNeedsInstallSubscriber implements EventSubscriberInterface { + use InstallerRedirectTrait; + + /** + * The default database connection. + * + * @var \Drupal\Core\Database\Connection + */ + protected $connection; + + /** + * Constructs a new ExceptionDetectNeedsInstallSubscriber. + * + * @param \Drupal\Core\Database\Connection $connection + * The default database connection. + */ + public function __construct(Connection $connection) { + $this->connection = $connection; + } + + /** + * Handles errors for this subscriber. + * + * @param \Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent $event + * The event to process. + */ + public function onException(GetResponseForExceptionEvent $event) { + $exception = $event->getException(); + if ($this->shouldRedirectToInstaller($exception, $this->connection)) { + // Only redirect if this is an HTML response (i.e., a user trying to view + // the site in a web browser before installing it). + $request = $event->getRequest(); + $format = $request->query->get(MainContentViewSubscriber::WRAPPER_FORMAT, $request->getRequestFormat()); + if ($format == 'html') { + $event->setResponse(new RedirectResponse($request->getBasePath() . '/core/install.php', 302, ['Cache-Control' => 'no-cache'])); + } + } + } + + /** + * Registers the methods in this class that should be listeners. + * + * @return array + * An array of event listener definitions. + */ + public static function getSubscribedEvents() { + $events[KernelEvents::EXCEPTION][] = ['onException', 100]; + return $events; + } + +} diff --git a/core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php b/core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php new file mode 100644 index 000000000000..067802d2bbbf --- /dev/null +++ b/core/lib/Drupal/Core/Installer/InstallerRedirectTrait.php @@ -0,0 +1,90 @@ +<?php + +namespace Drupal\Core\Installer; + +use Drupal\Core\Database\Connection; +use Drupal\Core\Database\Database; +use Drupal\Core\Database\DatabaseException; +use Drupal\Core\Database\DatabaseNotFoundException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; + +/** + * Provides methods for checking if Drupal is already installed. + */ +trait InstallerRedirectTrait { + + /** + * Returns whether the current PHP process runs on CLI. + * + * @return bool + */ + protected function isCli() { + return PHP_SAPI === 'cli'; + } + + /** + * Determines if an exception handler should redirect to the installer. + * + * @param \Exception $exception + * The exception to check. + * @param \Drupal\Core\Database\Connection|null $connection + * (optional) The default database connection. If not provided, a less + * comprehensive check will be performed. This can be the case if the + * exception occurs early enough that a database connection object isn't + * available from the container yet. + * + * @return bool + * TRUE if the exception handler should redirect to the installer because + * Drupal is not installed yet, or FALSE otherwise. + */ + protected function shouldRedirectToInstaller(\Exception $exception, Connection $connection = NULL) { + // Never redirect on the command line. + if ($this->isCli()) { + return FALSE; + } + + // Never redirect if we're already in the installer. + if (drupal_installation_attempted()) { + return FALSE; + } + + // If the database wasn't found, assume the user hasn't entered it properly + // and redirect to the installer. This check needs to come first because a + // DatabaseNotFoundException is also an instance of DatabaseException. + if ($exception instanceof DatabaseNotFoundException) { + return TRUE; + } + + // To avoid unnecessary queries, only act if the exception is one that is + // expected to occur when Drupal has not yet been installed. This includes + // NotFoundHttpException because an uninstalled site won't have route + // information available yet and therefore can return 404 errors. + if (!($exception instanceof \PDOException || $exception instanceof DatabaseException || $exception instanceof NotFoundHttpException)) { + return FALSE; + } + + // Redirect if there isn't even any database connection information in + // settings.php yet, since that means Drupal is not installed. + if (!Database::getConnectionInfo()) { + return TRUE; + } + + // Redirect if the database is empty. + if ($connection) { + try { + return !$connection->schema()->tableExists('sessions'); + } + catch (\Exception $e) { + // If we still have an exception at this point, we need to be careful + // since we should not redirect if the exception represents an error on + // an already-installed site (for example, if the database server went + // down). Assume we shouldn't redirect, just in case. + return FALSE; + } + } + + // When in doubt, don't redirect. + return FALSE; + } + +} diff --git a/core/modules/system/src/Tests/System/UncaughtExceptionTest.php b/core/modules/system/src/Tests/System/UncaughtExceptionTest.php index c155781d518d..059d6adc23a0 100644 --- a/core/modules/system/src/Tests/System/UncaughtExceptionTest.php +++ b/core/modules/system/src/Tests/System/UncaughtExceptionTest.php @@ -223,7 +223,7 @@ public function testLostDatabaseConnection() { 'value' => $incorrect_username, 'required' => TRUE, ); - $settings['databases']['default']['default']['passowrd'] = (object) array( + $settings['databases']['default']['default']['password'] = (object) array( 'value' => $this->randomMachineName(16), 'required' => TRUE, ); @@ -232,7 +232,7 @@ public function testLostDatabaseConnection() { $this->drupalGet(''); $this->assertResponse(500); - $this->assertRaw('PDOException'); + $this->assertRaw('DatabaseAccessDeniedException'); $this->assertErrorLogged($this->expectedExceptionMessage); } diff --git a/core/tests/Drupal/Tests/Core/Installer/InstallerRedirectTraitTest.php b/core/tests/Drupal/Tests/Core/Installer/InstallerRedirectTraitTest.php new file mode 100644 index 000000000000..8ca55d28eaef --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Installer/InstallerRedirectTraitTest.php @@ -0,0 +1,132 @@ +<?php + +namespace Drupal\Tests\Core\Installer; + +use Drupal\Core\Database\Connection; +use Drupal\Core\Database\Database; +use Drupal\Core\Database\DatabaseExceptionWrapper; +use Drupal\Core\Database\DatabaseNotFoundException; +use Drupal\Core\Database\Schema; +use Drupal\Core\Installer\InstallerRedirectTrait; +use Drupal\Tests\UnitTestCase; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; + +// The function drupal_installation_attempted() has to be included. +$root = dirname(dirname(dirname(dirname(dirname(dirname(__DIR__)))))); +require_once $root . '/core/includes/bootstrap.inc'; + +/** + * @coversDefaultClass \Drupal\Core\Installer\InstallerRedirectTrait + * + * @group Installer + */ +class InstallerRedirectTraitTest extends UnitTestCase { + + /** + * Data provider for testShouldRedirectToInstaller(). + * + * @return array + * - Expected result from shouldRedirectToInstaller(). + * - 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. + */ + public function providerShouldRedirectToInstaller() { + return array( + [TRUE, DatabaseNotFoundException::class, FALSE, FALSE], + [TRUE, DatabaseNotFoundException::class, TRUE, FALSE], + [TRUE, DatabaseNotFoundException::class, FALSE, TRUE], + [TRUE, DatabaseNotFoundException::class, TRUE, TRUE], + [TRUE, DatabaseNotFoundException::class, TRUE, TRUE, FALSE], + + [TRUE, \PDOException::class, FALSE, FALSE], + [TRUE, \PDOException::class, TRUE, FALSE], + [FALSE, \PDOException::class, FALSE, TRUE], + [FALSE, \PDOException::class, TRUE, TRUE], + [TRUE, \PDOException::class, TRUE, TRUE, FALSE], + + [TRUE, DatabaseExceptionWrapper::class, FALSE, FALSE], + [TRUE, DatabaseExceptionWrapper::class, TRUE, FALSE], + [FALSE, DatabaseExceptionWrapper::class, FALSE, TRUE], + [FALSE, DatabaseExceptionWrapper::class, TRUE, TRUE], + [TRUE, DatabaseExceptionWrapper::class, TRUE, TRUE, FALSE], + + [TRUE, NotFoundHttpException::class, FALSE, FALSE], + [TRUE, NotFoundHttpException::class, TRUE, FALSE], + [FALSE, NotFoundHttpException::class, FALSE, TRUE], + [FALSE, NotFoundHttpException::class, TRUE, TRUE], + [TRUE, NotFoundHttpException::class, TRUE, TRUE, FALSE], + + [FALSE, \Exception::class, FALSE, FALSE], + [FALSE, \Exception::class, TRUE, FALSE], + [FALSE, \Exception::class, FALSE, TRUE], + [FALSE, \Exception::class, TRUE, TRUE], + [FALSE, \Exception::class, TRUE, TRUE, FALSE], + ); + } + + /** + * @covers ::shouldRedirectToInstaller + * @dataProvider providerShouldRedirectToInstaller + */ + function testShouldRedirectToInstaller($expected, $exception, $connection, $connection_info, $session_table_exists = TRUE) { + try { + throw new $exception(); + } + catch (\Exception $e) { + // Mock the trait. + $trait = $this->getMockBuilder(InstallerRedirectTrait::class) + ->setMethods(array('isCli')) + ->getMockForTrait(); + + // Make sure that the method thinks we are not using the cli. + $trait->expects($this->any()) + ->method('isCli') + ->willReturn(FALSE); + + // Un-protect the method using reflection. + $method_ref = new \ReflectionMethod($trait, 'shouldRedirectToInstaller'); + $method_ref->setAccessible(TRUE); + + // Mock the database connection info. + $db = $this->getMockForAbstractClass(Database::class); + $property_ref = new \ReflectionProperty($db, 'databaseInfo'); + $property_ref->setAccessible(TRUE); + $property_ref->setValue($db, ['default' => $connection_info]); + + if ($connection) { + // Mock the database connection. + $connection = $this->getMockBuilder(Connection::class) + ->disableOriginalConstructor() + ->setMethods(array('schema')) + ->getMockForAbstractClass(); + + if ($connection_info) { + // Mock the database schema class. + $schema = $this->getMockBuilder(Schema::class) + ->disableOriginalConstructor() + ->setMethods(array('tableExists')) + ->getMockForAbstractClass(); + + $schema->expects($this->any()) + ->method('tableExists') + ->with('sessions') + ->willReturn($session_table_exists); + + $connection->expects($this->any()) + ->method('schema') + ->willReturn($schema); + } + } + else { + // Set the database connection if there is none. + $connection = NULL; + } + + // Call shouldRedirectToInstaller. + $this->assertSame($expected, $method_ref->invoke($trait, $e, $connection)); + } + } + +} -- GitLab