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
|
This section collects changes as they land; the release process turns it into a numbered entry when
|
||||||
a version is cut.
|
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
|
## 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.
|
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