sulu / SuluRedirectBundle

Sulu bundle for managing redirects
MIT License
18 stars 19 forks source link

Remove strtolower transform from RedirectRoute::setTarget #85

Closed luca-rath closed 2 years ago

luca-rath commented 2 years ago
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets -
Related issues/PRs -
License MIT

What's in this PR?

This PR removes the mb_strtolower transform from RedirectRoute::setSource(), RedirectRoute::setSourceHost() and RedirectRoute::setTarget().

Why?

I'm not sure if there was a real reason, that the mb_strtolower has been used for source and sourceHost, but at least for target it needs to be removed, because some websites return different responses for https://example.com/en_US and https://example.com/en_us e.g.

luca-rath commented 2 years ago

@alexander-schranz Can you remember what the reason was to set mb_strtolower for these properties?

alexander-schranz commented 2 years ago

@luca-rath it was a requirement, the behaviour is so expected, we match against lower case urls always. /redirect-URL and /redirect-url should be redirected to the same target. Atleast the source should stay with mb_strtolower. The target I'm curious where the case was that it was redirect to a none case url it should also be there a lower url and stay that way. Where did it appear?