diff --git a/core/.phpstan-baseline.php b/core/.phpstan-baseline.php index c4283234b8679c5d8048f54feef75aa51498cdf2..c0a427eec1731a1ccb8a46f161cbd4d8e3e81684 100644 --- a/core/.phpstan-baseline.php +++ b/core/.phpstan-baseline.php @@ -2164,12 +2164,6 @@ 'count' => 1, 'path' => __DIR__ . '/tests/Drupal/Tests/Core/Config/ConfigTest.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Call to deprecated method expectError\\(\\) of class PHPUnit\\\\Framework\\\\TestCase\\: -https\\://github\\.com/sebastianbergmann/phpunit/issues/5062$#', - 'count' => 1, - 'path' => __DIR__ . '/tests/Drupal/Tests/Core/Database/ConditionTest.php', -]; $ignoreErrors[] = [ 'message' => '#^Trying to mock an undefined method getRevisionId\\(\\) on class Drupal\\\\Tests\\\\Core\\\\Entity\\\\UrlTestEntity\\.$#', 'count' => 1, diff --git a/core/lib/Drupal/Core/Database/Query/Condition.php b/core/lib/Drupal/Core/Database/Query/Condition.php index 377d45cb59d567e793045628cab02c9dec19a9d0..2e738bf916bc4b72a37fd67cb640b1efabd2d66c 100644 --- a/core/lib/Drupal/Core/Database/Query/Condition.php +++ b/core/lib/Drupal/Core/Database/Query/Condition.php @@ -255,14 +255,7 @@ public function compile(Connection $connection, PlaceholderInterface $queryPlace // Provide a string which will result into an empty query result. $this->stringVersion = '( AND 1 = 0 )'; - // Conceptually throwing an exception caused by user input is bad - // as you result into a 'white screen of death', which depending on - // your webserver configuration can result into the assumption that - // your site is broken. - // On top of that the database API relies on __toString() which - // does not allow to throw exceptions. - trigger_error('Invalid characters in query operator: ' . $condition['operator'], E_USER_ERROR); - return; + throw new InvalidQueryException('Invalid characters in query operator: ' . $condition['operator']); } // For simplicity, we convert all operators to a data structure to diff --git a/core/tests/Drupal/KernelTests/Core/Database/QueryTest.php b/core/tests/Drupal/KernelTests/Core/Database/QueryTest.php index 112c101cd2b21bccf897a6c575be7d04f2ab5bd3..5893197ef74ff2b8a872ed21ed821b42fadd506c 100644 --- a/core/tests/Drupal/KernelTests/Core/Database/QueryTest.php +++ b/core/tests/Drupal/KernelTests/Core/Database/QueryTest.php @@ -4,6 +4,8 @@ namespace Drupal\KernelTests\Core\Database; +use Drupal\Core\Database\InvalidQueryException; + /** * Tests Drupal's extended prepared statement syntax. * @@ -70,16 +72,6 @@ public function testArrayArgumentsSQLInjection() { public function testConditionOperatorArgumentsSQLInjection() { $injection = "IS NOT NULL) ;INSERT INTO {test} (name) VALUES ('test12345678'); -- "; - $previous_error_handler = set_error_handler(function ($severity, $message, $filename, $lineno) use (&$previous_error_handler) { - // Normalize the filename to use UNIX directory separators. - if (preg_match('@core/lib/Drupal/Core/Database/Query/Condition.php$@', str_replace(DIRECTORY_SEPARATOR, '/', $filename))) { - // Convert errors to exceptions for testing purposes below. - throw new \ErrorException($message, 0, $severity, $filename, $lineno); - } - if ($previous_error_handler) { - return $previous_error_handler($severity, $message, $filename, $lineno); - } - }); try { $result = $this->connection->select('test', 't') ->fields('t') @@ -87,7 +79,8 @@ public function testConditionOperatorArgumentsSQLInjection() { ->execute(); $this->fail('Should not be able to attempt SQL injection via condition operator.'); } - catch (\ErrorException $e) { + catch (InvalidQueryException $e) { + $this->assertSame("Invalid characters in query operator: $injection", $e->getMessage()); // Expected exception; just continue testing. } @@ -115,7 +108,8 @@ public function testConditionOperatorArgumentsSQLInjection() { ->execute(); $this->fail('Should not be able to attempt SQL injection via operator.'); } - catch (\ErrorException $e) { + catch (InvalidQueryException $e) { + $this->assertSame("Invalid characters in query operator: $injection", $e->getMessage()); // Expected exception; just continue testing. } @@ -132,10 +126,10 @@ public function testConditionOperatorArgumentsSQLInjection() { ->execute(); $this->fail('Should not be able to attempt SQL injection via operator.'); } - catch (\ErrorException $e) { + catch (InvalidQueryException $e) { + $this->assertSame("Invalid characters in query operator: $injection", $e->getMessage()); // Expected exception; just continue testing. } - restore_error_handler(); } /** diff --git a/core/tests/Drupal/Tests/Core/Database/ConditionTest.php b/core/tests/Drupal/Tests/Core/Database/ConditionTest.php index 0cd5b4fcd3c3f120ee6b70e87532187fbccb1381..908b6265028366f1df52965924f2783cd4bd4789 100644 --- a/core/tests/Drupal/Tests/Core/Database/ConditionTest.php +++ b/core/tests/Drupal/Tests/Core/Database/ConditionTest.php @@ -5,6 +5,7 @@ namespace Drupal\Tests\Core\Database; use Drupal\Core\Database\Connection; +use Drupal\Core\Database\InvalidQueryException; use Drupal\Core\Database\Query\Condition; use Drupal\Core\Database\Query\PlaceholderInterface; use Drupal\Tests\Core\Database\Stub\StubCondition; @@ -169,7 +170,8 @@ public function testCompileWithSqlInjectionForOperator($operator) { $condition = $connection->condition('AND'); $condition->condition('name', 'value', $operator); - $this->expectError(); + $this->expectException(InvalidQueryException::class); + $this->expectExceptionMessage('Invalid characters in query operator:'); $condition->compile($connection, $query_placeholder); }