8000 [HttpFoundation] Recheck method if httpMethodParameterOverride enabled by dirkluijk · Pull Request #9465 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] Recheck method if httpMethodParameterOverride enabled #9465

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
wants to merge 2 commits into from
Closed

[HttpFoundation] Recheck method if httpMethodParameterOverride enabled #9465

wants to merge 2 commits into from

Conversation

dirkluijk
Copy link
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #9464
License MIT
Doc PR n/a

This PR fixes an issue where method parameter overriding fails if the getMethod is called BEFORE the overriding is enabled, because the value is cached.

See #9464

@dirkluijk
Copy link
Author

Now the problem is solved, but I don't like this solution. The caching mechanism is not used at all if overriding is enabled.

Another approach would be that enabling overriding (i.e. calling enableHttpMethodParameterOverride) must ALWAYS be done BEFORE calling getMethod(). In that case, the real bug is in the FrameworkBundle.

Anyone?

@dirkluijk
Copy link
Author

I was wondering why this was not covered by any test, but I discovered that the method getMethod is never called before Request::enableHttpMethodParameterOverride. When I do, the test fails. Is that on purpose?

@dirkluijk
Copy link
Author

I am sorry. I didn't read #8481 about the front controller.

@dirkluijk dirkluijk closed this Nov 7, 2013
@dirkluijk dirkluijk deleted the method-overriding branch November 8, 2013 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@dirkluijk
0