Commit 62a176da authored by catch's avatar catch
Browse files

Issue #2828706 by smustgrave, Chi, saidatom, dagmar, dawehner, mfb, catch,...

Issue #2828706 by smustgrave, Chi, saidatom, dagmar, dawehner, mfb, catch, larowlan, longwave, joshua1234511, Berdir, quietone, FeyP, slip, samiullah, johnwebdev, Primsi, ravi.shankar: ExceptionLoggingSubscriber should not log HTTP 4XX errors using PHP logger channel

(cherry picked from commit 528a2e38)
parent cded23dd
Loading
Loading
Loading
Loading
Loading
+21 −1
Original line number Diff line number Diff line
@@ -75,6 +75,22 @@ public function onError(ExceptionEvent $event) {
    }
  }

  /**
   * Log 4xx errors that are not 403 or 404.
   *
   * @param \Symfony\Component\HttpKernel\Event\ExceptionEvent $event
   *   The event to process.
   */
  public function onClientError(ExceptionEvent $event) {
    $exception = $event->getThrowable();
    $error = Error::decodeException($exception);
    $error += [
      'status_code' => $exception->getStatusCode(),
    ];
    $this->logger->get('client error')
      ->log($error['severity_level'], Error::DEFAULT_ERROR_MESSAGE, $error);
  }

  /**
   * Log all exceptions.
   *
@@ -88,10 +104,14 @@ public function onException(ExceptionEvent $event) {

    // Treat any non-HTTP exception as if it were one, so we log it the same.
    if ($exception instanceof HttpExceptionInterface) {
      $possible_method = 'on' . $exception->getStatusCode();
      $status_code = $exception->getStatusCode();
      $possible_method = 'on' . $status_code;
      if (method_exists($this, $possible_method)) {
        $method = $possible_method;
      }
      elseif ($status_code >= 400 && $status_code < 500) {
        $method = 'onClientError';
      }
    }

    $this->$method($event);
+81 −0
Original line number Diff line number Diff line
<?php

namespace Drupal\KernelTests\Core\EventSubscriber;

use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\KernelTests\KernelTestBase;
use Symfony\Component\ErrorHandler\BufferingLogger;
use Symfony\Component\HttpFoundation\Request;

/**
 * Tests that HTTP exceptions are logged correctly.
 *
 * @group system
 */
class ExceptionLoggingSubscriberTest extends KernelTestBase {

  /**
   * The service name for a logger implementation that collects anything logged.
   *
   * @var string
   */
  protected $testLogServiceName = 'exception_logging_subscriber_test.logger';

  /**
   * {@inheritdoc}
   */
  protected static $modules = ['system', 'test_page_test'];

  /**
   * Tests \Drupal\Core\EventSubscriber\ExceptionLoggingSubscriber::onException().
   */
  public function testExceptionLogging() {
    $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',
    ];

    // 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');
    foreach ($channel_map as $code => $channel) {
      $request = Request::create('/test-http-response-exception/' . $code);
      $http_kernel->handle($request);
    }
    ini_set('error_log', $error_log);

    $expected_channels = array_values($channel_map);

    $logs = $this->container->get($this->testLogServiceName)->cleanLogs();
    foreach ($expected_channels as $key => $expected_channel) {
      $log_message = $logs[$key][2]['channel'];
      $this->assertEquals($expected_channel, $log_message);
    }
  }

  /**
   * {@inheritdoc}
   */
  public function register(ContainerBuilder $container) {
    parent::register($container);
    $container
      ->register($this->testLogServiceName, BufferingLogger::class)
      ->addTag('logger');
  }

}