Skip to content

[Routing] Missing redirection when route param does not accept trailing slash #30863

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

Closed
marc-romera opened this issue Apr 4, 2019 · 3 comments

Comments

@marc-romera
Copy link

marc-romera commented Apr 4, 2019

Symfony version(s) affected: 4.2.4

Description
Symfony 4 does not redirect in a route ending with a param which does not accept a trailing slash. Route can disallow a trailing slash if configured as shown here. Symfony 4 should detect that the route is invalid with a trailing slash (but valid without it) and so return a redirect response to the same route without the slash.

How to reproduce
Step 1. Set your route

#app/config/routing.yml
index:
    path:       /check/{token}
    controller:  AppBundle:Default:token
    defaults:
        token: 1
    requirements:
        token: \d+

Step 2. Implement the action

// src/AppBundle/Controller/DefaultController.php
class DefaultController extends Controller
{
    /**
     * @param $token
     *
     * @return Response
     */
    public function tokenAction(string $token)
    {
        return new Response('token is ' . $token);
    }
}

Step 3. Serve the application and visit the path http://127.0.0.1:8000/check/123/
The output is the one it would provide without the slash:
token is 123

But we want it to actually be redirected to http://127.0.0.1:8000/check/123, which is the only valid one, already working and returning the same exact response.

Additional context
Route-related log is identical in both cases (i.e., with and without the trailing slash):
[2019-04-04 09:50:35] request.INFO: Matched route "test". {"route":"test","route_parameters":{"_route":"test","token":"123","_controller":"AppBundle\\Controller\\DefaultController::tokenAction"},"request_uri":"http://127.0.0.1:8000/check/123/","method":"GET"} []

PS: Example heavily based on #29924, which seems somewhat related to this issue

@nicolas-grekas
Copy link
Member

Related to #30013
The logic behind this is complex and I'm not sure your requirements don't conflict with some others expressed in the issues linked to this PR.

@marc-romera
Copy link
Author

marc-romera commented Apr 4, 2019

The related issue you provide allows trailing slashes, while the problem here is when these are not allowed, so requirements don't conflict.

One difference can be easily seen in the profiler. With my example the routing profiler says the path does not match (although it finally reinterprets it somehow and returns a 200):
image

If I change the regex in the route definition to .+ instead of \d+, then the route matches, and also the response changes to token is 123/ instead of token is 123.

In summary: Your related issue and this one already behave different (routing treats them differently and even response is different), so they are different situations with different requirements, and this one behaves inconsistently

@nicolas-grekas
Copy link
Member

Fixed in #31107

fabpot pushed a commit that referenced this issue Apr 17, 2019
…railing vars (nicolas-grekas)

This PR was merged into the 4.2 branch.

Discussion
----------

[Routing] fix trailing slash redirection with non-greedy trailing vars

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #30863, #31066
| License       | MIT
| Doc PR        | -

Fixes redirecting `/123/` to `/123` when the route is defined as `/{foo<\d+>}`

Commits
-------

d88833d [Routing] fix trailing slash redirection with non-greedy trailing vars
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants