Remove SafeMarkup usage in debug - it is not necessary and since SafeMarkup has side effects just really odd.

Incorrect usage of SafeMarkup for escaping purposes only is a bug.

CommentFileSizeAuthor
#2 2545882.2.patch3.5 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2545882.2.patch3.5 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Review all usages of Xss::filter(), Xss::filterAdmin(), SafeMarkup::checkPlain() and SafeMarkup::escape() » Remove SafeMarkup::checkPlain() from debug()
dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/includes/common.inc
@@ -1376,12 +1376,13 @@ function _drupal_flush_css_js() {
+  $string = htmlspecialchars($string, ENT_QUOTES, 'UTF-8');

So the reason that we need to escape here is that code in _drupal_log_error() drupal_set_message(SafeMarkup::set($message), $class, TRUE); ?

I'm curious whether we could get rid of that SafeMarkup::set() call.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Relating to #4 we touched that bit in this patch here #2501319: Remove SafeMarkup::set() in Error::renderExceptionSafe() and _drupal_log_error() and improve backtrace formatting consistency in _drupal_log_error(), Error::renderExceptionSafe(), and DefaultExceptionSubscriber::onHtml ()

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Needs review » Closed (outdated)

debug() is now using Html::escape() which has similar behavior as suggested on the patch. I think this issue has been fixed somewhere else already.