MAGETWO-57271: Modify escapeHtml function to filter not allowed attributes and tags

Igor Melnikov · Igor Melnikov · commit 624ee867d568 · 2016-09-02T12:36:14.000-05:00
Modifying function to filter not allowed tags and attributes
diff --git a/lib/internal/Magento/Framework/Escaper.php b/lib/internal/Magento/Framework/Escaper.php
@@ -23,12 +23,12 @@ class Escaper
     /**
      * @var string[]
      */
-    private $notAllowedTags = ['script', 'img'];
+    private $notAllowedTags = ['script', 'img', 'embed', 'iframe', 'video', 'source', 'object', 'audio'];
 
     /**
      * @var string[]
      */
-    private $allowedAttributes = ['id', 'class', 'href', 'target', 'title'];
+    private $allowedAttributes = ['id', 'class', 'href', 'target', 'title', 'style'];
 
     /**
      * @var string[]
@@ -59,7 +59,7 @@ public function escapeHtml($data, $allowedTags = null)
                     $this->getLogger()->critical(
                         'The following tag(s) are not allowed: ' . implode(', ', $notAllowedTags)
                     );
-                    return '';
+                    $allowedTags = array_diff($allowedTags, $this->notAllowedTags);
                 }
                 $wrapperElementId = uniqid();
                 $domDocument = new \DOMDocument('1.0', 'UTF-8');
@@ -76,7 +76,6 @@ function ($errorNumber, $errorString) {
                 } catch (\Exception $e) {
                     restore_error_handler();
                     $this->getLogger()->critical($e);
-                    return $this->escapeHtml($data);
                 }
                 restore_error_handler();
 
@@ -87,7 +86,7 @@ function ($errorNumber, $errorString) {
 
                 $result = mb_convert_encoding($domDocument->saveHTML(), 'UTF-8', 'HTML-ENTITIES');
                 preg_match('/<body id="' . $wrapperElementId . '">(.+)<\/body><\/html>$/si', $result, $matches);
-                return $matches[1];
+                return !empty($matches) ? $matches[1] : '';
             } else {
                 $result = htmlspecialchars($data, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8', false);
             }
diff --git a/lib/internal/Magento/Framework/Test/Unit/EscaperTest.php b/lib/internal/Magento/Framework/Test/Unit/EscaperTest.php
@@ -219,13 +219,18 @@ public function escapeHtmlDataProvider()
                 'allowedTags' => ['span', 'b'],
             ],
             'text with non ascii characters' => [
-                'data' => ['абвгд', 'مثال'],
-                'expected' => ['абвгд', 'مثال'],
+                'data' => ['абвгд', 'مثال', '幸福'],
+                'expected' => ['абвгд', 'مثال', '幸福'],
                 'allowedTags' => [],
             ],
             'html and body tags' => [
                 'data' => '<html><body><span>String</span></body></html>',
-                'expected' => '&lt;html&gt;&lt;body&gt;&lt;span&gt;String&lt;/span&gt;&lt;/body&gt;&lt;/html&gt;',
+                'expected' => '<span>String</span>',
+                'allowedTags' => ['span'],
+            ],
+            'invalid tag' => [
+                'data' => '<some tag> some text',
+                'expected' => ' some text',
                 'allowedTags' => ['span'],
             ],
         ];
@@ -239,12 +244,12 @@ public function escapeHtmlInvalidDataProvider()
         return [
             'text with allowed script tag' => [
                 'data' => '<span><script>some text in tags</script></span>',
-                'expected' => '',
+                'expected' => '<span>some text in tags</span>',
                 'allowedTags' => ['span', 'script'],
             ],
             'text with invalid html' => [
                 'data' => '<spa>n id="id1">Some string</span>',
-                'expected' => '&lt;spa&gt;n id=&quot;id1&quot;&gt;Some string&lt;/span&gt;',
+                'expected' => 'n id=&quot;id1&quot;&gt;Some string',
                 'allowedTags' => ['span'],
             ],
         ];
