Symfony 3.3 routing automatic trailing slash missing

[dubaksk]

Q	A
Bug report?	yes
Feature request?	yes
BC Break report?	no
RFC?	?
Symfony version	3.3.0

After upgrading to Symfony 3.3.0 we realised, that when we had something like this:

     * @Route(
     *     "/c/{articleId}/",
     *     name="article_simple_no_alias",
     *     methods={"GET"},
     *     requirements={"articleId": "[0-9]+?"}
     * )

when I put into browser URL without trailing slash, I was redirected automatically to URL with trailing slash. And it worked also other way.

In profiler -> request -> parameters, I could see:

_controller	
"Symfony\Bundle\FrameworkBundle\Controller\RedirectController::urlRedirectAction"
permanent	true
scheme	null

It was ok. But since 3.3 version this feature is missing and we could not find it in changelog if it was change intentionally. Now the page without trailing slash is giving us error 404 and we really don't want to go through whole project and update all routes, so this trailing slash would be optional..

[mvrhov]

This is probably because of the huge speed optimizations that have been done on the routing component. Ping @frankdejonge

[frankdejonge]

@dubaksk does this happen only in the prod env or also on your dev env?

[dubaksk]

@frankdejonge It happens on all environments. We do not consider this happen due to different environment set up.

[frankdejonge]

Ok, so then we know it's not because of the matcher dumper optimisation because those aren't used in dev. We could look into the RouteCompiler next, I'll check that out.

[frankdejonge]

Hmm, it seems the matcher dumper is actually used, digging into that now.

[frankdejonge]

@dubaksk what version did you upgrade from?

[frankdejonge]

@dubaksk from what I see those redirects only happen for HEAD requests, or for routes that haven't defined methods. Which is both a little wonky. In my perception it should be done on non HEAD and GET routes. The code that determines it dates back 6 years, so it popping up now is a bit odd (https://github.com/symfony/symfony/blame/master/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php#L241). Perhaps this was a side-effect from another commit.

[frankdejonge]

@dubaksk I've created a PR, perhaps you can check it out? See above.

[dubaksk]

@frankdejonge Great, so you allowed also GET method. Give me a some time to verify the fix. I will let you know.

[frankdejonge]

@dubaksk 👍

[dubaksk]

@frankdejonge We verified your pull request and it's working as before on 3.2.9 version 👍
So now we don't have to do create something hacky like this:

     * @Route(
     *     "/c/{articleId}/{articleAlias}{trailingSlash}",
     *     name="sport_article_simple",
     *     methods={"GET"},
     *     defaults={"page": 1, "trailingSlash" = "/"},
     *     requirements={"articleId": "[0-9]+?", "articleAlias": "[a-zA-Z0-9\-\_]+?", "trailingSlash" = "[/]{0,1}"}
     * )

So what's the scenario now? We have to wait until pull request is accepted and then you will release 3.3.X version with this fix?

[frankdejonge]

@dubaksk another option for now would be to include the HEAD method in addition to the GET method. But for the actual fix we'd need to wait for 3.3.1 like you mentioned.

[dubaksk]

@frankdejonge Yep, this will be the way. Thank you guys and mostly you @frankdejonge for such a prompt solution.