Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #2501319 by joelpittet, cwells, stefan.r, YesCT, nlisgo, Cottser, cmanalansan, dawehner, xjm, alexpott, josephdpurcell: Remove SafeMarkup::set() in Error::renderExceptionSafe() and _drupal_log_error() and improve backtrace formatting consistency in _drupal_log_error(), Error::renderExceptionSafe(), and DefaultExceptionSubscriber::onHtml()

8.0.x
Alex Pott 2015-08-20 09:56:42 +01:00
parent b728936c03
commit bb2d279978
16 changed files with 80 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
core/includes/bootstrap.inc
View File

@ -405,10 +405,7 @@ function watchdog_exception($type, Exception $exception, $message = NULL, $varia
// Use a default value if $message is not set.
if (empty($message)) {
// The exception message is run through
// \Drupal\Component\Utility\SafeMarkup::checkPlain() by
// \Drupal\Core\Utility\Error:decodeException().
$message = '%type: !message in %function (line %line of %file).';
$message = '%type: @message in %function (line %line of %file).';
}
if ($link) {

36
core/includes/errors.inc
View File

@ -8,6 +8,7 @@
use Drupal\Component\Utility\SafeMarkup;
use Drupal\Component\Utility\Xss;
use Drupal\Core\Logger\RfcLogLevel;
use Drupal\Core\Render\SafeString;
use Drupal\Core\Utility\Error;
use Symfony\Component\HttpFoundation\Response;
@ -68,7 +69,7 @@ function _drupal_error_handler_real($error_level, $message, $filename, $line, $c
'%type' => isset($types[$error_level]) ? $severity_msg : 'Unknown error',
// The standard PHP error handler considers that the error messages
// are HTML. We mimick this behavior here.
'!message' => Xss::filterAdmin($message),
'@message' => SafeString::create(Xss::filterAdmin($message)),
'%function' => $caller['function'],
'%file' => $caller['file'],
'%line' => $caller['line'],
@ -109,9 +110,9 @@ function error_displayable($error = NULL) {
* Logs a PHP error or exception and displays an error page in fatal cases.
*
* @param $error
* An array with the following keys: %type, !message, %function, %file,
* An array with the following keys: %type, @message, %function, %file,
* %line, severity_level, and backtrace. All the parameters are plain-text,
* with the exception of !message, which needs to be a safe HTML string, and
* with the exception of @message, which needs to be an HTML string, and
* backtrace, which is a standard PHP backtrace.
* @param $fatal
* TRUE if the error is fatal.
@ -130,7 +131,7 @@ function _drupal_log_error($error, $fatal = FALSE) {
// as it uniquely identifies each PHP error.
static $number = 0;
$assertion = array(
$error['!message'],
$error['@message'],
$error['%type'],
array(
'function' => $error['%function'],
@ -154,12 +155,12 @@ function _drupal_log_error($error, $fatal = FALSE) {
// installer.
if (\Drupal::hasService('logger.factory')) {
try {
\Drupal::logger('php')->log($error['severity_level'], '%type: !message in %function (line %line of %file).', $error);
\Drupal::logger('php')->log($error['severity_level'], '%type: @message in %function (line %line of %file).', $error);
}
catch (\Exception $e) {
// We can't log, for example because the database connection is not
// available. At least try to log to PHP error log.
error_log(strtr('Failed to log error: %type: !message in %function (line %line of %file).', $error));
error_log(strtr('Failed to log error: %type: @message in %function (line %line of %file).', $error));
}
}
@ -167,7 +168,7 @@ function _drupal_log_error($error, $fatal = FALSE) {
if ($fatal) {
// When called from CLI, simply output a plain text message.
// Should not translate the string to avoid errors producing more errors.
$response->setContent(html_entity_decode(strip_tags(format_string('%type: !message in %function (line %line of %file).', $error))). "\n");
$response->setContent(html_entity_decode(strip_tags(SafeMarkup::format('%type: @message in %function (line %line of %file).', $error))). "\n");
$response->send();
exit;
}
@ -178,7 +179,7 @@ function _drupal_log_error($error, $fatal = FALSE) {
if (error_displayable($error)) {
// When called from JavaScript, simply output the error message.
// Should not translate the string to avoid errors producing more errors.
$response->setContent(format_string('%type: !message in %function (line %line of %file).', $error));
$response->setContent(SafeMarkup::format('%type: @message in %function (line %line of %file).', $error));
$response->send();
}
exit;
@ -208,20 +209,29 @@ function _drupal_log_error($error, $fatal = FALSE) {
$error['%file'] = substr($error['%file'], $root_length + 1);
}
}
// Should not translate the string to avoid errors producing more errors.
$message = format_string('%type: !message in %function (line %line of %file).', $error);
// Check if verbose error reporting is on.
$error_level = _drupal_get_error_level();
if ($error_level == ERROR_REPORTING_DISPLAY_VERBOSE) {
if ($error_level != ERROR_REPORTING_DISPLAY_VERBOSE) {
// Without verbose logging, use a simple message.
// We call SafeMarkup::format() directly here, rather than use t() since
// we are in the middle of error handling, and we don't want t() to
// cause further errors.
$message = SafeMarkup::format('%type: @message in %function (line %line of %file).', $error);
}
else {
// With verbose logging, we will also include a backtrace.
// First trace is the error itself, already contained in the message.
// While the second trace is the error source and also contained in the
// message, the message doesn't contain argument values, so we output it
// once more in the backtrace.
array_shift($backtrace);
// Generate a backtrace containing only scalar argument values.
$message .= '<pre class="backtrace">' . Error::formatBacktrace($backtrace) . '</pre>';
$error['@backtrace'] = Error::formatBacktrace($backtrace);
$message = SafeMarkup::format('%type: @message in %function (line %line of %file). <pre class="backtrace">@backtrace</pre>', $error);
}
}
@ -252,7 +262,7 @@ function _drupal_log_error($error, $fatal = FALSE) {
if ($message) {
if (\Drupal::hasService('session')) {
// Message display is dependent on sessions being available.
drupal_set_message(SafeMarkup::set($message), $class, TRUE);
drupal_set_message($message, $class, TRUE);
}
else {
print $message;

10
core/includes/update.inc
View File

@ -188,10 +188,7 @@ function update_do_one($module, $number, $dependency_map, &$context) {
$variables = Error::decodeException($e);
unset($variables['backtrace']);
// The exception message is run through
// \Drupal\Component\Utility\SafeMarkup::checkPlain() by
// \Drupal\Core\Utility\Error::decodeException().
$ret['#abort'] = array('success' => FALSE, 'query' => t('%type: !message in %function (line %line of %file).', $variables));
$ret['#abort'] = array('success' => FALSE, 'query' => t('%type: @message in %function (line %line of %file).', $variables));
}
}
@ -235,10 +232,7 @@ function update_entity_definitions(&$context) {
watchdog_exception('update', $e);
$variables = Error::decodeException($e);
unset($variables['backtrace']);
// The exception message is run through
// \Drupal\Component\Utility\SafeMarkup::checkPlain() by
// \Drupal\Core\Utility\Error::decodeException().
$ret['#abort'] = array('success' => FALSE, 'query' => t('%type: !message in %function (line %line of %file).', $variables));
$ret['#abort'] = array('success' => FALSE, 'query' => t('%type: @message in %function (line %line of %file).', $variables));
$context['results']['core']['update_entity_definitions'] = $ret;
$context['results']['#abort'][] = 'update_entity_definitions';
}

2
core/lib/Drupal/Core/EventSubscriber/DefaultExceptionHtmlSubscriber.php
View File

@ -161,7 +161,7 @@ class DefaultExceptionHtmlSubscriber extends HttpExceptionSubscriberBase {
// just log it. The DefaultExceptionSubscriber will catch the original
// exception and handle it normally.
$error = Error::decodeException($e);
$this->logger->log($error['severity_level'], '%type: !message in %function (line %line of %file).', $error);
$this->logger->log($error['severity_level'], '%type: @message in %function (line %line of %file).', $error);
}
}
}

24
core/lib/Drupal/Core/EventSubscriber/DefaultExceptionSubscriber.php
View File

@ -91,12 +91,20 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface {
if (substr($error['%file'], 0, $root_length) == DRUPAL_ROOT) {
$error['%file'] = substr($error['%file'], $root_length + 1);
}
// Do not translate the string to avoid errors producing more errors.
unset($error['backtrace']);
$message = SafeMarkup::format('%type: !message in %function (line %line of %file).', $error);
// Check if verbose error reporting is on.
if ($this->getErrorLevel() == ERROR_REPORTING_DISPLAY_VERBOSE) {
unset($error['backtrace']);
if ($this->getErrorLevel() != ERROR_REPORTING_DISPLAY_VERBOSE) {
// Without verbose logging, use a simple message.
// We call SafeMarkup::format directly here, rather than use t() since
// we are in the middle of error handling, and we don't want t() to
// cause further errors.
$message = SafeMarkup::format('%type: @message in %function (line %line of %file).', $error);
}
else {
// With verbose logging, we will also include a backtrace.
$backtrace_exception = $exception;
while ($backtrace_exception->getPrevious()) {
$backtrace_exception = $backtrace_exception->getPrevious();
@ -108,9 +116,9 @@ class DefaultExceptionSubscriber implements EventSubscriberInterface {
// once more in the backtrace.
array_shift($backtrace);
// Generate a backtrace containing only scalar argument values. Make
// sure the backtrace is escaped as it can contain user submitted data.
$message .= '<pre class="backtrace">' . SafeMarkup::escape(Error::formatBacktrace($backtrace)) . '</pre>';
// Generate a backtrace containing only scalar argument values.
$error['@backtrace'] = Error::formatBacktrace($backtrace);
$message = SafeMarkup::format('%type: @message in %function (line %line of %file). <pre class="backtrace">@backtrace</pre>', $error);
}
}

2
core/lib/Drupal/Core/EventSubscriber/ExceptionLoggingSubscriber.php
View File

@ -68,7 +68,7 @@ class ExceptionLoggingSubscriber implements EventSubscriberInterface {
public function onError(GetResponseForExceptionEvent $event) {
$exception = $event->getException();
$error = Error::decodeException($exception);
$this->logger->get('php')->log($error['severity_level'], '%type: !message in %function (line %line of %file).', $error);
$this->logger->get('php')->log($error['severity_level'], '%type: @message in %function (line %line of %file).', $error);
$is_critical = !$exception instanceof HttpExceptionInterface || $exception->getStatusCode() >= 500;
if ($is_critical) {

2
core/lib/Drupal/Core/EventSubscriber/ExceptionTestSiteSubscriber.php
View File

@ -51,7 +51,7 @@ class ExceptionTestSiteSubscriber extends HttpExceptionSubscriberBase {
// as it uniquely identifies each PHP error.
static $number = 0;
$assertion = array(
$error['!message'],
$error['@message'],
$error['%type'],
array(
'function' => $error['%function'],

7
core/lib/Drupal/Core/Utility/Error.php
View File

@ -70,7 +70,7 @@ class Error {
'%type' => get_class($exception),
// The standard PHP exception handler considers that the exception message
// is plain-text. We mimic this behavior here.
'!message' => SafeMarkup::checkPlain($message),
'@message' => $message,
'%function' => $caller['function'],
'%file' => $caller['file'],
'%line' => $caller['line'],
@ -95,14 +95,13 @@ class Error {
// Remove 'main()'.
array_shift($backtrace);
$output = SafeMarkup::format('%type: !message in %function (line %line of %file).', $decode);
// Even though it is possible that this method is called on a public-facing
// site, it is only called when the exception handler itself threw an
// exception, which normally means that a code change caused the system to
// no longer function correctly (as opposed to a user-triggered error), so
// we assume that it is safe to include a verbose backtrace.
$output .= '<pre>' . static::formatBacktrace($backtrace) . '</pre>';
return SafeMarkup::set($output);
$decode['@backtrace'] = Error::formatBacktrace($backtrace);
return SafeMarkup::format('%type: @message in %function (line %line of %file). <pre class="backtrace">@backtrace</pre>', $decode);
}
/**

2
core/modules/migrate/src/MigrateExecutable.php
View File

@ -405,7 +405,7 @@ class MigrateExecutable implements MigrateExecutableInterface {
*/
protected function handleException(\Exception $exception, $save = TRUE) {
$result = Error::decodeException($exception);
$message = $result['!message'] . ' (' . $result['%file'] . ':' . $result['%line'] . ')';
$message = $result['@message'] . ' (' . $result['%file'] . ':' . $result['%line'] . ')';
if ($save) {
$this->saveMessage($message);
}

2
core/modules/node/src/Tests/NodeCreationTest.php
View File

@ -203,7 +203,7 @@ class NodeCreationTest extends NodeTestBase {
$query = db_query("SELECT wid, variables FROM {watchdog}");
foreach ($query as $row) {
$variables = (array) unserialize($row->variables);
if (isset($variables['!message']) && $variables['!message'] === 'Test exception for rollback.') {
if (isset($variables['@message']) && $variables['@message'] === 'Test exception for rollback.') {
$matches[] = $row->wid;
}
}

6
core/modules/simpletest/src/TestBase.php
View File

@ -1384,12 +1384,10 @@ abstract class TestBase {
'line' => $exception->getLine(),
'file' => $exception->getFile(),
));
// \Drupal\Core\Utility\Error::decodeException() runs the exception
// message through \Drupal\Component\Utility\SafeMarkup::checkPlain().
$decoded_exception = Error::decodeException($exception);
unset($decoded_exception['backtrace']);
$message = SafeMarkup::format('%type: !message in %function (line %line of %file). <pre class="backtrace">!backtrace</pre>', $decoded_exception + array(
'!backtrace' => Error::formatBacktrace($verbose_backtrace),
$message = SafeMarkup::format('%type: @message in %function (line %line of %file). <pre class="backtrace">@backtrace</pre>', $decoded_exception + array(
'@backtrace' => Error::formatBacktrace($verbose_backtrace),
));
$this->error($message, 'Uncaught exception', Error::getLastCaller($backtrace));
}

2
core/modules/system/src/Tests/Common/SimpleTestErrorCollectorTest.php
View File

@ -44,7 +44,7 @@ class SimpleTestErrorCollectorTest extends WebTestBase {
if (count($this->collectedErrors) == 3) {
$this->assertError($this->collectedErrors[0], 'Notice', 'Drupal\error_test\Controller\ErrorTestController->generateWarnings()', 'ErrorTestController.php', 'Undefined variable: bananas');
$this->assertError($this->collectedErrors[1], 'Warning', 'Drupal\error_test\Controller\ErrorTestController->generateWarnings()', 'ErrorTestController.php', 'Division by zero');
$this->assertError($this->collectedErrors[2], 'User warning', 'Drupal\error_test\Controller\ErrorTestController->generateWarnings()', 'ErrorTestController.php', 'Drupal is awesome');
$this->assertError($this->collectedErrors[2], 'User warning', 'Drupal\error_test\Controller\ErrorTestController->generateWarnings()', 'ErrorTestController.php', 'Drupal &amp; awesome');
}
else {
// Give back the errors to the log report.

34
core/modules/system/src/Tests/System/ErrorHandlerTest.php
View File

@ -30,32 +30,32 @@ class ErrorHandlerTest extends WebTestBase {
$config = $this->config('system.logging');
$error_notice = array(
'%type' => 'Notice',
'!message' => 'Undefined variable: bananas',
'@message' => 'Undefined variable: bananas',
'%function' => 'Drupal\error_test\Controller\ErrorTestController->generateWarnings()',
'%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
);
$error_warning = array(
'%type' => 'Warning',
'!message' => 'Division by zero',
'@message' => 'Division by zero',
'%function' => 'Drupal\error_test\Controller\ErrorTestController->generateWarnings()',
'%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
);
$error_user_notice = array(
'%type' => 'User warning',
'!message' => 'Drupal is awesome',
'@message' => 'Drupal & awesome',
'%function' => 'Drupal\error_test\Controller\ErrorTestController->generateWarnings()',
'%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
);
$fatal_error = array(
'%type' => 'Recoverable fatal error',
'%function' => 'Drupal\error_test\Controller\ErrorTestController->Drupal\error_test\Controller\{closure}()',
'!message' => 'Argument 1 passed to Drupal\error_test\Controller\ErrorTestController::Drupal\error_test\Controller\{closure}() must be of the type array, string given, called in ' . \Drupal::root() . '/core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php on line 66 and defined',
'@message' => 'Argument 1 passed to Drupal\error_test\Controller\ErrorTestController::Drupal\error_test\Controller\{closure}() must be of the type array, string given, called in ' . \Drupal::root() . '/core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php on line 66 and defined',
);
if (version_compare(PHP_VERSION, '7.0.0-dev') >= 0) {
// In PHP 7, instead of a recoverable fatal error we get a TypeError.
$fatal_error['%type'] = 'TypeError';
// The error message also changes in PHP 7.
$fatal_error['!message'] = 'Argument 1 passed to Drupal\error_test\Controller\ErrorTestController::Drupal\error_test\Controller\{closure}() must be of the type array, string given, called in ' . \Drupal::root() . '/core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php on line 66';
$fatal_error['@message'] = 'Argument 1 passed to Drupal\error_test\Controller\ErrorTestController::Drupal\error_test\Controller\{closure}() must be of the type array, string given, called in ' . \Drupal::root() . '/core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php on line 66';
}
// Set error reporting to display verbose notices.
@ -66,6 +66,9 @@ class ErrorHandlerTest extends WebTestBase {
$this->assertErrorMessage($error_warning);
$this->assertErrorMessage($error_user_notice);
$this->assertRaw('<pre class="backtrace">', 'Found pre element with backtrace class.');
// Ensure we are escaping but not double escaping.
$this->assertRaw('&amp;');
$this->assertNoRaw('&amp;amp;');
// Set error reporting to display verbose notices.
$this->config('system.logging')->set('error_level', ERROR_REPORTING_DISPLAY_VERBOSE)->save();
@ -73,6 +76,9 @@ class ErrorHandlerTest extends WebTestBase {
$this->assertResponse(500, 'Received expected HTTP status code.');
$this->assertErrorMessage($fatal_error);
$this->assertRaw('<pre class="backtrace">', 'Found pre element with backtrace class.');
// Ensure we are escaping but not double escaping.
$this->assertRaw('&#039;');
$this->assertNoRaw('&amp;#039;');
// Remove the recoverable fatal error from the assertions, it's wanted here.
// Ensure that we just remove this one recoverable fatal error (in PHP 7 this
@ -124,21 +130,21 @@ class ErrorHandlerTest extends WebTestBase {
$error_exception = array(
'%type' => 'Exception',
'!message' => 'Drupal is awesome',
'@message' => 'Drupal & awesome',
'%function' => 'Drupal\error_test\Controller\ErrorTestController->triggerException()',
'%line' => 56,
'%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
);
$error_pdo_exception = array(
'%type' => 'DatabaseExceptionWrapper',
'!message' => 'SELECT * FROM bananas_are_awesome',
'@message' => 'SELECT * FROM bananas_are_awesome',
'%function' => 'Drupal\error_test\Controller\ErrorTestController->triggerPDOException()',
'%line' => 64,
'%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
);
$error_renderer_exception = array(
'%type' => 'Exception',
'!message' => 'This is an exception that occurs during rendering',
'@message' => 'This is an exception that occurs during rendering',
'%function' => 'Drupal\error_test\Controller\ErrorTestController->Drupal\error_test\Controller\{closure}()',
'%line' => 82,
'%file' => drupal_get_path('module', 'error_test') . '/error_test.module',
@ -153,9 +159,9 @@ class ErrorHandlerTest extends WebTestBase {
// We cannot use assertErrorMessage() since the exact error reported
// varies from database to database. Check that the SQL string is displayed.
$this->assertText($error_pdo_exception['%type'], format_string('Found %type in error page.', $error_pdo_exception));
$this->assertText($error_pdo_exception['!message'], format_string('Found !message in error page.', $error_pdo_exception));
$this->assertText($error_pdo_exception['@message'], format_string('Found @message in error page.', $error_pdo_exception));
$error_details = format_string('in %function (line ', $error_pdo_exception);
$this->assertRaw($error_details, format_string("Found '!message' in error page.", array('!message' => $error_details)));
$this->assertRaw($error_details, format_string("Found '@message' in error page.", array('@message' => $error_details)));
$this->drupalGet('error-test/trigger-renderer-exception');
$this->assertTrue(strpos($this->drupalGetHeader(':status'), '500 Service unavailable (with message)'), 'Received expected HTTP status line.');
@ -181,16 +187,16 @@ class ErrorHandlerTest extends WebTestBase {
* Helper function: assert that the error message is found.
*/
function assertErrorMessage(array $error) {
$message = t('%type: !message in %function (line ', $error);
$this->assertRaw($message, format_string('Found error message: !message.', array('!message' => $message)));
$message = t('%type: @message in %function (line ', $error);
$this->assertRaw($message, format_string('Found error message: @message.', array('@message' => $message)));
}
/**
* Helper function: assert that the error message is not found.
*/
function assertNoErrorMessage(array $error) {
$message = t('%type: !message in %function (line ', $error);
$this->assertNoRaw($message, format_string('Did not find error message: !message.', array('!message' => $message)));
$message = t('%type: @message in %function (line ', $error);
$this->assertNoRaw($message, format_string('Did not find error message: @message.', array('@message' => $message)));
}
/**

2
core/modules/system/tests/modules/error_service_test/src/Logger/TestLog.php
View File

@ -24,7 +24,7 @@ class TestLog implements LoggerInterface {
public function log($level, $message, array $context = array()) {
$trigger = [
'%type' => 'Exception',
'!message' => 'Deforestation',
'@message' => 'Deforestation',
'%function' => 'Drupal\error_service_test\MonkeysInTheControlRoom->handle()',
'severity_level' => 3,
'channel' => 'php',

6
core/modules/system/tests/modules/error_test/src/Controller/ErrorTestController.php
View File

@ -51,8 +51,8 @@ class ErrorTestController extends ControllerBase {
$monkey_love = $bananas;
// This will generate a warning.
$awesomely_big = 1/0;
// This will generate a user error.
trigger_error("Drupal is awesome", E_USER_WARNING);
// This will generate a user error. Use & to check for double escaping.
trigger_error("Drupal & awesome", E_USER_WARNING);
return [];
}
@ -72,7 +72,7 @@ class ErrorTestController extends ControllerBase {
*/
public function triggerException() {
define('SIMPLETEST_COLLECT_ERRORS', FALSE);
throw new \Exception("Drupal is awesome");
throw new \Exception("Drupal & awesome");
}
/**

6
core/modules/views/src/Tests/Plugin/ArgumentDefaultTest.php
View File

@ -94,11 +94,11 @@ class ArgumentDefaultTest extends PluginTestBase {
// Note, the undefined index error has two spaces after it.
$error = array(
'%type' => 'Notice',
'!message' => 'Undefined index: ' . $argument_type,
'@message' => 'Undefined index: ' . $argument_type,
'%function' => 'views_handler_argument->validateOptionsForm()',
);
$message = t('%type: !message in %function', $error);
$this->assertNoRaw($message, t('Did not find error message: !message.', array('!message' => $message)));
$message = t('%type: @message in %function', $error);
$this->assertNoRaw($message, format_string('Did not find error message: @message.', array('@message' => $message)));
}
/**