8000 [HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For by magnusnordlander · Pull Request #18688 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For #18688

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

Closed
wants to merge 11 commits into from
Closed

[HttpFoundation] Warning when request has both Forwarded and X-Forwarded-For #18688

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
4c262d4
Emit a warning when a request has both a trusted
magnusnordlander May 2, 2016
da4b6c5
Updated according to fabbot review
magnusnordlander May 3, 2016
f0c3d15
Changed warning to exception, and only throws when headers disagree.
magnusnordlander May 5, 2016
2eea0f1
CS fix
magnusnordlander May 5, 2016
c017290
Added a listener to validate requests
magnusnordlander Jun 25, 2016
99b072e
Changed listener to only throw on master requests (to preserve except…
magnusnordlander Jun 25, 2016
41d2603
Added fabbot CS fixes
magnusnordlander Jun 25, 2016
ea691e7
Made the profiler handle unknown client IPs.
magnusnordlander Jun 25, 2016
a9f864d
Updated tests
magnusnordlander Jun 25, 2016
66f19f1
Renamed listener and CS fixes.
magnusnordlander Jun 28, 2016
d52d098
Fixed test failures
magnusnordlander Jun 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updated according to fabbot review
  • Loading branch information
@magnusnordlander
magnusnordlander committed May 3, 2016
commit da4b6c5fc610c66b3a2e58f7eb9499562245a6ae
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/Request.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,7 @@ public function getClientIps()
$hasTrustedClientIpHeader = self::$trustedHeaders[self::HEADER_CLIENT_IP] && $this->headers->has(self::$trustedHeaders[self::HEADER_CLIENT_IP]);

if ($hasTrustedForwardedHeader && $hasTrustedClientIpHeader) {
trigger_error("The request has both a trusted Forwarded header and a trusted Client IP header. This is likely a misconfiguration. You should either configure your proxy only to send one of these headers, or configure Symfony to distrust one of them. When both headers are set and trusted, this method returns only IPs from the Forwarded header.", E_USER_WARNING);
trigger_error('The request has both a trusted Forwarded header and a trusted Client IP header. This is likely a misconfiguration. You should either configure your proxy only to send one of these headers, or configure Symfony to distrust one of them. When both headers are set and trusted, this method returns only IPs from the Forwarded header.', E_USER_WARNING);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we could instead throw an exception here. What do you think? Any use case where setting both would be legitimate?

Copy link
Contributor Author
@magnusnordlander magnusnordlander May 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be legitimate as a legacy measure from proxies, if both headers contain the same information, I suppose.

That can be detected of course, I mean, it's just a matter of parsing both headers and see if they have the same IPs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update this PR to do that, because we probably don't want neither to warn nor throw exceptions if a proxy sends both headers for backwards compatibility.

}

if ($hasTrustedForwardedHeader) {
Expand Down
0