From b878dde667393f17c71d2297b6772a1d7070383c Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Tue, 19 Nov 2024 15:54:40 +1000
Subject: [PATCH] Revert "Issue #3486972 by catch, longwave:
DefaultExceptionHtmlSubscriber should not clone the request for
400/BadRequestException"
This reverts commit a4dbd3f2d3df1a81bd0718caeb2bce2fa0171ddf.
---
.../DefaultExceptionHtmlSubscriber.php | 10 ---
.../ExceptionLoggingSubscriberTest.php | 74 ++++++++++---------
2 files changed, 41 insertions(+), 43 deletions(-)
diff --git a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php
index 4642d434fcf6..615f2ee3c01d 100644
--- a/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php
+++ b/core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php
@@ -92,16 +92,6 @@ public function on4xx(ExceptionEvent $event) {
}
}
- /**
- * Handles a 400 error for HTML.
- *
- * @param \Symfony\Component\HttpKernel\Event\ExceptionEvent $event
- * The event to process.
- */
- public function on400(ExceptionEvent $event): void {
- throw $event->getThrowable();
- }
-
/**
* Handles a 401 error for HTML.
*
diff --git a/core/tests/Drupal/KernelTests/Core/EventSubscriber/ExceptionLoggingSubscriberTest.php b/core/tests/Drupal/KernelTests/Core/EventSubscriber/ExceptionLoggingSubscriberTest.php
index 77507fbf8c4a..785a52c981be 100644
--- a/core/tests/Drupal/KernelTests/Core/EventSubscriber/ExceptionLoggingSubscriberTest.php
+++ b/core/tests/Drupal/KernelTests/Core/EventSubscriber/ExceptionLoggingSubscriberTest.php
@@ -9,7 +9,6 @@
use Drupal\KernelTests\KernelTestBase;
use Symfony\Component\ErrorHandler\BufferingLogger;
use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Exception\HttpException;
/**
* Tests that HTTP exceptions are logged correctly.
@@ -32,55 +31,64 @@ class ExceptionLoggingSubscriberTest extends KernelTestBase {
/**
* Tests \Drupal\Core\EventSubscriber\ExceptionLoggingSubscriber::onException().
- *
- * @dataProvider exceptionDataProvider
*/
- public function testExceptionLogging(int $error_code, string $channel, int $log_level, string $exception = ''): void {
+ public function testExceptionLogging(): void {
$http_kernel = \Drupal::service('http_kernel');
+ $channel_map = [
+ 400 => 'client error',
+ 401 => 'client error',
+ 403 => 'access denied',
+ 404 => 'page not found',
+ 405 => 'client error',
+ 408 => 'client error',
+ // Do not check the 500 status code here because it would be caught by
+ // Drupal\Core\EventSubscriberExceptionTestSiteSubscriber which has lower
+ // priority.
+ 501 => 'php',
+ 502 => 'php',
+ 503 => 'php',
+ ];
+
+ $level_map = [
+ 400 => RfcLogLevel::WARNING,
+ 401 => RfcLogLevel::WARNING,
+ 403 => RfcLogLevel::WARNING,
+ 404 => RfcLogLevel::WARNING,
+ 405 => RfcLogLevel::WARNING,
+ 408 => RfcLogLevel::WARNING,
+ 501 => RfcLogLevel::ERROR,
+ 502 => RfcLogLevel::ERROR,
+ 503 => RfcLogLevel::ERROR,
+ ];
+
// Ensure that noting is logged.
$this->assertEmpty($this->container->get($this->testLogServiceName)->cleanLogs());
// Temporarily disable error log as the ExceptionLoggingSubscriber logs 5xx
// HTTP errors using error_log().
$error_log = ini_set('error_log', '/dev/null');
- $request = Request::create('/test-http-response-exception/' . $error_code);
-
- if ($exception) {
- $this->expectException($exception);
+ foreach ($channel_map as $code => $channel) {
+ $request = Request::create('/test-http-response-exception/' . $code);
+ $http_kernel->handle($request);
}
- $http_kernel->handle($request);
ini_set('error_log', $error_log);
+ $expected_channels = array_values($channel_map);
+ $expected_levels = array_values($level_map);
+
$logs = $this->container->get($this->testLogServiceName)->cleanLogs();
- $this->assertEquals($channel, $logs[0][2]['channel']);
- $this->assertEquals($log_level, $logs[0][0]);
+ foreach ($expected_channels as $key => $expected_channel) {
+ $this->assertEquals($expected_channel, $logs[$key][2]['channel']);
+ $this->assertEquals($expected_levels[$key], $logs[$key][0]);
- // Verify that @backtrace_string is removed from client error.
- if ($logs[0][2]['channel'] === 'client error') {
- $this->assertArrayNotHasKey('@backtrace_string', $logs[0][2]);
+ // Verify that @backtrace_string is removed from client error.
+ if ($logs[$key][2]['channel'] === 'client error') {
+ $this->assertArrayNotHasKey('@backtrace_string', $logs[$key][2]);
+ }
}
}
- public static function exceptionDataProvider(): array {
- return [
- // When a BadRequestException is thrown, DefaultHttpExceptionSubscriber
- // will rethrow the exception.
- [400, 'client error', RfcLogLevel::WARNING, HttpException::class],
- [401, 'client error', RfcLogLevel::WARNING],
- [403, 'access denied', RfcLogLevel::WARNING],
- [404, 'page not found', RfcLogLevel::WARNING],
- [405, 'client error', RfcLogLevel::WARNING],
- [408, 'client error', RfcLogLevel::WARNING],
- // Do not check the 500 status code here because it would be caught by
- // Drupal\Core\EventSubscriberExceptionTestSiteSubscriber which has lower
- // priority.
- [501, 'php', RfcLogLevel::ERROR],
- [502, 'php', RfcLogLevel::ERROR],
- [503, 'php', RfcLogLevel::ERROR],
- ];
- }
-
/**
* {@inheritdoc}
*/
--
GitLab