Skip to content

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

Closed
wants to merge 4 commits into from

Conversation

JarJak
Copy link
Contributor

@JarJak JarJak commented May 3, 2017

"[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

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
Copy link
Member

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.

Copy link
Contributor Author

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.

@weaverryan weaverryan added this to the 3.3 milestone May 8, 2017
Copy link
Member

@xabbuh xabbuh left a 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.

@weaverryan
Copy link
Member

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, 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 do you think you could make that change?

@JarJak
Copy link
Contributor Author

JarJak commented May 15, 2017 via email

@JarJak
Copy link
Contributor Author

JarJak commented May 16, 2017

Sorry, but actually I am not sure how to describe this "header" option because I've never used it :(

@nicolas-grekas
Copy link
Member

I think that this PR should also be the one fixing #7045
by telling about Request::HEADER_X_FORWARDED_AWS_ELB

@weaverryan
Copy link
Member

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 X-FORWARDED-* headers:

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 [...] part, this is still the same IP address or IP range of your proxy as before. And as before if your IP constantly changes (like on ELB), you should still use $request->server->get('REMOTE_ADDR') (assuming public traffic cannot hit your server directly). In other words, this note is still valid: http://symfony.com/doc/current/request/load_balancer_reverse_proxy.html#but-what-if-the-ip-of-my-reverse-proxy-changes-constantly (it just needs to be updated to have a 2nd argument).

@JarJak Would you care to update your PR given this new info? I also had problems understanding it at first :)

@weaverryan
Copy link
Member

Hey @JarJak! I've just finished this PR in #7952 - just because this is an important change and the release is upon us! But, your commits are in that PR - so you are a Symfony Docs contributor :).

Thanks!

@weaverryan weaverryan closed this May 27, 2017
weaverryan added a commit that referenced this pull request May 30, 2017
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants