8000 [Router] Auto-redirect for trailing slashes fails to match when both routes exist · Issue #36203 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[Router] Auto-redirect for trailing slashes fails to match when both routes exist #36203

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.

Sign up for GitHub

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

Jump to bottom
Closed
Seldaek opened this issue Mar 25, 2020 · 10 comments
Closed

[Router] Auto-redirect for trailing slashes fails to match when both routes exist #36203

Seldaek opened this issue Mar 25, 2020 · 10 comments
Labels
Bug Routing Status: Reviewed

Comments

@Seldaek
Copy link
Member
Seldaek commented Mar 25, 2020

Symfony version(s) affected: 4.4.x, 5.*

Description
Router auto-redirection for trailing slashes (https://symfony.com/doc/master/routing.html#redirecting-urls-with-trailing-slashes) fails to match the route correctly when both routes (with and without trailing slashes) are defined. In that case, whichever the first route is is matched, and the other redirects to it always.

How to reproduce

Define two routes on a controller

     * @Route("/foo", name="foo", defaults={"_format"="json"}, methods={"GET"})
     * @Route("/foo/", name="foo_bc", defaults={"_format"="json"}, methods={"GET"})

Accessing /foo works, accessing /foo/ redirects to /foo, even though the exact match route exists. That sounds to me like a bug/overly greediness of the trailing slash redirecter.

Alternative solution or in addition to the bugfix, could be to offer an option for this behavior, which could be configured to either:

  • redirect (current)
  • allow /foo/ to match a /foo route silently, and /foo to match a /foo/ route also
  • no magic, just strict match or 404
@Seldaek Seldaek added the Bug label Mar 25, 2020
@xabbuh xabbuh added the Routing label Mar 26, 2020
@fancyweb
Copy link
Contributor

Status: reviewed

I was able to reproduce the problem. I'm looking for a fix.

@blowski
Copy link
blowski commented Mar 28, 2020

Inspired by yesterday's online session, I'm happy to take this on if nobody else has started?

@nicolas-grekas
Copy link
Member

To me there is no bug here, this is the behavior we've always had for routes with dynamic parameters, and we fixed static routes in 4.1.

@nicolas-grekas
Copy link
Member

Check #33342 for a still open discussion on the topic, which contains a link to a way to reclaim back control of trailing slashes: #33362 (comment)

@Seldaek
Copy link
Member Author
Seldaek commented Mar 28, 2020

Thanks for the issue links.. I read a lot there and I still disagree. I can see that for /Foo/{bar} where placeholder is optional then it gets complicated as it can conflict with a /Foo route, but imo if the exact route exists without optional placeholder and both trailing and no trailing slash then both should be routable.

Ironically.. this is possibly all my fault symfony/routing@c39683b
8000

@Tobion
Copy link
Contributor
Tobion commented Mar 30, 2020

See also #32996 (comment) ff.

@blowski
Copy link
blowski commented Mar 30, 2020

What's the desired behaviour here? Is there a bug that needs fixing? I'm looking for something to do on Symfony core, so I'm happy to have a go at this, but don't really understand what needs doing.

@nicolas-grekas
Copy link
Member

@blowski this won't be resolved soon, the current situation is well defined and consistent. There is no bug here - but there are objections to the current behavior, "that's all". Better look for something else if you want to contribute.

@blowski
Copy link
blowski commented Mar 31, 2020

Ah ok, thanks for letting me know @nicolas-grekas .

@Seldaek
Copy link
Member Author
Seldaek commented Apr 2, 2020

Closing as it doesn't seem like this will go anywhere.

@Seldaek Seldaek closed this as completed Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Routing Status: Reviewed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@Seldaek @nicolas-grekas @Tobion @blowski @xabbuh @fancyweb @carsonbot
0