Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The default test from MakerBundle is failing #345

Open
florentdestremau opened this issue Feb 12, 2025 · 8 comments · May be fixed by symfony/maker-bundle#1669
Open

The default test from MakerBundle is failing #345

florentdestremau opened this issue Feb 12, 2025 · 8 comments · May be fixed by symfony/maker-bundle#1669

Comments

@florentdestremau
Copy link

florentdestremau commented Feb 12, 2025

I lost 2 hours debugging the generated test from the Maker command.

This part:

        // Test the link sent in the email is valid
        $email = $messages[0]->toString();
        preg_match('#(/reset-password/reset/[a-zA-Z0-9]+)#', $email, $resetLink);

...actually truncates the reset link and thus creates a regex where only half of the token is captured in the regex. This is my dd($emailBody):

From: =?utf-8?Q?Ceph=C3=A9e?=  <[email protected]>\r\n
To: [email protected]\r\n
Subject: Votre demande de =?utf-8?Q?r=C3=A9initialisation?= de mot de passe\r\n
MIME-Version: 1.0\r\n
Date: Wed, 12 Feb 2025 23:04:19 +0100\r\n
Message-ID: <[email protected]>\r\n
Content-Type: multipart/alternative; boundary=22I1bWE0\r\n
\r\n
--22I1bWE0\r\n
Content-Type: text/plain; charset=utf-8\r\n
Content-Transfer-Encoding: quoted-printable\r\n
\r\n
Bonjour !\r\n
\r\n
Pour r=C3=A9initialiser votre mot de passe, veuillez visiter=\r\n
 le lien suivant\r\n
\r\n
http://localhost/reset-password/reset/3xdOKluZom9sYOL=\r\n
wfeRvJUZVwkrya9HkzcTJd5zl\r\n
\r\n
Ce lien expirera dans 1 heure.\r\n
\r\n
=C3=\r\n
=80 bient=C3=B4t !\r\n
\r\n
--22I1bWE0\r\n
Content-Type: text/html; charset=utf-8\r\n
Content-Transfer-Encoding: quoted-printable\r\n
\r\n
<h1>Bonjour !</h1>\r\n
\r\n
<p>Pour r=C3=A9initialiser votre mot de passe, veui=\r\n
llez visiter le lien suivant</p>\r\n
\r\n
<a href=3D"http://localhost/reset-pas=\r\n
sword/reset/3xdOKluZom9sYOLwfeRvJUZVwkrya9HkzcTJd5zl">http://localhost/rese=\r\n
t-password/reset/3xdOKluZom9sYOLwfeRvJUZVwkrya9HkzcTJd5zl</a>\r\n
\r\n
<p>Ce li=\r\n
en expirera dans 1 heure.</p>\r\n
\r\n
<p>=C3=80 bient=C3=B4t !</p>\r\n
\r\n
--22I1bWE0--\r\n

The email content should be clearly the html or the text but toString is not reliable.

florentdestremau added a commit to florentdestremau/maker-bundle that referenced this issue Feb 12, 2025
More precise fetching for email body so that the link is not truncated as stated here SymfonyCasts/reset-password-bundle#345
@bocharsky-bw
Copy link
Member

Hey @florentdestremau ,

Thanks for reporting it. What do you suggest instead toString() there? Would you like to create a PR to fix it?

@florentdestremau
Copy link
Author

This is my solution, I type-hint the $messages and use the getTextBody. Maybe the regex could be updated to ignore \r\n characters also, what solution would you prefer ?

        // Test the link sent in the email is valid
        /** @var array<TemplatedEmail> $messages */
        $emailBody = $messages[0]->getTextBody();
        self::assertIsString($emailBody);
        preg_match('#(/reinitialisation-mot-de-passe/reset/[a-zA-Z0-9]+)#', $emailBody, $resetLink);

        self::assertArrayHasKey(1, $resetLink);
        $this->client->request('GET', $resetLink[1] ?? 'http://localhost');

@bocharsky-bw
Copy link
Member

As I understand, we're just lucky in our tests that the link was not split by the \r\n so that's why it works, but probably with your email content the split hit the link.

So that getTextBody() returns the text without those \r\n? If so, it makes sense to apply your solution - pretty straightforward, no need to complicate the regex IMO.

Could you please create a PR for this?

@florentdestremau
Copy link
Author

Yes will do in the week to come

@florentdestremau
Copy link
Author

florentdestremau commented Mar 5, 2025

Actually I had forgotten but I already made the PR:
florentdestremau/maker-bundle#1

edit: didn't point the branch to the main repository, tho 😅

@bocharsky-bw
Copy link
Member

Look good to me, I think we're ready to create this PR on symfony/maker-bundle repo

@florentdestremau
Copy link
Author

I did it at the same time: symfony/maker-bundle#1669

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants