Skip to content

[11.x] Fix EncodedHtmlString to ignore instance of HtmlString#55543

Merged
crynobone merged 8 commits into
laravel:11.xfrom
jbraband:laravel-11-encoded-html-string-fix
Apr 25, 2025
Merged

[11.x] Fix EncodedHtmlString to ignore instance of HtmlString#55543
crynobone merged 8 commits into
laravel:11.xfrom
jbraband:laravel-11-encoded-html-string-fix

Conversation

@jbraband

@jbraband jbraband commented Apr 24, 2025

Copy link
Copy Markdown
Contributor

I do not see existing tests for the sub-mail components (header, footer, table, etc) and so are excluded from this PR. The related PR #55149 also do not contain tests for similar changes to button, header, layout, message, and panel.


After updating to Laravel 11.44.3 yesterday, the mail components started rendering escaped HTML. Note that some mail components were fixed by @crynobone in #55149.

Here is a sample markdown template that demonstrates the 3 mail components included in this PR, subcopy, table, and footer

<x-mail::message subcopy="This is my subcopy">

<x-mail::table>
| Laravel       | Table         | Example       |
| ------------- | :-----------: | ------------: |
| Col 2 is      | Centered      | $10           |
| Col 3 is      | Right-Aligned | $20           |
</x-mail::table>

</x-mail::message>

The resulting rendered mailable.

image

Note: the closing } seen at the end of the sub-copy in the image has already been fixed by @crynobone in #55530 and is waiting for release next week

I am happy to makes changes or additions.

@jbraband

Copy link
Copy Markdown
Contributor Author

I should add that with this PR, the mailable renders properly

image

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
avoid double encoding

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
@crynobone

crynobone commented Apr 25, 2025

Copy link
Copy Markdown
Member

@jbraband I made some changes to how we solve this. Markdown::parse() should already return an instance of HtmlString and that shouldn't be encoded again.

Also add some integration tests to cover this usage


if ($value instanceof BackedEnum) {
$value = $value->value;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy the logic from e() function

function e($value, $doubleEncode = true)
{
if ($value instanceof DeferringDisplayableValue) {
$value = $value->resolveDisplayableValue();
}
if ($value instanceof Htmlable) {
return $value->toHtml();
}
if ($value instanceof BackedEnum) {
$value = $value->value;
}
return htmlspecialchars($value ?? '', ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8', $doubleEncode);
}

@jbraband

jbraband commented Apr 25, 2025

Copy link
Copy Markdown
Contributor Author

@crynobone I'm not at a computer at the moment, but one of the other templates in the mail folder had the parse method with the unescaped blade {!! !!}

@jbraband

Copy link
Copy Markdown
Contributor Author

@crynobone I'm not at a computer at the moment, but one of the other templates in the mail folder had the parse method with the unescaped blade {!! !!}

It's panel.blade.php @crynobone

@jbraband

Copy link
Copy Markdown
Contributor Author

@crynobone I'm not at a computer at the moment, but one of the other templates in the mail folder had the parse method with the unescaped blade {!! !!}

It's panel.blade.php @crynobone

Was able to commit it from mobile

@jbraband

Copy link
Copy Markdown
Contributor Author

I'll look to add a test for panel in the morning (CST)

Signed-off-by: Mior Muhammad Zaki <crynobone@gmail.com>
@crynobone crynobone changed the title [11.x] Do not escape slots in additional mail components [11.x] Fix EncodedHtmlString to ignore instance of HtmlString Apr 25, 2025
@crynobone
crynobone merged commit ec51a66 into laravel:11.x Apr 25, 2025
@jbraband

Copy link
Copy Markdown
Contributor Author

Thanks @crynobone!

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants