[2.x] fix: make URLs in informational emails clickable - #5038
Open
karl-bullock wants to merge 1 commit into
Open
[2.x] fix: make URLs in informational emails clickable#5038karl-bullock wants to merge 1 commit into
karl-bullock wants to merge 1 commit into
Conversation
The HTML part of an informational email (account activation, email
confirmation, password reset, the admin test mail) printed its body with
`{{ }}`, so the address the reader is asked to visit arrived as text that
no mail client turns into a link, and the blank lines between its
paragraphs collapsed, since a newline is not a break in HTML.
The body is translated before it reaches the view, so its parameters, the
recipient's display name among them, are already substituted into the
string and carry no `SafeSubstitution` markers. Rendering it with
`convert()` would put those values in front of the parser, which is what
`MailTranslator` exists to prevent, so add `MailFormatter::plainToHtml()`
instead: it escapes the content, keeps its line breaks, and links URLs
with the address itself as the link text.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported at https://discuss.flarum.org/d/39814-urls-in-emails-should-be-clickable.
The problem
The HTML part of every informational email (account activation, email confirmation, password reset, the admin test mail) comes from
views/email/html/information/generic.blade.php, which printed the body with{{ }}:Two things follow. The address the reader is asked to visit arrives as text, so no mail client makes it a link and the reader has to select and copy it by hand. And the blank lines between the paragraphs vanish, because a newline is not a break in HTML, so the message renders as one run-on block.
That is what an activation email looks like on
2.xtoday, taken from a real forum withmail_driver=log:Why not
{!! $formatter->convert(...) !!}That is the obvious one-line fix, it is what the footer of the same component does, and it does linkify. It is not safe here.
An informational body is translated before it ever reaches a view:
AccountActivationMailerTrait::sendConfirmationEmail()and the other three callers build it with the ordinary translator and handSendInformationalEmailJoba finished string. Its parameters, the recipient's display name among them, are part of that string by the time the template sees it, and noSafeSubstitutionmarkers are left forMailFormatterto put back. Passing it toconvert()would therefore put those values in front of the parser, which is the thingMailTranslatorwas added to prevent: withflarum/markdownenabled, a display name of[Click here](https://example.com)picks its own link text and destination.The fix
MailFormatter::plainToHtml(), for content that reaches a view as plain text. It escapes, keeps the line breaks, and wraps URLs in anchors. The visible text of every link it produces is the address that link points at, so nothing in a body can name its own link text, and nothing in it is parsed as markup.Trailing sentence punctuation and a closing bracket the address never opened are left outside the link, so
(https://example.com/page).links what a reader means by it.Only the HTML view changes. The plain-text part already carried real newlines and a bare URL, which is right for plain text.
Tests
tests/integration/mail/InformationalEmailLinksTestrenders the real blade through the real view factory, the waySendInformationalEmailJobdoes: the URL is a link, the paragraphs and line breaks survive, markup in the body is escaped rather than rendered, and the two punctuation cases. Putting{{ $infoContent }}back fails all five.The full
flarum/coreintegration suite and PHPStan are green locally.