Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpKernel] Inline ValidateRequestListener logic into HttpKernel#19217
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| "php":">=5.3.9", | ||
| "symfony/event-dispatcher":"~2.6,>=2.6.7", | ||
| "symfony/http-foundation":"~2.7,>=2.7.15", | ||
| "symfony/http-foundation":"~2.7,>=2.7.15|~2.8,>=2.8.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this will need to be"~2.8,>=2.8.8|~3.0,>=3.0.8|~3.1,>=3.1.2", when merging into 3.0
nicolas-grekas commentedJun 29, 2016
Ready |
nicolas-grekas commentedJun 29, 2016
ping@magnusnordlander for info |
| { | ||
| if (self::MASTER_REQUEST ===$type) { | ||
| try { | ||
| // This will throw an exception if the headers are inconsistent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Maybe this comment is superfluous because the code is short and the exception message is very clear?
962e1f8 to0691a22Compare| "php":">=5.3.9", | ||
| "symfony/event-dispatcher":"~2.6,>=2.6.7", | ||
| "symfony/http-foundation":"~2.7,>=2.7.15", | ||
| "symfony/http-foundation":"~2.7.15|~2.8,>=2.8.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"~2.8.8|~3.0.8|~3.1,>=3.1.2" on 3.0
| "php":">=5.3.9", | ||
| "symfony/event-dispatcher":"~2.6,>=2.6.7", | ||
| "symfony/http-foundation":"~2.7,>=2.7.15", | ||
| "symfony/http-foundation":"~2.7.15|~2.8.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
"~2.8.8|~3.0.8|~3.1.2|~3.2" on 3.0 (really this time :) )
magnusnordlander commentedJun 29, 2016
No objections from me :) |
…pKernel (nicolas-grekas)This PR was merged into the 2.7 branch.Discussion----------[HttpKernel] Inline ValidateRequestListener logic into HttpKernel| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#18688#19216| License | MIT| Doc PR | -I propose to inline the listener introduced in#18688 into HttpKernel.Commits-------9d3ae85 [HttpKernel] Inline ValidateRequestListener logic into HttpKernel
fabpot commentedJun 29, 2016
Why? I'm 👎 for this, that's ugly as hell! |
magnusnordlander commentedJun 29, 2016
I agree that it's uglier. It does have an upside though, and that is that any project using HttpKernel will automatically get the client IP exceptions early, even if they don't use the listener in FrameworkBundle. When I implemented#18688 I considered an approach like this for that very reason. For me, the ugliness tipped the scale towards a listener though, but both approaches have merit. |
nicolas-grekas commentedJun 29, 2016
See#19233 |
…catch block (magnusnordlander, nicolas-grekas)This PR was merged into the 2.7 branch.Discussion----------[HttpKernel] Move handling of conflicting origin IPs to catch block| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#19217| License | MIT| Doc PR | -Commits-------db84101 [HttpKernel] Add listener that checks when request has both Forwarded and X-Forwarded-For1f00b55 [HttpKernel] Move conflicting origin IPs handling to catch block
Uh oh!
There was an error while loading.Please reload this page.
I propose to inline the listener introduced in#18688 into HttpKernel.