Skip to content

Conversation

SerheyDolgushev
Copy link

Symfony expects that if Varnish (or any other reverse proxy) is used then it's IP is stored as the value for REMOTE_ADDR:

For example, instead of reading the REMOTE_ADDR header (which will now be the IP address of your reverse proxy), the user’s true IP will be stored in a standard Forwarded: for="..." header or a X-Forwarded-For header.

But that is not true for Platform.sh. On Platform.sh REMOTE_ADDR variable contains the real user IP (Please ping @damz for details).

  • So in Platform.sh case IP will be extracted from the HTTP_X_FORWARDED_FOR header.
  • HTTP_X_FORWARDED_FOR contains the real user IP (it is the same as REMOTE_ADDR and Varnish IP)
  • Because Symfony assumes that REMOTE_ADDR is the Varnish IP (but not the real user IP, which it is) the another IP will be extracted as the real user IP from HTTP_X_FORWARDED_FOR

So Varnish IP is extracted as clean IP on all Symfony projects which run on Platform.sh and are using Varnish.

This PR adds "a special" check for Platform.sh, and returns REMOTE_ADDR instead of extracting the wrong IP from HTTP_X_FORWARDED_FOR.

@symfony-bot
Copy link

symfony-bot bot commented Sep 8, 2020

Thanks for your pull request! We love contributions.

However, this repository is what we call a "subtree split": a read-only copy of one directory of the main Symfony repository. It is used by Composer to allow developers to depend on specific Symfony components.

If you want to contribute, you should instead open a pull request on the main repository:

https://github.com/symfony/symfony

Thank you for your contribution!

PS: if you haven't already, please add tests, and beware that bug fixes should be submitted on the lowest maintained branch where they apply; only features should be submitted against the master branch.

@symfony-bot symfony-bot bot closed this Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant