8000 [HttpKernel] Fix exception when serializing request attributes by nicolas-grekas · Pull Request #20621 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[HttpKernel] Fix exception when serializing request attributes #20621

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

Merged
merged 1 commit into from
Nov 24, 2016

Conversation

nicolas-grekas
Copy link
Member
@nicolas-grekas nicolas-grekas commented Nov 24, 2016
Q A
Branch? 3.2
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

Serializing request attributes obviously fails easily since once can put anything there (I've just got an "Exception: Serialization of 'Closure' is not allowed").
Yet, we don't need attributes when forging a request to feed the TraceableUrlMatcher in RouterController.
Well, technically one could for sure register a listener before the router to add an attribute to the request, then use that in the url matcher.
But, it makes no sense. And if it were to have, the profiler is already broken in this respect because in e.g. 3.1, the attribute array that is used here has already been processed by the ValueExporter. So the original values have already been altered.

Let's just handle request attributes as all the other collected data: using var dumper.

@nicolas-grekas nicolas-grekas changed the base branch from master to 3.2 November 24, 2016 17:03
@nicolas-grekas nicolas-grekas added this to the 3.2 milestone Nov 24, 2016
@fabpot
Copy link
Member
fabpot commented Nov 24, 2016

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 2e404d0 into symfony:3.2 Nov 24, 2016
fabpot added a commit that referenced this pull request Nov 24, 2016
…utes (nicolas-grekas)

This PR was merged into the 3.2 branch.

Discussion
----------

[HttpKernel] Fix exception when serializing request attributes

| Q             | A
| ------------- | ---
| Branch?       | 3.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations?
B1D8
 | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Serializing request attributes obviously fails easily since once can put anything there (I've just got an "Exception: Serialization of 'Closure' is not allowed").
Yet, we don't need attributes when forging a request to feed the TraceableUrlMatcher in RouterController.
Well, technically one could for sure register a listener before the router to add an attribute to the request, then use that in the url matcher.
But, it makes no sense. And if it were to have, the profiler is already broken in this respect because in e.g. 3.1, the attribute array that is used here has already been processed by the ValueExporter. So the original values have already been altered.

Let's just handle request attributes as all the other collected data: using var dumper.

Commits
-------

2e404d0 [HttpKernel] Fix exception when serializing request attributes
@nicolas-grekas nicolas-grekas deleted the profiler-req-attr branch November 24, 2016 17:41
nicolas-grekas added a commit that referenced this pull request Nov 25, 2016
…rController (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[WebProfilerBundle] Dont use request attributes in RouterController

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

As spotted in #20621, it makes little sense to use request attributes here, and if it were to have, the current code is broken because the returned values here have already been processed by ValueExporter in RequestDataCollector.

Commits
-------

962325a [WebProfilerBundle] Dont use request attributes in RouterController
@fabpot fabpot mentioned this pull request Nov 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0