Issue #2504529 by joelpittet, alexpott, catch: SafeMarkup does not escape some filter tips - remove SafeMarkup usage from FilterHtml

core/modules/filter/src/Plugin/Filter/FilterHtml.php

@@ -7,8 +7,8 @@
 
 namespace Drupal\filter\Plugin\Filter;
 
-use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Core\Form\FormStateInterface;
+use Drupal\Component\Utility\Html;
 use Drupal\filter\FilterProcessResult;
 use Drupal\filter\Plugin\FilterBase;
 
@@ -102,7 +102,7 @@ class FilterHtml extends FilterBase {
     $output .= '<p>' . $this->t('This site allows HTML content. While learning all of HTML may feel intimidating, learning how to use a very small number of the most basic HTML "tags" is very easy. This table provides examples for each tag that is enabled on this site.') . '</p>';
     $output .= '<p>' . $this->t('For more information see W3C\'s <a href="@html-specifications">HTML Specifications</a> or use your favorite search engine to find other sites that explain HTML.', array('@html-specifications' => 'http://www.w3.org/TR/html/')) . '</p>';
     $tips = array(
-      'a' => array($this->t('Anchors are used to make links to other pages.'), '<a href="' . $base_url . '">' . SafeMarkup::checkPlain(\Drupal::config('system.site')->get('name')) . '</a>'),
+      'a' => array($this->t('Anchors are used to make links to other pages.'), '<a href="' . $base_url . '">' . Html::escape(\Drupal::config('system.site')->get('name')) . '</a>'),
       'br' => array($this->t('By default line break tags are automatically added, so use this tag to add additional ones. Use of this tag is different because it is not used with an open/close pair like all the others. Use the extra " /" inside the tag to maintain XHTML 1.0 compatibility'), $this->t('Text with <br />line break')),
       'p' => array($this->t('By default paragraph tags are automatically added, so use this tag to add additional ones.'), '<p>' . $this->t('Paragraph one.') . '</p> <p>' . $this->t('Paragraph two.') . '</p>'),
       'strong' => array($this->t('Strong', array(), array('context' => 'Font weight')), '<strong>' . $this->t('Strong', array(), array('context' => 'Font weight')) . '</strong>'),
@@ -144,8 +144,21 @@ class FilterHtml extends FilterBase {
       if (!empty($tips[$tag])) {
         $rows[] = array(
           array('data' => $tips[$tag][0], 'class' => array('description')),
-          array('data' => SafeMarkup::format('<code>@var</code>', array('@var' => $tips[$tag][1])), 'class' => array('type')),
-          array('data' => SafeMarkup::format($tips[$tag][1]), 'class' => array('get'))
+          // The markup must be escaped because this is the example code for the
+          // user.
+          array('data' =>
+            array(
+              '#prefix' => '<code>',
+              '#markup' => Html::escape($tips[$tag][1]),
+              '#suffix' => '</code>'
+            ),
+            'class' => array('type')),
+          // The markup must not be escaped because this is the example output
+          // for the user.
+          array('data' =>
+            array('#markup' => $tips[$tag][1]),
+            'class' => array('get'),
+          ),
         );
       }
       else {
@@ -175,8 +188,22 @@ class FilterHtml extends FilterBase {
     foreach ($entities as $entity) {
       $rows[] = array(
         array('data' => $entity[0], 'class' => array('description')),
-        array('data' => SafeMarkup::format('<code>@var</code>', array('@var' => $entity[1])), 'class' => array('type')),
-        array('data' => SafeMarkup::format($entity[1]), 'class' => array('get'))
+        // The markup must be escaped because this is the example code for the
+        // user.
+        array(
+          'data' => array(
+            '#prefix' => '<code>',
+            '#markup' => Html::escape($entity[1]),
+            '#suffix' => '</code>',
+          ),
+          'class' => array('type'),
+        ),
+        // The markup must not be escaped because this is the example output
+        // for the user.
+        array(
+          'data' => array('#markup' => $entity[1]),
+          'class' => array('get'),
+        ),
       );
     }
     $table = array(

core/modules/filter/src/Tests/FilterAdminTest.php

@@ -7,7 +7,7 @@
 
 namespace Drupal\filter\Tests;
 
-use Drupal\Component\Utility\SafeMarkup;
+use Drupal\Component\Utility\Html;
 use Drupal\Component\Utility\Unicode;
 use Drupal\simpletest\WebTestBase;
 use Drupal\user\RoleInterface;
@@ -312,7 +312,7 @@ class FilterAdminTest extends WebTestBase {
     $edit['body[0][format]'] = $plain;
     $this->drupalPostForm('node/' . $node->id() . '/edit', $edit, t('Save'));
     $this->drupalGet('node/' . $node->id());
-    $this->assertText(SafeMarkup::checkPlain($text), 'The "Plain text" text format escapes all HTML tags.');
+    $this->assertEscaped($text, 'The "Plain text" text format escapes all HTML tags.');
     $this->config('filter.settings')
       ->set('always_show_fallback_choice', FALSE)
       ->save();
@@ -368,12 +368,15 @@ class FilterAdminTest extends WebTestBase {
     $this->drupalLogin($this->adminUser);
     global $base_url;
 
+    $site_name_with_markup = 'Filter test <script>alert(\'here\');</script> site name';
+    $this->config('system.site')->set('name', $site_name_with_markup)->save();
+
     // It is not possible to test the whole filter tip page.
     // Therefore we test only some parts.
-    $link = '<a href="' . $base_url . '">' . SafeMarkup::checkPlain(\Drupal::config('system.site')->get('name')) . '</a>';
+    $link = '<a href="' . $base_url . '">' . Html::escape($site_name_with_markup) . '</a>';
     $ampersand = '&amp;';
-    $link_as_code = '<code>' . $link . '</code>';
-    $ampersand_as_code = '<code>' . $ampersand . '</code>';
+    $link_as_code = '<code>' . Html::escape($link) . '</code>';
+    $ampersand_as_code = '<code>' . Html::escape($ampersand) . '</code>';
 
     $this->drupalGet('filter/tips');