From e6444bb57e60d56648ea60cf6b30f737c052e2c1 Mon Sep 17 00:00:00 2001 From: Aidan Woods Date: Thu, 15 Mar 2018 10:42:29 +0000 Subject: [PATCH 1/6] Add unsafeHtml option for extensions to use on trusted input --- Parsedown.php | 21 +++++++++++++++++++-- test/ParsedownTest.php | 12 ++++++++++++ test/UnsafeExtension.php | 14 ++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 test/UnsafeExtension.php diff --git a/Parsedown.php b/Parsedown.php index 2725170..b274f52 100644 --- a/Parsedown.php +++ b/Parsedown.php @@ -1488,7 +1488,20 @@ class Parsedown } } + $unsafeHtml = false; if (isset($Element['text'])) + { + $text = $Element['text']; + } + // very strongly consider an alternative if you're writing an + // extension + elseif (isset($Element['unsafeHtml']) and !$this->safeMode) + { + $text = $Element['unsafeHtml']; + $unsafeHtml = true; + } + + if (isset($text)) { $markup .= $hasName ? '>' : ''; @@ -1499,11 +1512,15 @@ class Parsedown if (isset($Element['handler'])) { - $markup .= $this->{$Element['handler']}($Element['text'], $Element['nonNestables']); + $markup .= $this->{$Element['handler']}($text, $Element['nonNestables']); + } + elseif ($unsafeHtml !== true or $this->safeMode) + { + $markup .= self::escape($text, true); } else { - $markup .= self::escape($Element['text'], true); + $markup .= $text; } $markup .= $hasName ? '' : ''; diff --git a/test/ParsedownTest.php b/test/ParsedownTest.php index c28cedf..3cd796e 100644 --- a/test/ParsedownTest.php +++ b/test/ParsedownTest.php @@ -1,4 +1,5 @@ assertEquals($expectedMarkup, $actualMarkup); } + function testUnsafeHtml() + { + $markdown = "```php\nfoobar\n```"; + $expectedMarkup = '
foobar
'; + + $unsafeExtension = new UnsafeExtension; + $actualMarkup = $unsafeExtension->text($markdown); + + $this->assertEquals($expectedMarkup, $actualMarkup); + } + function data() { $data = array(); diff --git a/test/UnsafeExtension.php b/test/UnsafeExtension.php new file mode 100644 index 0000000..f2343c4 --- /dev/null +++ b/test/UnsafeExtension.php @@ -0,0 +1,14 @@ +$text

"; + + return $Block; + } +} From e4c5be026d4c776261edf5e8ef802e258bfaee9f Mon Sep 17 00:00:00 2001 From: Aidan Woods Date: Thu, 15 Mar 2018 11:00:03 +0000 Subject: [PATCH 2/6] Further attempt to dissuade this feature's use --- test/UnsafeExtension.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/UnsafeExtension.php b/test/UnsafeExtension.php index f2343c4..9a8dcc7 100644 --- a/test/UnsafeExtension.php +++ b/test/UnsafeExtension.php @@ -7,6 +7,11 @@ class UnsafeExtension extends Parsedown $text = $Block['element']['text']['text']; unset($Block['element']['text']['text']); + // WARNING: There is almost always a better way of doing things! + // + // This example is one of them, unsafe behaviour is NOT needed here. + // Only use this if you trust the input and have no idea what + // the output HTML will look like (e.g. using an external parser). $Block['element']['text']['unsafeHtml'] = "

$text

"; return $Block; From ef7ed7b66cf22b268c459a98e4fe3f7f809d40b5 Mon Sep 17 00:00:00 2001 From: Aidan Woods Date: Thu, 15 Mar 2018 11:09:55 +0000 Subject: [PATCH 3/6] Still grab the text if safe mode enabled, but output it escaped --- Parsedown.php | 3 ++- test/ParsedownTest.php | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Parsedown.php b/Parsedown.php index b274f52..9558525 100644 --- a/Parsedown.php +++ b/Parsedown.php @@ -1495,9 +1495,10 @@ class Parsedown } // very strongly consider an alternative if you're writing an // extension - elseif (isset($Element['unsafeHtml']) and !$this->safeMode) + elseif (isset($Element['unsafeHtml'])) { $text = $Element['unsafeHtml']; + $unsafeHtml = true; } diff --git a/test/ParsedownTest.php b/test/ParsedownTest.php index 3cd796e..8f3e6c8 100644 --- a/test/ParsedownTest.php +++ b/test/ParsedownTest.php @@ -60,11 +60,17 @@ class ParsedownTest extends TestCase { $markdown = "```php\nfoobar\n```"; $expectedMarkup = '
foobar
'; + $expectedSafeMarkup = '
<p>foobar</p>
'; $unsafeExtension = new UnsafeExtension; $actualMarkup = $unsafeExtension->text($markdown); $this->assertEquals($expectedMarkup, $actualMarkup); + + $unsafeExtension->setSafeMode(true); + $actualSafeMarkup = $unsafeExtension->text($markdown); + + $this->assertEquals($expectedSafeMarkup, $actualSafeMarkup); } function data() From 3fc54bc966caea29a633dba41ebb6728a917ee67 Mon Sep 17 00:00:00 2001 From: Aidan Woods Date: Thu, 15 Mar 2018 19:46:03 +0000 Subject: [PATCH 4/6] Allow extension to "vouch" for raw HTML they produce Rename "unsafeHtml" to "rawHtml" --- Parsedown.php | 25 ++++++++++++++++++++----- test/ParsedownTest.php | 21 +++++++++++++++++++-- test/SampleExtensions.php | 39 +++++++++++++++++++++++++++++++++++++++ test/UnsafeExtension.php | 19 ------------------- 4 files changed, 78 insertions(+), 26 deletions(-) create mode 100644 test/SampleExtensions.php delete mode 100644 test/UnsafeExtension.php diff --git a/Parsedown.php b/Parsedown.php index 9558525..160594e 100644 --- a/Parsedown.php +++ b/Parsedown.php @@ -1488,18 +1488,33 @@ class Parsedown } } - $unsafeHtml = false; + $permitRawHtml = false; + if (isset($Element['text'])) { $text = $Element['text']; } // very strongly consider an alternative if you're writing an // extension - elseif (isset($Element['unsafeHtml'])) + elseif (isset($Element['rawHtml'])) { - $text = $Element['unsafeHtml']; + $text = $Element['rawHtml']; - $unsafeHtml = true; + $allowRawHtmlInSafeMode = false; + + if (isset($Element['allowRawHtmlInSafeMode'])) + { + $allowRawHtmlInSafeMode = (true === $Element['allowRawHtmlInSafeMode']); + } + + if ($this->safeMode !== true) + { + $permitRawHtml = true; + } + elseif ($this->safeMode and $allowRawHtmlInSafeMode) + { + $permitRawHtml = true; + } } if (isset($text)) @@ -1515,7 +1530,7 @@ class Parsedown { $markup .= $this->{$Element['handler']}($text, $Element['nonNestables']); } - elseif ($unsafeHtml !== true or $this->safeMode) + elseif ($permitRawHtml !== true) { $markup .= self::escape($text, true); } diff --git a/test/ParsedownTest.php b/test/ParsedownTest.php index 8f3e6c8..cc0cc1d 100644 --- a/test/ParsedownTest.php +++ b/test/ParsedownTest.php @@ -1,5 +1,5 @@ assertEquals($expectedMarkup, $actualMarkup); } - function testUnsafeHtml() + function testRawHtml() { $markdown = "```php\nfoobar\n```"; $expectedMarkup = '
foobar
'; @@ -73,6 +73,23 @@ class ParsedownTest extends TestCase $this->assertEquals($expectedSafeMarkup, $actualSafeMarkup); } + function testTrustDelegatedRawHtml() + { + $markdown = "```php\nfoobar\n```"; + $expectedMarkup = '
foobar
'; + $expectedSafeMarkup = $expectedMarkup; + + $unsafeExtension = new TrustDelegatedExtension; + $actualMarkup = $unsafeExtension->text($markdown); + + $this->assertEquals($expectedMarkup, $actualMarkup); + + $unsafeExtension->setSafeMode(true); + $actualSafeMarkup = $unsafeExtension->text($markdown); + + $this->assertEquals($expectedSafeMarkup, $actualSafeMarkup); + } + function data() { $data = array(); diff --git a/test/SampleExtensions.php b/test/SampleExtensions.php new file mode 100644 index 0000000..6d7ec9f --- /dev/null +++ b/test/SampleExtensions.php @@ -0,0 +1,39 @@ +$text

"; + + return $Block; + } +} + + +class TrustDelegatedExtension extends Parsedown +{ + protected function blockFencedCodeComplete($Block) + { + $text = $Block['element']['text']['text']; + unset($Block['element']['text']['text']); + + // WARNING: There is almost always a better way of doing things! + // + // This example is one of them, unsafe behaviour is NOT needed here. + // Only use this if you trust the input and have no idea what + // the output HTML will look like (e.g. using an external parser). + $Block['element']['text']['rawHtml'] = "

$text

"; + $Block['element']['text']['allowRawHtmlInSafeMode'] = true; + + return $Block; + } +} diff --git a/test/UnsafeExtension.php b/test/UnsafeExtension.php deleted file mode 100644 index 9a8dcc7..0000000 --- a/test/UnsafeExtension.php +++ /dev/null @@ -1,19 +0,0 @@ -$text

"; - - return $Block; - } -} From 624a08b7eb8c3a4b0b99e7175251b76483e387cf Mon Sep 17 00:00:00 2001 From: Aidan Woods Date: Thu, 15 Mar 2018 19:55:33 +0000 Subject: [PATCH 5/6] Update commment --- test/SampleExtensions.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/SampleExtensions.php b/test/SampleExtensions.php index 6d7ec9f..c66190c 100644 --- a/test/SampleExtensions.php +++ b/test/SampleExtensions.php @@ -28,9 +28,10 @@ class TrustDelegatedExtension extends Parsedown // WARNING: There is almost always a better way of doing things! // - // This example is one of them, unsafe behaviour is NOT needed here. - // Only use this if you trust the input and have no idea what - // the output HTML will look like (e.g. using an external parser). + // This behaviour is NOT needed in the demonstrated case. + // Only use this if you are sure that the result being added into + // rawHtml is safe. + // (e.g. using an external parser with escaping capabilities). $Block['element']['text']['rawHtml'] = "

$text

"; $Block['element']['text']['allowRawHtmlInSafeMode'] = true; From 88dc94989039d37e09f186e94ec3a3964af82b36 Mon Sep 17 00:00:00 2001 From: Aidan Woods Date: Sun, 18 Mar 2018 19:42:14 +0000 Subject: [PATCH 6/6] Refactor based on suggestion by @PhrozenByte --- Parsedown.php | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/Parsedown.php b/Parsedown.php index 160594e..bf08700 100644 --- a/Parsedown.php +++ b/Parsedown.php @@ -1500,21 +1500,8 @@ class Parsedown { $text = $Element['rawHtml']; - $allowRawHtmlInSafeMode = false; - - if (isset($Element['allowRawHtmlInSafeMode'])) - { - $allowRawHtmlInSafeMode = (true === $Element['allowRawHtmlInSafeMode']); - } - - if ($this->safeMode !== true) - { - $permitRawHtml = true; - } - elseif ($this->safeMode and $allowRawHtmlInSafeMode) - { - $permitRawHtml = true; - } + $allowRawHtmlInSafeMode = isset($Element['allowRawHtmlInSafeMode']) && $Element['allowRawHtmlInSafeMode']; + $permitRawHtml = !$this->safeMode || $allowRawHtmlInSafeMode; } if (isset($text)) @@ -1530,7 +1517,7 @@ class Parsedown { $markup .= $this->{$Element['handler']}($text, $Element['nonNestables']); } - elseif ($permitRawHtml !== true) + elseif (!$permitRawHtml) { $markup .= self::escape($text, true); }