From d78cde2640762a1c916306ed832e238526ba0b3c Mon Sep 17 00:00:00 2001 From: Karl Bullock Date: Tue, 8 Sep 2026 00:08:39 +0000 Subject: [PATCH] fix: make URLs in informational emails clickable 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. --- framework/core/src/Mail/MailFormatter.php | 70 +++++++++++ .../mail/InformationalEmailLinksTest.php | 114 ++++++++++++++++++ .../email/html/information/generic.blade.php | 2 +- 3 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 framework/core/tests/integration/mail/InformationalEmailLinksTest.php diff --git a/framework/core/src/Mail/MailFormatter.php b/framework/core/src/Mail/MailFormatter.php index 09dbe99d5f..4b1c95bbd1 100644 --- a/framework/core/src/Mail/MailFormatter.php +++ b/framework/core/src/Mail/MailFormatter.php @@ -34,6 +34,76 @@ public function convert(?string $content): string return SafeSubstitution::restore($this->formatter->convert($content)); } + /** + * Turn plain text an email view was handed into HTML: escaped, with its + * line breaks kept and its URLs made clickable. + * + * Informational emails (account activation, email confirmation, password + * reset) do not build their body in a view. It arrives already translated, + * so its parameters, a display name among them, are part of the string by + * the time a template sees it, and no markers are left for + * {@see SafeSubstitution} to put back. Rendering such a body with + * `convert()` would therefore put those values in front of the parser, + * which is the thing the mail translator exists to prevent. The content is + * escaped instead, and only URLs are linked: the visible text of every link + * produced here is the address it points at. + */ + public function plainToHtml(?string $content): string + { + if (! $content) { + return ''; + } + + $paragraphs = array_filter( + preg_split('/\R{2,}/', trim($content)) ?: [], + fn (string $paragraph) => trim($paragraph) !== '' + ); + + return implode("\n", array_map( + fn (string $paragraph) => '

'.nl2br($this->linkUrls(trim($paragraph)), false).'

', + $paragraphs + )); + } + + /** + * Escape a run of plain text, wrapping every URL in it in an anchor. + * + * Escaping happens as the text is assembled rather than up front, so that + * an address containing `&` is escaped once (for the attribute and for the + * link text) instead of the entity being fed back through the matcher. + */ + private function linkUrls(string $text): string + { + $html = ''; + $offset = 0; + + preg_match_all('~\bhttps?://[^\s<>"]+~', $text, $matches, PREG_OFFSET_CAPTURE); + + foreach ($matches[0] as [$match, $position]) { + // Sentence punctuation after an address is not part of it. + $url = rtrim($match, '.,:;!?'); + + // Neither is a closing bracket that was never opened inside it, + // as in "(https://example.com/page)". + while (str_ends_with($url, ')') && substr_count($url, ')') > substr_count($url, '(')) { + $url = substr($url, 0, -1); + } + + $html .= $this->escape(substr($text, $offset, $position - $offset)); + $html .= ''.$this->escape($url).''; + $html .= $this->escape(substr($match, strlen($url))); + + $offset = $position + strlen($match); + } + + return $html.$this->escape(substr($text, $offset)); + } + + private function escape(string $text): string + { + return htmlspecialchars($text, ENT_QUOTES, 'UTF-8'); + } + /** * Anything else an email view might reach for goes to the real formatter. * diff --git a/framework/core/tests/integration/mail/InformationalEmailLinksTest.php b/framework/core/tests/integration/mail/InformationalEmailLinksTest.php new file mode 100644 index 0000000000..3b4560e0f8 --- /dev/null +++ b/framework/core/tests/integration/mail/InformationalEmailLinksTest.php @@ -0,0 +1,114 @@ +app()->getContainer()->make(Factory::class); + + // The data SendInformationalEmailJob shares before rendering. + $view->share([ + 'forumTitle' => 'Test Forum', + 'userEmail' => 'recipient@example.com', + 'title' => null, + 'username' => 'Recipient', + ]); + + return $view->make('mail::html.information.generic', compact('infoContent'))->render(); + } + + /** + * The `main-content` div is the body itself. The footer and the header hold + * links of their own, so matching the whole document would not tell us + * anything about the body. + */ + private function body(string $html): string + { + $this->assertMatchesRegularExpression('#
(.*?)
#s', $html); + preg_match('#
(.*?)
#s', $html, $matches); + + return trim($matches[1]); + } + + #[Test] + public function url_in_an_informational_email_is_a_link(): void + { + $body = $this->body($this->render('Click the following link:'."\n".self::URL)); + + $this->assertStringContainsString(''.self::URL.'', $body); + } + + #[Test] + public function line_breaks_in_an_informational_email_survive(): void + { + $body = $this->body($this->render("First paragraph.\n\nSecond line.\nThird line.")); + + // A blank line starts a paragraph, a single newline is a break. + $this->assertStringContainsString('

First paragraph.

', $body); + $this->assertMatchesRegularExpression('#Second line\.
\s*Third line\.#', $body); + } + + #[Test] + public function markup_in_an_informational_email_is_not_rendered(): void + { + // The body carries values that were substituted by the translator + // before the view saw them, a display name among them, so nothing in it + // may be treated as markup. + $body = $this->body($this->render('Hello [label](https://evil.example.com) & welcome.')); + + // The tags are shown, not applied. + $this->assertStringNotContainsString('', $body); + $this->assertStringContainsString('<b>', $body); + $this->assertStringContainsString('&', $body); + + // The address is linked, but the markdown around it is not: the link + // text is the address itself, never the label someone chose for it. + $this->assertStringNotContainsString('>label', $body); + $this->assertStringContainsString('[label](https://evil.example.com)', $body); + } + + #[Test] + public function punctuation_after_a_url_is_left_out_of_the_link(): void + { + $body = $this->body($this->render('Visit '.self::URL.'.')); + + $this->assertStringContainsString(''.self::URL.'.', $body); + } + + #[Test] + public function a_bracket_the_url_did_not_open_is_left_out_of_the_link(): void + { + $body = $this->body($this->render('Visit ('.self::URL.') today')); + + $this->assertStringContainsString('('.self::URL.')', $body); + } +} diff --git a/framework/core/views/email/html/information/generic.blade.php b/framework/core/views/email/html/information/generic.blade.php index c2196adf42..4d6c98b00c 100644 --- a/framework/core/views/email/html/information/generic.blade.php +++ b/framework/core/views/email/html/information/generic.blade.php @@ -1,3 +1,3 @@ - {{ $infoContent ?? '' }} + {!! $formatter->plainToHtml($infoContent ?? '') !!}