Framework: Markdown mail::panel shows RAW HTML

[MarekGogol]

• Laravel Version: v6.10.0
• PHP Version: 7.2.21 (Valet / Mac OS)
• Database Driver & Version:

Description:

Markdown parser @component('mail::panel') shows RAW HTML tags. This issue is not present in Laravel version v6.9.0 and bellow.

Steps To Reproduce:

1. Install fresh laravel installation and setup SMTP...
2. Create App/Mail/TestMail.php

...

public function build()
{
    return $this->subject('test markdown error')
                ->markdown('email');
}

1. Create resources/views/email.blade.php

@component('mail::message')
# Hello

@component('mail::panel')
This is the panel content.
@endcomponent

@endcomponent

1. Send email
   Mail::to('[email protected]')->send(new TestMail);

2. Email with markdown panel component will be broken.
   

[jochensengier]

I can confirm the issue on v6.10.0, will have a look and submit a PR if possible...

[BrandonSurowiec]

The markdown parser was switched to CommonMark under the hood.

[jochensengier]

Did some debugging and if I remove a part of the indenting from vendor\laravel\framework\src\Illuminate\Mail\resources\views\html\panel.blade.php it appears to work.

<table class="panel" width="100%" cellpadding="0" cellspacing="0" role="presentation">
    <tr>
        <td class="panel-content">
<!--REMOVE INDENTING FROM THIS PART-->
<table width="100%" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td class="panel-item">
    {{ Illuminate\Mail\Markdown::parse($slot) }}
</td>
</tr>
</table>
<!--END-->
        </td>
    </tr>
</table>

Otherwise CommonMark detects it as IndentedCode and wraps it in a PRE tag.
See: vendor\league\commonmark\src\Block\Renderer\IndentedCodeRenderer

[MarekGogol]

Did some debugging and if I remove a part of the indenting from vendor\laravel\framework\src\Illuminate\Mail\resources\views\html\panel.blade.php it appears to work.

<table class="panel" width="100%" cellpadding="0" cellspacing="0" role="presentation">
    <tr>
        <td class="panel-content">
<!--REMOVE INDENTING FROM THIS PART-->
<table width="100%" cellpadding="0" cellspacing="0" role="presentation">
<tr>
<td class="panel-item">
    {{ Illuminate\Mail\Markdown::parse($slot) }}
</td>
</tr>
</table>
<!--END-->
        </td>
    </tr>
</table>

Otherwise CommonMark detects it as IndentedCode and wraps it in a PRE tag.
See: vendor\league\commonmark\src\Block\Renderer\IndentedCodeRenderer

But we need to mention, that panel.blade.php could be extracted into application. If someone updates laravel, backward compactibility wont work, and bug will persist in all previous laravel apps with extracted panel template.

[jochensengier]

@MarekGogol my comment wasn't supposed to be interpreted as a fix, I'm just trying to pinpoint the problem and sharing my findings along the way...

[Cannonb4ll]

Can confirm issue.

[taylorotwell]

Will be fixed in next 6.x maintenance release.

[taylorotwell]

Fixed in 6.10.1.

[MarekGogol]

Fixed in 6.10.1.
https://github.com/laravel/framework/commit/f17e347b15e8d27b4e775a8f961bda083326ee8f

Thanks for fixing this @taylorotwell. But I think this is not the right solution for fixing this problem. Backward compatibility is broken with your new release v6.10.1 for all laravel users which have exported panel.blade.php into resources/views/vendor before version v6.10.1.

So this is not an appropriate fix. Or is it?

Check my previous comment.
https://github.com/laravel/framework/issues/31065#issuecomment-572031647

[evilangelmd]

@taylorotwell this is not fix. this is workaround. just a workaround.

[BnitoBzh]

@evilangelmd @taylorotwell Yes, just a workaround, because apps with custom emails and/or custom markdown components have the same issue

[driesvints]

We can't revert the parser changes because we had to swap it because it had an XSS vulnerability. There's really no other way around this, sorry.

[lukasmu]

@driesvints Wouldn't it be an option to remove the indentation at the beginning of a line (that causes the issues) before CommonMark kicks in? So similar to my proposal in https://github.com/laravel/framework/pull/31076, but then in a proper way (so that the tests do not fail)?

[driesvints]

@lukasmu the problem is that with the panel view, for example, views get parsed twice sometimes because of inheritance. Any indentation left in HTML files will be re-parsed.

[MarekGogol]

@driesvints And what if we bind render view callback on view instance from markdown parser, which could be applied to all recursive views renders.

I made an API method setViewsCallback for recursive callback into View.php, which will apply callback on the main view including every nested subviews.

I tried this solution a few days before, and It works. But I didn't send a pull request. What do you think guys about this?

See my commit in my fork repo:
https://github.com/MarekGogol/framework/commit/5d0b1d5e75542821bc6fcfe72eaefde0dc303e24

So, if we apply callback into render() method, callback will be applied only on main view render, not nested subviews. So this is not fix for this issue.
Illuminate/Mail/Markdown.php

$contents = $this->view->replaceNamespace(
    'mail', $this->htmlComponentPaths()
)->make($view, $data)->render(function($view, $contents){
    return preg_replace('/^\s+/m', '', $contents);
});

But if we create global callback, which will be applied on main template including every nested view like this, It will fix the issue.

$contents = $this->view->replaceNamespace(
    'mail', $this->htmlComponentPaths()
)->make($view, $data)->setViewsCallback(function($contents){
    return preg_replace('/^\s+/m', '', $contents);
})->render();

[driesvints]

@MarekGogol wouldn't that break things for people depending on the current implementation?

[MarekGogol]

@driesvints I don't think so. Would it?

Method setViewsCallback which I created in View.php is optionable, so any other components (as blade etc..) wont be affected. Maybe there is better way to do It, I don't know.

At least it will cause less damage in apps, for all developers... In actual "fix" behavior where all developers first need to find out that problem is not on their side, but on frameworks (it may cost them few hours to figure it out, if they know how to debug framework). And then they spend a couple of minutes on this issue, they won't be so happy...

I think we need to find out right solutions for this issue, because every day here is someone who has this problem.

[driesvints]

@MarekGogol please see the answer by Taylor here: https://github.com/laravel/framework/issues/31151#issuecomment-575619249

[MarekGogol]

@driesvints Sorry if I don't understand it well. Is this answer by Taylor directed to my commit https://github.com/MarekGogol/framework/commit/5d0b1d5e75542821bc6fcfe72eaefde0dc303e24 ? Because I haven't sent PR.

If we apply callback on Views layer as I created. It will be applied on every view which will be loaded from storage/framework/views. So any ident shouldn't left. Or which case wouldn't works? I can look at it.

[driesvints]

@MarekGogol yeah I think he meant your commit

[MarekGogol]

There is other PR https://github.com/laravel/framework/pull/31076 by @lukasmu which have similar solution to fixing this problem as I have... But this PR has problem, which may happen for users, who have included components into their markdown template by other way than parse() method in Markdown.php. Which is used for example in resources/views/footer.blade.php , subcopy.blade.php or any other parent component.

So if we apply global callback on whole view instance as I applied, It should be okay.

Or am I misstaken? @driesvints

[driesvints]

@MarekGogol honestly I think the best thing for you to do is to send in a PR, add a thorough description what you want to fix and definitely address the concerns Taylor told about. I don't know if it's going to be accepted or not but that's your best bet.

[MichMich]

Just for the record: this is still a issue in 6.12.0.

Code:

@component('mail::message')

@component('mail::panel')
This is the panel content.
@endcomponent

@endcomponent

Result:

[GrahamCampbell]

If you have published the views from the framework, you will need to update them.

[MichMich]

Thanks, php artisan optimize:clear solved the issue.

[saraswat19]

I tried to run

php artisan vendor:publish --tag=laravel-mail

but now got this new error

In VendorPublishCommand.php line 233:

Class 'League\Flysystem\MountManager' not found

[nat-mystudiosessions]

I realized my issue was I had published the vendor files to customize the blade templates.

php artisan vendor:publish --tag=laravel-mail --force worked for me.

Of course, if you made any modifications to the blade templates you'll have to redo them.

[MDaskalovAtBE]

Wow, Laravel 7.• and that problem still appears... Sooo I fixed it by NOT exporting the vendor files and not touching anything other than the blade file that I have created for the mail... Just remove all the indents, for example -
-> from

@component('mail::message')
    # Order Placed

    Your order has been placed!

    @component('mail::button', ['url' => $url])
        View Order
    @endcomponent

    You have paid: {{ $order->billing_total }}

    Thanks,<br>
    {{ config('app.name') }}
@endcomponent

to

@component('mail::message')
# Order Placed

Your order has been placed!

@component('mail::button', ['url' => $url])
View Order
@endcomponent

You have paid: {{ $order->billing_total }}

Thanks,<br>
{{ config('app.name') }}
@endcomponent

not the most beautiful file to go over after but it works with not much of a hacky way...
Hope it helps someone!

[ekntrtmz]

Removing the indents did not solve it for me.
I had to run:

php artisan vendor:publish --tag=laravel-mail --force

[ArminCoder]

Still an issue in Laravel 7.6.

[driesvints]

Hey everyone, as explained a couple of times already this isn't a bug but expected behavior. Because of the new Markdown parses in Laravel 7 templates need to be de-indented. See Taylor's response here: https://github.com/laravel/framework/issues/31151#issuecomment-575619249