Merge pull request #33199 from basvanpoppel/set-self-closing-tags-for-void-elements

Set self closing tags for void elements

Author: sivaschenko

dev/tests/integration/testsuite/Magento/Csp/CspUtilTest.php

@@ -30,7 +30,7 @@ public function testPhtmlHelper(): void
         $content = $this->getResponse()->getContent();
 
         $this->assertStringContainsString(
-            '<script src="http&#x3A;&#x2F;&#x2F;my.magento.com&#x2F;static&#x2F;script.js"/>',
+            '<script src="http&#x3A;&#x2F;&#x2F;my.magento.com&#x2F;static&#x2F;script.js"></script>',
             $content
         );
         $this->assertStringContainsString("<script>\n    let myVar = 1;\n</script>", $content);

dev/tests/integration/testsuite/Magento/Csp/Helper/InlineUtilTest.php

@@ -96,7 +96,7 @@ public function getTags(): array
                 'script',
                 ['src' => 'http://magento.com/static/some-script.js'],
                 null,
-                '<script src="http&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;some-script.js"/>',
+                '<script src="http&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;some-script.js"></script>',
                 [new FetchPolicy('script-src', false, ['http://magento.com'])]
             ],
             'inline-script' => [
@@ -209,7 +209,7 @@ public function getTags(): array
                 'iframe',
                 ['src' => 'http://magento.com/some-page'],
                 null,
-                '<iframe src="http&#x3A;&#x2F;&#x2F;magento.com&#x2F;some-page"/>',
+                '<iframe src="http&#x3A;&#x2F;&#x2F;magento.com&#x2F;some-page"></iframe>',
                 [new FetchPolicy('frame-src', false, ['http://magento.com'])]
             ],
             'remote-track' => [
@@ -230,21 +230,21 @@ public function getTags(): array
                 'video',
                 ['src' => 'https://magento.com/static/video.mp4'],
                 null,
-                '<video src="https&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;video.mp4"/>',
+                '<video src="https&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;video.mp4"></video>',
                 [new FetchPolicy('media-src', false, ['https://magento.com'])]
             ],
             'remote-audio' => [
                 'audio',
                 ['src' => 'https://magento.com/static/audio.mp3'],
                 null,
-                '<audio src="https&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;audio.mp3"/>',
+                '<audio src="https&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;audio.mp3"></audio>',
                 [new FetchPolicy('media-src', false, ['https://magento.com'])]
             ],
             'remote-object' => [
                 'object',
                 ['data' => 'http://magento.com/static/flash.swf'],
                 null,
-                '<object data="http&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;flash.swf"/>',
+                '<object data="http&#x3A;&#x2F;&#x2F;magento.com&#x2F;static&#x2F;flash.swf"></object>',
                 [new FetchPolicy('object-src', false, ['http://magento.com'])]
             ],
             'remote-embed' => [
@@ -259,7 +259,7 @@ public function getTags(): array
                 ['code' => 'SomeApplet.class', 'archive' => 'https://magento.com/applet/my-applet.jar'],
                 null,
                 '<applet code="SomeApplet.class" '
-                    . 'archive="https&#x3A;&#x2F;&#x2F;magento.com&#x2F;applet&#x2F;my-applet.jar"/>',
+                    . 'archive="https&#x3A;&#x2F;&#x2F;magento.com&#x2F;applet&#x2F;my-applet.jar"></applet>',
                 [new FetchPolicy('object-src', false, ['https://magento.com'])]
             ]
         ];
@@ -294,7 +294,7 @@ public function testSecureHtmlRenderer(): void
         $eventListener = $this->secureHtmlRenderer->renderEventListener('onclick', 'alert()');
 
         $this->assertEquals(
-            '<script src="https&#x3A;&#x2F;&#x2F;test.magento.com&#x2F;static&#x2F;script.js"/>',
+            '<script src="https&#x3A;&#x2F;&#x2F;test.magento.com&#x2F;static&#x2F;script.js"></script>',
             $scriptTag
         );
         $this->assertEquals(

dev/tests/integration/testsuite/Magento/Framework/View/Helper/SecureHtmlRendererTemplateTest.php

@@ -33,7 +33,7 @@ public function testTemplateUsage(): void
             $content
         );
         $this->assertStringContainsString(
-            '<script src="http&#x3A;&#x2F;&#x2F;my.magento.com&#x2F;static&#x2F;script.js"/>',
+            '<script src="http&#x3A;&#x2F;&#x2F;my.magento.com&#x2F;static&#x2F;script.js"></script>',
             $content
         );
         $this->assertStringContainsString(

lib/internal/Magento/Framework/View/Helper/SecureHtmlRender/HtmlRenderer.php

@@ -13,6 +13,31 @@
  */
 class HtmlRenderer
 {
+
+    /**
+     * List of void elements which require a self-closing tag and don't allow content
+     *
+     * @var array
+     */
+    private const VOID_ELEMENTS_MAP = [
+        'area' => true,
+        'base' => true,
+        'br' => true,
+        'col' => true,
+        'command' => true,
+        'embed' => true,
+        'hr' => true,
+        'img' => true,
+        'input' => true,
+        'keygen' => true,
+        'link' => true,
+        'meta' => true,
+        'param' => true,
+        'source' => true,
+        'track' => true,
+        'wbr' => true,
+    ];
+
     /**
      * @var Escaper
      */
@@ -49,10 +74,10 @@ public function renderTag(TagData $tagData): string
         }
 
         $html = '<' .$tagData->getTag() .$attributesHtml;
-        if ($content) {
-            $html .= '>' .$content .'</' .$tagData->getTag() .'>';
-        } else {
+        if (isset(self::VOID_ELEMENTS_MAP[$tagData->getTag()])) {
             $html .= '/>';
+        } else {
+            $html .= '>' .$content .'</' .$tagData->getTag() .'>';
         }
 
         return $html;

lib/internal/Magento/Framework/View/Test/Unit/Helper/SecureHtmlRenderer/HtmlRendererTest.php

@@ -0,0 +1,60 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Framework\View\Test\Unit\Helper\SecureHtmlRenderer;
+
+use Magento\Framework\Escaper;
+use Magento\Framework\View\Helper\SecureHtmlRender\HtmlRenderer;
+use Magento\Framework\View\Helper\SecureHtmlRender\TagData;
+use PHPUnit\Framework\TestCase;
+
+class HtmlRendererTest extends TestCase
+{
+    /**
+     * @return void
+     */
+    protected function setUp(): void
+    {
+        $this->escaperMock = $this->createMock(Escaper::class);
+    }
+
+    /**
+     * @covers \Magento\Framework\View\Helper\SecureHtmlRender\HtmlRenderer::renderTag
+     */
+    public function testRenderTag()
+    {
+        $helper = new HtmlRenderer($this->escaperMock);
+
+        /** Test void element to have closing tag */
+        $tag = new TagData('hr', [], null, true);
+        $this->assertEquals(
+            "<hr/>",
+            $helper->renderTag($tag)
+        );
+
+        /** Test void element to never have content */
+        $tag = new TagData('hr', [], 'content', false);
+        $this->assertEquals(
+            "<hr/>",
+            $helper->renderTag($tag)
+        );
+
+        /** Test any non-void element to not have a closing tag while not having content */
+        $tags = new TagData('script', [], null, false);
+        $this->assertEquals(
+            "<script></script>",
+            $helper->renderTag($tags)
+        );
+
+        /** Test any non-void element to not have a closing tag and allow content */
+        $tags = new TagData('script', [], 'content', false);
+        $this->assertEquals(
+            "<script>content</script>",
+            $helper->renderTag($tags)
+        );
+    }
+}