Add a new html_input option

Fixes #253. The new `html_input` option allows 3 different behaviors: - `strip` HTML input - `allow` HTML input - `escape` HTML input
thephpleague · Jun 28, 2016 · 45f4bf2 · 45f4bf2
1 parent 4d3cc5f
commit 45f4bf2
Show file tree

Hide file tree

Showing 12 changed files with 216 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,11 @@ Updates should follow the [Keep a CHANGELOG](http://keepachangelog.com/) princip
 
 ## [Unreleased][unreleased]
 
+### Added
+ - Added new `html_input` option to handle untrusted HTML input (#253)
+   - The `safe` option is kept for backward compatibility and still controls if unsafe links are preserved or not
+   - To use the new option, set it to `strip`, `allow` or `escape`
+
 ## [0.13.4] - 2016-06-14
 
 ### Fixed

diff --git a/README.md b/README.md
@@ -62,7 +62,7 @@ $environment = Environment::createCommonMarkEnvironment();
 // For example:  $environment->addInlineParser(new TwitterHandleParser());
 
 // Define your configuration:
-$config = ['safe' => true];
+$config = ['html_input' => 'escape'];
 
 // Create the converter
 $converter = new CommonMarkConverter($config, $environment);

diff --git a/src/Block/Renderer/HtmlBlockRenderer.php b/src/Block/Renderer/HtmlBlockRenderer.php
@@ -17,6 +17,7 @@
 use League\CommonMark\Block\Element\AbstractBlock;
 use League\CommonMark\Block\Element\HtmlBlock;
 use League\CommonMark\ElementRendererInterface;
+use League\CommonMark\Environment;
 use League\CommonMark\Util\Configuration;
 use League\CommonMark\Util\ConfigurationAwareInterface;
 
@@ -40,10 +41,19 @@ public function render(AbstractBlock $block, ElementRendererInterface $htmlRende
             throw new \InvalidArgumentException('Incompatible block type: ' . get_class($block));
         }
 
+        // Kept for BC reasons
         if ($this->config->getConfig('safe') === true) {
             return '';
         }
 
+        if ($this->config->getConfig('html_input') === Environment::HTML_INPUT_STRIP) {
+            return '';
+        }
+
+        if ($this->config->getConfig('html_input') === Environment::HTML_INPUT_ESCAPE) {
+            return htmlentities($block->getStringContent(), ENT_NOQUOTES);
+        }
+
         return $block->getStringContent();
     }
 

diff --git a/src/Environment.php b/src/Environment.php
@@ -27,6 +27,10 @@
 
 class Environment
 {
+    const HTML_INPUT_STRIP = 'strip';
+    const HTML_INPUT_ESCAPE = 'escape';
+    const HTML_INPUT_ALLOW = 'allow';
+
     /**
      * @var ExtensionInterface[]
      */
@@ -479,6 +483,7 @@ public static function createCommonMarkEnvironment()
                 'soft_break'      => "\n",
             ],
             'safe' => false,
+            'html_input' => self::HTML_INPUT_ALLOW,
         ]);
 
         return $environment;

diff --git a/src/Inline/Renderer/HtmlInlineRenderer.php b/src/Inline/Renderer/HtmlInlineRenderer.php
@@ -15,6 +15,7 @@
 namespace League\CommonMark\Inline\Renderer;
 
 use League\CommonMark\ElementRendererInterface;
+use League\CommonMark\Environment;
 use League\CommonMark\Inline\Element\AbstractInline;
 use League\CommonMark\Inline\Element\HtmlInline;
 use League\CommonMark\Util\Configuration;
@@ -39,10 +40,19 @@ public function render(AbstractInline $inline, ElementRendererInterface $htmlRen
             throw new \InvalidArgumentException('Incompatible inline type: ' . get_class($inline));
         }
 
+        // Kept for BC reasons
         if ($this->config->getConfig('safe') === true) {
             return '';
         }
 
+        if ($this->config->getConfig('html_input') === Environment::HTML_INPUT_STRIP) {
+            return '';
+        }
+
+        if ($this->config->getConfig('html_input') === Environment::HTML_INPUT_ESCAPE) {
+            return htmlentities($inline->getContent(), ENT_NOQUOTES);
+        }
+
         return $inline->getContent();
     }
 

diff --git a/tests/functional/HtmlInputTest.php b/tests/functional/HtmlInputTest.php
@@ -0,0 +1,53 @@
+<?php
+
+namespace League\CommonMark\Tests\Functional;
+
+use League\CommonMark\CommonMarkConverter;
+use League\CommonMark\Environment;
+
+class HtmlInputTest extends \PHPUnit_Framework_TestCase
+{
+    public function testDefaultConfig()
+    {
+        $input = file_get_contents(__DIR__ . '/data/html_input/input.md');
+        $expectedOutput = trim(file_get_contents(__DIR__ . '/data/html_input/unsafe_output.html'));
+
+        $converter = new CommonMarkConverter();
+        $actualOutput = trim($converter->convertToHtml($input));
+
+        $this->assertEquals($expectedOutput, $actualOutput);
+    }
+
+    public function testAllowHtmlInputConfig()
+    {
+        $input = file_get_contents(__DIR__ . '/data/html_input/input.md');
+        $expectedOutput = trim(file_get_contents(__DIR__ . '/data/html_input/unsafe_output.html'));
+
+        $converter = new CommonMarkConverter(['html_input' => Environment::HTML_INPUT_ALLOW]);
+        $actualOutput = trim($converter->convertToHtml($input));
+
+        $this->assertEquals($expectedOutput, $actualOutput);
+    }
+
+    public function testEscapeHtmlInputConfig()
+    {
+        $input = file_get_contents(__DIR__ . '/data/html_input/input.md');
+        $expectedOutput = trim(file_get_contents(__DIR__ . '/data/html_input/escaped_output.html'));
+
+        $converter = new CommonMarkConverter(['html_input' => Environment::HTML_INPUT_ESCAPE]);
+        $actualOutput = trim($converter->convertToHtml($input));
+
+        $this->assertEquals($expectedOutput, $actualOutput);
+    }
+
+    public function testStripHtmlInputConfig()
+    {
+        $input = file_get_contents(__DIR__ . '/data/html_input/input.md');
+        $expectedOutput = trim(file_get_contents(__DIR__ . '/data/html_input/safe_output.html'));
+
+        $converter = new CommonMarkConverter(['html_input' => Environment::HTML_INPUT_STRIP]);
+        $actualOutput = trim($converter->convertToHtml($input));
+
+        $this->assertEquals($expectedOutput, $actualOutput);
+    }
+}
diff --git a/tests/functional/data/html_input/escaped_output.html b/tests/functional/data/html_input/escaped_output.html
@@ -0,0 +1,5 @@
+&lt;div onclick="alert('XSS')"&gt;click me&lt;/div&gt;
+&lt;script&gt;alert("XSS")&lt;/script&gt;
+&lt;script&gt;
+    alert("XSS");
+&lt;/script&gt;
diff --git a/tests/functional/data/html_input/input.md b/tests/functional/data/html_input/input.md
@@ -0,0 +1,7 @@
+<div onclick="alert('XSS')">click me</div>
+
+<script>alert("XSS")</script>
+
+<script>
+    alert("XSS");
+</script>
diff --git a/tests/functional/data/html_input/safe_output.html b/tests/functional/data/html_input/safe_output.html
@@ -0,0 +1,2 @@
+
+
diff --git a/tests/functional/data/html_input/unsafe_output.html b/tests/functional/data/html_input/unsafe_output.html
@@ -0,0 +1,5 @@
+<div onclick="alert('XSS')">click me</div>
+<script>alert("XSS")</script>
+<script>
+    alert("XSS");
+</script>
diff --git a/tests/unit/Block/Renderer/HtmlBlockRendererTest.php b/tests/unit/Block/Renderer/HtmlBlockRendererTest.php
@@ -16,6 +16,7 @@
 
 use League\CommonMark\Block\Element\HtmlBlock;
 use League\CommonMark\Block\Renderer\HtmlBlockRenderer;
+use League\CommonMark\Environment;
 use League\CommonMark\Tests\Unit\FakeHtmlRenderer;
 use League\CommonMark\Util\Configuration;
 
@@ -72,6 +73,72 @@ public function testRenderSafeMode()
         $this->assertEquals('', $result);
     }
 
+    public function testRenderAllowHtml()
+    {
+        $this->renderer->setConfiguration(new Configuration([
+            'html_input' => Environment::HTML_INPUT_ALLOW,
+        ]));
+
+        /** @var HtmlBlock|\PHPUnit_Framework_MockObject_MockObject $block */
+        $block = $this->getMockBuilder('League\CommonMark\Block\Element\HtmlBlock')
+            ->setConstructorArgs([HtmlBlock::TYPE_6_BLOCK_ELEMENT])
+            ->getMock();
+        $block->expects($this->any())
+            ->method('getStringContent')
+            ->will($this->returnValue('<button>Test</button>'));
+
+        $fakeRenderer = new FakeHtmlRenderer();
+
+        $result = $this->renderer->render($block, $fakeRenderer);
+
+        $this->assertInternalType('string', $result);
+        $this->assertContains('<button>Test</button>', $result);
+    }
+
+    public function testRenderEscapeHtml()
+    {
+        $this->renderer->setConfiguration(new Configuration([
+            'html_input' => Environment::HTML_INPUT_ESCAPE,
+        ]));
+
+        /** @var HtmlBlock|\PHPUnit_Framework_MockObject_MockObject $block */
+        $block = $this->getMockBuilder('League\CommonMark\Block\Element\HtmlBlock')
+            ->setConstructorArgs([HtmlBlock::TYPE_6_BLOCK_ELEMENT])
+            ->getMock();
+        $block->expects($this->any())
+            ->method('getStringContent')
+            ->will($this->returnValue('<button class="test">Test</button>'));
+
+        $fakeRenderer = new FakeHtmlRenderer();
+
+        $result = $this->renderer->render($block, $fakeRenderer);
+
+        $this->assertInternalType('string', $result);
+        $this->assertContains('&lt;button class="test"&gt;Test&lt;/button&gt;', $result);
+    }
+
+    public function testRenderStripHtml()
+    {
+        $this->renderer->setConfiguration(new Configuration([
+            'html_input' => Environment::HTML_INPUT_STRIP,
+        ]));
+
+        /** @var HtmlBlock|\PHPUnit_Framework_MockObject_MockObject $block */
+        $block = $this->getMockBuilder('League\CommonMark\Block\Element\HtmlBlock')
+            ->setConstructorArgs([HtmlBlock::TYPE_6_BLOCK_ELEMENT])
+            ->getMock();
+        $block->expects($this->any())
+            ->method('getStringContent')
+            ->will($this->returnValue('<button>Test</button>'));
+
+        $fakeRenderer = new FakeHtmlRenderer();
+
+        $result = $this->renderer->render($block, $fakeRenderer);
+
+        $this->assertInternalType('string', $result);
+        $this->assertEquals('', $result);
+    }
+
     /**
      * @expectedException \InvalidArgumentException
      */

diff --git a/tests/unit/Inline/Renderer/HtmlInlineRendererTest.php b/tests/unit/Inline/Renderer/HtmlInlineRendererTest.php
@@ -14,6 +14,7 @@
 
 namespace League\CommonMark\Tests\Unit\Inline\Renderer;
 
+use League\CommonMark\Environment;
 use League\CommonMark\Inline\Element\HtmlInline;
 use League\CommonMark\Inline\Renderer\HtmlInlineRenderer;
 use League\CommonMark\Tests\Unit\FakeHtmlRenderer;
@@ -58,6 +59,51 @@ public function testRenderSafeMode()
         $this->assertEquals('', $result);
     }
 
+    public function testRenderAllowHtml()
+    {
+        $this->renderer->setConfiguration(new Configuration([
+            'html_input' => Environment::HTML_INPUT_ALLOW,
+        ]));
+
+        $inline = new HtmlInline('<h1>Test</h1>');
+        $fakeRenderer = new FakeHtmlRenderer();
+
+        $result = $this->renderer->render($inline, $fakeRenderer);
+
+        $this->assertInternalType('string', $result);
+        $this->assertContains('<h1>Test</h1>', $result);
+    }
+
+    public function testRenderEscapeHtml()
+    {
+        $this->renderer->setConfiguration(new Configuration([
+            'html_input' => Environment::HTML_INPUT_ESCAPE,
+        ]));
+
+        $inline = new HtmlInline('<h1 class="test">Test</h1>');
+        $fakeRenderer = new FakeHtmlRenderer();
+
+        $result = $this->renderer->render($inline, $fakeRenderer);
+
+        $this->assertInternalType('string', $result);
+        $this->assertContains('&lt;h1 class="test"&gt;Test&lt;/h1&gt;', $result);
+    }
+
+    public function testRenderStripHtml()
+    {
+        $this->renderer->setConfiguration(new Configuration([
+            'html_input' => Environment::HTML_INPUT_STRIP,
+        ]));
+
+        $inline = new HtmlInline('<h1>Test</h1>');
+        $fakeRenderer = new FakeHtmlRenderer();
+
+        $result = $this->renderer->render($inline, $fakeRenderer);
+
+        $this->assertInternalType('string', $result);
+        $this->assertEquals('', $result);
+    }
+
     /**
      * @expectedException \InvalidArgumentException
      */