Skip to content

Commit

Permalink
[6.x] Use CommonMark For Mailable (#30982)
Browse files Browse the repository at this point in the history
This switches Markdown Mailable parsing to CommonMark instead of Parsedown. The main reason for this is better granularity of security related features in CommonMark, which allows us to prevent unsafe links in user input (a user input with the content: "a") without needing to prevent all HTML tags in general which would break our current mailable system.

I think it would be good to do this basically security related change on 6.x because it is an LTS.
  • Loading branch information
taylorotwell committed Dec 30, 2019
2 parents 0ba0569 + a68d163 commit 9308a4e
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 6 deletions.
3 changes: 2 additions & 1 deletion composer.json
Expand Up @@ -22,7 +22,8 @@
"doctrine/inflector": "^1.1",
"dragonmantank/cron-expression": "^2.0",
"egulias/email-validator": "^2.1.10",
"erusev/parsedown": "^1.7",
"league/commonmark": "^1.1",
"league/commonmark-ext-table": "^2.1",
"league/flysystem": "^1.0.8",
"monolog/monolog": "^1.12|^2.0",
"nesbot/carbon": "^2.0",
Expand Down
14 changes: 11 additions & 3 deletions src/Illuminate/Mail/Markdown.php
Expand Up @@ -5,7 +5,9 @@
use Illuminate\Contracts\View\Factory as ViewFactory;
use Illuminate\Support\HtmlString;
use Illuminate\Support\Str;
use Parsedown;
use League\CommonMark\CommonMarkConverter;
use League\CommonMark\Environment;
use League\CommonMark\Ext\Table\TableExtension;
use TijsVerkoyen\CssToInlineStyles\CssToInlineStyles;

class Markdown
Expand Down Expand Up @@ -98,9 +100,15 @@ public function renderText($view, array $data = [])
*/
public static function parse($text)
{
$parsedown = new Parsedown;
$environment = Environment::createCommonMarkEnvironment();

return new HtmlString($parsedown->text($text));
$environment->addExtension(new TableExtension);

$converter = new CommonMarkConverter([
'allow_unsafe_links' => false,
], $environment);

return new HtmlString($converter->convertToHtml($text));
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/Illuminate/Mail/composer.json
Expand Up @@ -16,7 +16,8 @@
"require": {
"php": "^7.2",
"ext-json": "*",
"erusev/parsedown": "^1.7",
"league/commonmark": "^1.1",
"league/commonmark-ext-table": "^2.1",
"illuminate/container": "^6.0",
"illuminate/contracts": "^6.0",
"illuminate/support": "^6.0",
Expand Down
2 changes: 1 addition & 1 deletion tests/Mail/MailMarkdownTest.php
Expand Up @@ -66,6 +66,6 @@ public function testParseReturnsParsedMarkdown()

$result = $markdown->parse('# Something')->toHtml();

$this->assertSame('<h1>Something</h1>', $result);
$this->assertSame("<h1>Something</h1>\n", $result);
}
}

0 comments on commit 9308a4e

Please sign in to comment.