symfony-cmf / Routing

Routing component building on the Symfony Routing component
Other
289 stars 70 forks source link

Redirect routes with trailing slash #247

Closed EmmanuelVella closed 2 years ago

EmmanuelVella commented 4 years ago

Subject

Hi here !

I just upgraded my app from symfony 3.4 to 4.4, and I have an issue with trailing slashes. In symfony 3.4, I used this method which worked well.

But since symfony 4.1 and the new trailing slashes handling, I get an infinite loop when trying to match a cmf route with a trailing slash. Removing the custom redirect action works, but cmf urls with trailing slashes are not redirected anymore.

Steps to reproduce

Use symfony 4.1+, and try to match a cmf route ending with an url with a trailing slash.

Expected results

I should be redirected to the same url without the trailing slash.

Actual results

I get a 404.

dbu commented 4 years ago

hi emmanuel, long time no see :wave:

yeah, unless you store the route with the flag to have a trailing /, it will not be matched. if i understand https://symfony.com/doc/4.1/routing.html#routing-trailing-slash-redirection correctly, they now automatically redirect, but only if the route without a slash has been found. we could implement the same in the cmf route matcher (we already have the logic to see if the flag that a route is supposed to have a trailing slash) - i assume the only difference is that we would need to also provide the inverse logic to redirect removing the slash if no trailing slash is wanted. the candidates class has the logic to strip trailing slashes.

i would assume that we need something in the UrlMatcher to find out that we need to redirect, or in DynamicRouter - i am not sure at which level core symfony does the redirection, but think that we should do it on the same level.

do you want to do a pull request for that?

dannyvw commented 4 years ago

Is there any update for this?

dbu commented 4 years ago

i don't think anyone worked on it, no. want to give it a shot?

EmmanuelVella commented 4 years ago

Hi all ! Sorry, I just wanted to report the issue, I would be glad to help but unfortunately I have no time for this.

dbu commented 4 years ago

no worries @EmmanuelVella , reporting also helps. maybe @dannyvw wants to work on it?

dannyvw commented 4 years ago

After some debugging it looks like it is no problem in cmf routing bundle for us.

dbu commented 2 years ago

fixed in #465