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

Match source and sourcehost case insensitively #63

Merged

Conversation

niklasnatter
Copy link
Contributor

@niklasnatter niklasnatter commented Feb 25, 2021

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
License MIT

What's in this PR?

This PR adjusts the code to match the source and sourceHost of a redirect in a case-insensitive manner.

Assuming a redirect with source: "redirect-url", sourceHost: "www.test.com":

Behaviour before: matches only www.test.com/redirect-url
Behaviour after: matches www.test.com/redirect-url, www.test.com/redirect-URL, www.test.com/REDIRECT-url, ...

Why?

Because MySQL is case insensitive by default. This means it is not possible to create two redirects that use the same characters (eg. one redirect for redirect-url and one redirect for redirect-URL).

Because of this, without this change, it is not possible to redirect both urls (www.test.com/redirect-url and www.test.com/redirect-URL) at all.

@@ -49,7 +49,7 @@ public function getRouteCollectionForRequest(Request $request)
}

$route = new Route(
$redirectRoute->getSource(),
$pathInfo,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nnatter why this change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the route name? Should we maybe more go with: 'sulu_redirect.redirect_' . $redirectRoute->getId() or something 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no it is the path of the route. this is used for generating the regex of the route here:
https://github.com/symfony/symfony/blob/252f85c2c249da7d8f2a490b57cc71199d51fcc9/src/Symfony/Component/Routing/RouteCompiler.php#L75-L92

it looks like the regex is evaluated case sensitive. if we pass /test123 here but the request contains TEST123, symfony will not match the request to the route

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay this make sense not sure if we need to use here $pathInfo or $request->getPathInfo 🤔

@niklasnatter niklasnatter force-pushed the enhancement/case-insensitive-url branch from 1acf7c9 to 04d6bdc Compare March 1, 2021 15:39
@niklasnatter niklasnatter force-pushed the enhancement/case-insensitive-url branch from 573762b to 3994a2c Compare March 1, 2021 16:33
@niklasnatter niklasnatter force-pushed the enhancement/case-insensitive-url branch from 3994a2c to 2adfae2 Compare March 1, 2021 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants