[10.x] Use fallback when previous URL is the same as the current

[rodrigopedra]

closes #46134

As highlighted in issue #46134, when the previous URL stored in session is equals to the current one, a user would expect the UrlGenerator@previous to use the fallback URL instead of returning the current one.

One can easily reproduce the current behavior with these steps

1. create a new Laravel project
2. add this single route: Route::get('/', fn () => url()->previous('/foo'));
3. serve application
4. On first load, the previous URL displayed is http://127.0.0.1:8000/foo, using the default fallback as expected
5. Reload the page
6. Now, the previous URL displayed is http://127.0.0.1:8000, which is just the same as the current page

This happens as, in the first load, the current URL is stored into the session.

As in the first load there is nothing in the request's session, nor a referrer header, the UrlGenerator@previous returns the fallback.

When reloading, the same page, now there is a URL stored into the request's session. So UrlGenerator@previous does not default to the fallback URL and returns the current URL instead.

This is particularly annoying when the url()->previous() helper is used in back buttons. On a page that auto-refreshes in an interval, such as a live report page, after the first reload this button would not work.

I have used a custom helper to work around this behavior on past projects.

This PR

• Changes UrlGenerator@previous to verify if the previous URL is different from the current one, if so it defaults to the fallback
• Add relevant tests

Notes:

I compared to $this->request->fullUrl(), as the Request@fullUrl method is used to store the previous URL on the StartSession@storeCurrentUrl middleware method.

Using the UrlGenerator@current could lead to false positives, for example, when using pagination, as it doesn't account for query parameters.

Another small issue is that UrlGenerator@to does not trim a trailing forward slash when its parameter is already a valid URL.

I used rtrim(..., '/') when comparing to Request@fullUrl, as Request@fullUrl never has a trailing forward slash, due to being constructed from its path.

I thought on changing UrlGenerator@to to trim the trailing forward slash in those cases, or at least make Url@Generator@previous to trim the ones it uses, but refrained to do so for the following reasons:

• As this would be a behavior change, it is possible many test cases on userland rely on current behavior
• It would widen the scope of this PR
• I believe this change should be discussed, if not already discussed in the past

This is why the added test case has similar assertions, to account for the trailing forward slash, which could be present when a Request has a referrer header.

[rodrigopedra]

I was thinking about it, and I guess it is better when a fallback is provided, we use it anyway, even when it is equal to the current URL.

It would be up to the user to choose a sensible fallback, and not to the framework to also override the fallback.

I will leave both commits for comparison, but I can squash them into one if desired.

[taylorotwell]

Is there any situation where the current behavior would be preferred by the developer and thus would make this a breaking change?

[rodrigopedra]

I whole heartily can't think of any. As I said on issue #46134, I often use a local helper to overcome this behavior.

But I wouldn't mind pushing this to 11.x, if you think it is safer regarding BC.

I just saw an opportunity to fix this behavior when reading #46134.

If it is pushed to 11.x, I can keep using my local helper until then.

[taylorotwell]

Will give it a shot.

[matical]

There seems to be some funky interaction when throwing ValidationException::withMessages. The end result is that the usual automatic validation redirect no longer works. Originally, I thought it was isolated to just manual throws to ValidationException, but it seems to affect $request->validate() as well.

To repro:

• Fresh project with 10.3.0
• Install breeze, scaffold with blade, migrate
• Trigger a validation error on either registration or login
• Get redirected back to /

[clheppell]

I think that this change could alter the behaviour of redirects after validation errors.

Imagine you have a page at /model/{id} containing a form and do a PUT to /model/{id} to update it, but a ValidationException is thrown. The behaviour of the invalid method on \Illuminate\Foundation\Exceptions\Handler is to either use redirectTo (if you manually provided one) or to send you to url()->previous().

In this case where the URL for the GET of the form and the PUT is the same, the modified UrlGenerator@previous will return $this->to('/'), kicking the user to the homepage rather than back to the form.

(See someone else posted the same issue while I was writing this)

[tobz-nz]

Would this fix the problem? (it essetnial reverts to the same functionality before the merge when no $fallback is supplied)

...} elseif ($fallback) {
    return $this->to($fallback ?: $url);
}

[tobz-nz]

oh wait.. 🤦‍♂️

[browner12]

This PR does not appear to take the request METHOD into account, and is therefore causing a breaking change.

I have linked a bug report.

[browner12]

@driesvints or @taylorotwell, would it be possible to get a PR revert and a bugfix release if it's gonna take a bit to review the new PR?

[taylorotwell]

10.3.1 released reverting this.

[browner12]

appreciate it