XSS vulnerability when displaying exception backtrace

Here's the fix and an example that triggers an exception containing unsafe HTML in method arguments.

Starting on a test.

I'm having trouble writing a test for this. My test case is very simple: I'm throwing an exception in a custom controller, then let HttpKernel handle the request and check if its output is correctly escaped.

I'm hitting a problem though, for some bizarre reason the call to error_log() in ExceptionLoggingSubscriber::onError() is not caught by the custom error handler that is set in TestBase.

  public function onError(GetResponseForExceptionEvent $event) {
    // ...
    $is_critical = !$exception instanceof HttpExceptionInterface || $exception->getStatusCode() >= 500;
    if ($is_critical) {
      // ... at this point if I check set_error_handler() it is set correctly to \Drupal\system\Tests\Routing\ExceptionHandlingTest::errorHandler().
      // ... but for some bizarre reason the following line is not handled by it.
      error_log(sprintf('Uncaught PHP Exception %s: "%s" at %s line %s', get_class($exception), $exception->getMessage(), $exception->getFile(), $exception->getLine()));
    }
  }

This error ends up in the error.log file in the simpletest folder. It will be picked up here in TestBase::restoreEnvironment():

    // In case a fatal error occurred that was not in the test process read the
    // log to pick up any fatal errors.
    simpletest_log_read($this->testId, $this->databasePrefix, get_class($this));

At this point my best guess is to implement a tearDown() and remove that line from the error log. That will make the test green.

Here's my work in progress. This is failing atm.

Thanks a lot @znerol, your suggestion works perfectly!

I have updated the patch. If this gets committed, please provide credit to znerol for his invaluable help :)

Hmm that seems a bit much, it's evident that plain text strings should be escaped before displaying them in HTML. I like @dawehner's idea to just document it inline.

I was wondering, is it necessary to add the subscriber back after removing it? In case additional tests are added later, some potential test failures might not be detected if they are not logged.

Updated documentation as suggested.