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

[5.7] Fix broken email subcopy template escaping #25723

Merged
merged 1 commit into from
Sep 21, 2018
Merged

[5.7] Fix broken email subcopy template escaping #25723

merged 1 commit into from
Sep 21, 2018

Conversation

laurencei
Copy link
Contributor

@laurencei laurencei commented Sep 20, 2018

Laravel 5.6.36 (d3c0a36) introduced a security change to prevent XSS in some situations.

Unfortunately a byproduct of this is a number of breaking changes as documented #25515, #25501, #25408, d3c0a36#commitcomment-30368450, #25716, #25430

One of those breaking changes linked above is email subcopy links do not work out of the box on a fresh Laravel 5.7 install (i.e. email verification subcopy links are broken).

This PR pulls the URL out of the @lang section on the email template, and places it immediately after, allowing the raw url to be un-escaped as required to generate a correct URL. Fixes #25716

Before PR:

image

After PR:

image

p.s. if this PR is accepted we'll need to backport to 5.6

@taylorotwell taylorotwell merged commit 3c397ea into laravel:5.7 Sep 21, 2018
@laurencei laurencei mentioned this pull request Sep 21, 2018
@staudenmeir
Copy link
Contributor

@laurencei Now that d3c0a36 got reverted, should we also revert this fix?

@bbashy
Copy link
Contributor

bbashy commented Jan 24, 2019

In the Content-Type: multipart/alternative; version of the verify email it's coming out with & in it. The Content-Type: text/plain; one is ok.

Is this to do with this?

http://domain.ext/email/verify/42152?expires=1548326980&signature=5d25ea89b968b2d030af454eb95e8c7bcd599344bb54a4de6431789fd587a578

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

Successfully merging this pull request may close these issues.

4 participants