8000 Incorrect route matching logs in web profiler for route with condition and query parameters · Issue #25206 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Incorrect route matching logs in web profiler for route with condition and query parameters #25206

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
qzminski opened this issue Nov 29, 2017 · 6 comments
Closed

Incorrect route matching logs in web profiler for route with condition and query parameters #25206

qzminski opened this issue Nov 29, 2017 · 6 comments
Labels
Bug Status: Reviewed WebProfilerBundle

Comments

@qzminski
Copy link
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.3.13

The web profiler's route matching logs do not seem to correctly match the route although the route is in fact matched. Given we have a route configuration:

magazin:
    path: magazin
    condition: "request.query.has('page')"
    defaults: ...

The URL project.dev/app_dev.php/magazin?page=123 will be correctly mapped to that route but the profiler information does not seem to properly report that in the logs:

2017-11-29 at 10 25

I think this is related to #20621 and probably can be fixed in https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/WebProfilerBundle/Controller/RouterController.php#L86:

    private function getTraces(RequestDataCollector $request, $method)
    {
        $traceRequest = Request::create(
            $request->getPathInfo(),
            $request->getRequestServer(true)->get('REQUEST_METHOD'),
-           array(),
+           $request->getRequestQuery()->all(),
            $request->getRequestCookies(true)->all(),
            array(),
            $request->getRequestServer(true)->all()
        );
@sroze
Copy link
Contributor
sroze commented Jan 15, 2018 •
edited
Loading

@qzminski it looks like this issue is pretty old and you did not come back on it... As this been fixed by #20621?

@qzminski
Copy link
Author

The fix you are referring to removes the request attributes from the request trace and actually breaks the correct behaviour. It causes the web profiler to output the incorrect route matching logs because the trace lacks the route query attributes. I understand that some request attributes (like mentioned closure) could cause problems but the query should be safe to store.

@carsonbot
Copy link

Hey, thanks for your report!
There has not been a lot of activity here for a while. Is this bug still relevant? Have you managed to find a workaround?

@carsonbot
Copy link

Friendly reminder that this issue exists. If I don't hear anything I'll close this.

@xabbuh
Copy link
Member
xabbuh commented Jan 4, 2021

Status: Reviewed

@xabbuh xabbuh mentioned this issue Jan 4, 2021
Merged
@xabbuh
Copy link
Member
xabbuh commented Jan 4, 2021

see #39708

nicolas-grekas added a commit that referenced this issue Jan 4, 2021
@nicolas-grekas
bug #39708 [WebProfilerBundle] take query and request parameters into…
e8d5d29 
… account when matching routes (xabbuh)

This PR was merged into the 4.4 branch.

Discussion
----------

[WebProfilerBundle] take query and request parameters into account when matching routes

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #25206
| License       | MIT
| Doc PR        |

Commits
-------

28b6051 take query and request parameters into account when matching routes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Reviewed WebProfilerBundle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@qzminski @nicolas-grekas @sroze @xabbuh @carsonbot
0