BUGFIX: Compare hosts when checking if target and source are the same

[robinroloff]

This PR fixes the "sameSourceAndTarget" and check. Previously, it was not correctly comparing the source and target host.

On the frontend, it was comparing the sourceHost to the host the user is currently on:

if (!host || host === location.host) {

This also had the unwanted side effect that this check would only work when the user is creating a redirect for the very same domain they are currently on (most of the time the primary site). When they were creating a redirect for another site, this would only be checked on the backend and give the user a generic error "Redirect could not be created".

And on the backend, it was creating the redirect using the $host variable:

$redirect = $this->redirectStorage->getOneBySourceUriPathAndHost($sourceUriPath, $host ?: null, false);

and then comparing it against itself:

$isSame = $this->isSame($sourceUriPath, $targetUriPath, $host, $statusCode, $redirect);

Example:

Domain: www.foo.bar
Source Path: test
Target Path: www.example.com/test

Before: This throws an error
Now: Works as expected

[Sebobo]

Thx for working on this!

Though this change doesn't look right yet to me. But I agree that it wasn't correct before.

[Sebobo]

The host is used to determine whether a redirect is triggered in the middleware. So it has no relation to the target. IMO this part of the change is incorrect.
I think rather the targetUriPath should be checked twice in case one time it was given without host and one time with a host.

[robinroloff]

I'm not sure I understand. $redirect->getHost() === $host will always be true, so I'm not sure if it's the correct check in this case.

Can you tell me how you would've solved it?

[Sebobo]

In this case the host comparison compares to equal, which is true and fine.

$targetUriPath can be a full url or just a path.
So you have to turn the existing and the new $targetUriPath into an absolute url for comparison and then check.

[Sebobo]

Shouldn't it be instead:

if (parsedSourceUrl.hostname === parsedTargetUrl.hostname && parsedSourceUrl.pathname === parsedTargetUrl.pathname) {

[robinroloff]

You're right, this probably got lost when I rebased something. Changed it

[Sebobo]

@robinroloff btw. make sure to commit the production build of the JS, you committed the dev build. And we need to squash those JS commits at the end, or they will add to much clutter to the history.