mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-11 22:38:54 +00:00
Stop the text half of an email printing its link twice in brackets
Laravel's notification view writes the subcopy URL as [$url]($url). The HTML half parses that into an anchor; the text half parses nothing, so it arrives as literal brackets around a duplicated address. With the button line above it the URL appeared three times in one message. It reads as broken, and it reads broken in a specific direction: a long opaque token, the recipient's address in the query string, and a duplicated link in brackets is the shape of a phishing template. On a password reset, which is often the first mail an installation ever sends somebody, from a domain with no reputation yet. Fixed the way every other component in that message already handles the same split -- one name, two files, Laravel picks per half. Which meant publishing the framework's view for a one-line change, so there is a note in it saying to re-copy on upgrade. Seen in a real reset mail, not in a test.
This commit is contained in:
@@ -13,6 +13,10 @@ Anything under **Upgrade notes** is something you have to do, not something we d
|
||||
This section collects changes as they land; the release process turns it into a numbered entry when
|
||||
a version is cut.
|
||||
|
||||
**Fixed**
|
||||
|
||||
- The plain-text version of an email no longer shows the link twice, wrapped in brackets.
|
||||
|
||||
## 2.2.0 — 27 August 2026
|
||||
|
||||
A big release. Most of it closes holes in who can see what. The rest is a handful of new things.
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
{{-- The action URL, spelled out for somebody who cannot click the button.
|
||||
|
||||
Paired with text/action-url.blade.php. Laravel resolves `mail::`
|
||||
components against html/ or text/ depending on which half of the
|
||||
message it is building, which is the whole reason this is a component
|
||||
rather than a line in the view: the two halves need different things
|
||||
from the same URL, and only one of them understands markdown. --}}
|
||||
<span class="break-all">[{{ $url }}]({{ $url }})</span>
|
||||
@@ -0,0 +1,9 @@
|
||||
{{-- The plain-text half of html/action-url.blade.php.
|
||||
|
||||
Just the URL. Laravel's own view writes `[$url]($url)` here, which is
|
||||
correct for the HTML half and wrong for this one: nothing parses
|
||||
markdown in a text/plain body, so it arrives as literal brackets with
|
||||
the address duplicated inside them — the shape a phishing template
|
||||
has. Seen in a real password reset, which is often the first mail an
|
||||
installation ever sends somebody. --}}
|
||||
{{ $url }}
|
||||
@@ -0,0 +1,72 @@
|
||||
{{-- Laravel's notification email view, published so the subcopy can spell
|
||||
the action URL out differently in each half of the message.
|
||||
|
||||
Upstream writes `[$url]($url)` there. That is right for the HTML half
|
||||
and wrong for the text one, where nothing parses markdown: it arrives
|
||||
as literal brackets around a duplicated address, which is what a
|
||||
badly-built phishing mail looks like — on a password reset, often the
|
||||
first mail an installation ever sends anybody. The x-mail::action-url
|
||||
component resolves to a different file per half, which is how every
|
||||
other component in this message already handles the same problem.
|
||||
|
||||
This is a copy of a framework view, so it does not follow Laravel
|
||||
forward on its own. If an upgrade changes the notification layout,
|
||||
re-copy it and re-apply the one-line change below. --}}
|
||||
<x-mail::message>
|
||||
{{-- Greeting --}}
|
||||
@if (! empty($greeting))
|
||||
# {{ $greeting }}
|
||||
@else
|
||||
@if ($level === 'error')
|
||||
# @lang('Whoops!')
|
||||
@else
|
||||
# @lang('Hello!')
|
||||
@endif
|
||||
@endif
|
||||
|
||||
{{-- Intro Lines --}}
|
||||
@foreach ($introLines as $line)
|
||||
{{ $line }}
|
||||
|
||||
@endforeach
|
||||
|
||||
{{-- Action Button --}}
|
||||
@isset($actionText)
|
||||
<?php
|
||||
$color = match ($level) {
|
||||
'success', 'error' => $level,
|
||||
default => 'primary',
|
||||
};
|
||||
?>
|
||||
<x-mail::button :url="$actionUrl" :color="$color">
|
||||
{{ $actionText }}
|
||||
</x-mail::button>
|
||||
@endisset
|
||||
|
||||
{{-- Outro Lines --}}
|
||||
@foreach ($outroLines as $line)
|
||||
{{ $line }}
|
||||
|
||||
@endforeach
|
||||
|
||||
{{-- Salutation --}}
|
||||
@if (! empty($salutation))
|
||||
{{ $salutation }}
|
||||
@else
|
||||
@lang('Regards,')<br>
|
||||
{{ config('app.name') }}
|
||||
@endif
|
||||
|
||||
{{-- Subcopy --}}
|
||||
@isset($actionText)
|
||||
<x-slot:subcopy>
|
||||
@lang(
|
||||
"If you're having trouble clicking the \":actionText\" button, copy and paste the URL below\n".
|
||||
'into your web browser:',
|
||||
[
|
||||
'actionText' => $actionText,
|
||||
]
|
||||
) <x-mail::action-url :url="$actionUrl" />
|
||||
</x-slot:subcopy>
|
||||
@endisset
|
||||
</x-mail::message>
|
||||
@@ -0,0 +1,65 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Models\User;
|
||||
use App\Modules\Identity\Notifications\ResetPasswordNotification;
|
||||
use Illuminate\Mail\Markdown;
|
||||
|
||||
/**
|
||||
* Every notification carrying an action button repeats its URL in the
|
||||
* subcopy, for somebody whose mail client will not let them click it.
|
||||
*
|
||||
* Laravel's own view writes that as `[$url]($url)`, which is right for
|
||||
* the HTML half and wrong for the text one: nothing parses markdown in a
|
||||
* text/plain body, so it arrives as literal brackets around a duplicated
|
||||
* address — the shape a badly-built phishing mail has, on what is often
|
||||
* the first message an installation ever sends anybody. Seen in the wild
|
||||
* on a real password reset before it was fixed.
|
||||
*/
|
||||
beforeEach(function () {
|
||||
User::factory()->create();
|
||||
});
|
||||
|
||||
/** The two halves Laravel builds for one markdown notification. */
|
||||
function renderResetMail(): array
|
||||
{
|
||||
$user = User::factory()->create();
|
||||
$mail = (new ResetPasswordNotification(str_repeat('a', 64)))->toMail($user);
|
||||
$mail->viewData['actionText'] = $mail->actionText;
|
||||
|
||||
$markdown = app(Markdown::class);
|
||||
$view = $mail->markdown ?: 'notifications::email';
|
||||
$data = array_merge($mail->toArray(), $mail->viewData);
|
||||
|
||||
return [
|
||||
'text' => (string) $markdown->renderText($view, $data),
|
||||
'html' => (string) $markdown->render($view, $data),
|
||||
'url' => $mail->actionUrl,
|
||||
];
|
||||
}
|
||||
|
||||
it('spells the action URL out plainly in the text half', function () {
|
||||
['text' => $text, 'url' => $url] = renderResetMail();
|
||||
|
||||
expect($text)->toContain($url)
|
||||
->and($text)->not->toContain('](')
|
||||
->and($text)->not->toContain('['.$url);
|
||||
});
|
||||
|
||||
it('still links the action URL in the html half', function () {
|
||||
['html' => $html, 'url' => $url] = renderResetMail();
|
||||
|
||||
// Twice: the button itself, and the subcopy that repeats it.
|
||||
expect(substr_count($html, 'href="'.e($url).'"'))->toBe(2)
|
||||
// The markdown must have been parsed, not passed through.
|
||||
->and($html)->not->toContain('['.e($url).']');
|
||||
});
|
||||
|
||||
it('does not repeat the URL more than the two places that need it', function () {
|
||||
['text' => $text, 'url' => $url] = renderResetMail();
|
||||
|
||||
// Once after the button label, once in the subcopy. A third meant the
|
||||
// markdown link had been left in place.
|
||||
expect(substr_count($text, $url))->toBe(2);
|
||||
});
|
||||
Reference in New Issue
Block a user