Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Don't assume port 0 for X-Forwarded-Port#32096
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
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedJun 23, 2019
Could you please add a test case? |
alexbowers commentedJun 30, 2019
@nicolas-grekas I've added a test for this. |
alexbowers commentedJul 1, 2019
Hmm, strange, its failing on the tests. But passes locally. I'll take a look at that this evening. |
alexbowers commentedJul 1, 2019
I'm a little confused, it is failing on php 5.5, but is fine on php 7.0+. Nothing I changed should affect 5.5, any ideas? |
xabbuh commentedJul 4, 2019
@alexbowers PHP changed how |
alexbowers commentedJul 4, 2019
Ah, thank you@xabbuh |
alexbowers commentedJul 4, 2019
Hmm, can AppVeyor be re-ran? I can't see any cause for it failing. |
| $request = Request::create('/'); | ||
| $request->server->set('REMOTE_ADDR','1.1.1.1'); | ||
| $request->headers->set('X-Forwarded-Host','test.example.com'); | ||
| $request->headers->set('X-Forwarded-Port',null); |
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.
HeaderBag::set does not accept null. Maybe you meant to use an empty string.
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.
fabpot commentedJul 8, 2019
Thank you@alexbowers. |
This PR was merged into the 3.4 branch.Discussion----------Don't assume port 0 for X-Forwarded-Port| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | none added| Fixed tickets || License | MIT| Doc PR | -If you use X-Forwarded-Host but don't provide X-Forwarded-Port, it will default to `0.0.0.0:` which then assumes port `0` instead of following its default assumption based on the scheme.Commits-------adcdd93 PHP 5 compat6c49a0c Add test casec266d6c Update Request.php23db9be Don't assume port 0 for X-Forwarded-Port
alexbowers commentedJul 26, 2019
@fabpot any idea when the next release will be? |
If you use X-Forwarded-Host but don't provide X-Forwarded-Port, it will default to
0.0.0.0:which then assumes port0instead of following its default assumption based on the scheme.