-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Remove deprecated trusted_proxies config option #7868
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
Conversation
xmlns:framework="http://symfony.com/schema/dic/symfony" | ||
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd | ||
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd"> | ||
.. code-block:: diff |
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.
Currently, diff
does not work (we need #7806 for it). We should rewrite the example a bit and use the common php
code block instead.
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.
Can you change it to fit the needs? I'm not sure how this parser works.
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.
👍 I just pushed an update for the code block.
This is great! But.... it's not quite done yet! This fixes #7671 (yay!). But check out the comment in that issue. In 3.3, We need to update the example to pass this value, and mention what it is. @JarJak do you think you could make that change? |
Yes, sure I will :)
14.05.2017 9:43 PM "Ryan Weaver" <notifications@github.com> napisał(a):
… This is great! But.... it's not quite done yet! This fixes #7671
<#7671> (yay!). But check
out the comment in that issue. In 3.3, setTrustedProxies() has a required
*second* argument: https://github.com/symfony/symfony/blob/
fb532bfc172a531bce4219e4f21db124eb972bbc/src/Symfony/
Component/HttpFoundation/Request.php#L591-L594
We need to update the example to pass this value, and mention what it is.
@JarJak <https://github.com/jarjak> do you think you could make that
change?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7868 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEwC8qSo_JZQfczwZG2amyJgU2tWjlHXks5r51l5gaJpZM4NPdar>
.
|
Sorry, but actually I am not sure how to describe this "header" option because I've never used it :( |
I think that this PR should also be the one fixing #7045 |
I just chatted with Nicolas about this! There are a few situations: A) On AWS behind an ELB Request::setTrustedProxies([...], Request::HEADER_X_FORWARDED_AWS_ELB); B) On most other systems (systems where your proxy passes Request::setTrustedProxies([...], Request::HEADER_X_FORWARDED_ALL); C) If your load balancer passes the "Forwarded" header from rfc 7239, then use: Request::setTrustedProxies([...], Request::HEADER_FORWARDED); For the IP address - the @JarJak Would you care to update your PR given this new info? I also had problems understanding it at first :) |
…roxies removal (JarJak, xabbuh, weaverryan) This PR was merged into the 3.3 branch. Discussion ---------- [3.3] Finishing setTrustedProxies() changes & trusted_proxies removal Finishes #7868 Commits ------- c7087bf tweaks thanks to review 45b419d Adding second argument to setTrustedProxies() and removing old information 2f83164 replace trusted_proxies reference 3632c08 replace diff code block with PHP code block 1825c83 Remove deprecated trusted_proxies config option df63034 Remove deprecated trusted_proxies config option
"[BC BREAK] The "framework.trusted_proxies" configuration option and the corresponding "kernel.trusted_proxies" parameter have been removed. Use the Request::setTrustedProxies() method in your front controller instead."
By the way, is 'trusted_hosts' config option still revellant?
Fixes: #7671