@mattab opened this Pull Request on August 16th 2016 Member

Fixes #10342

  • It's hard to know 100% whether this is a correct patch.
  • The RFC at https://tools.ietf.org/html/rfc7239 makes it clear that we should extract the first IP from the header HTTP_X_FORWARDED_FOR
  • assuming we have had this bug for 5+ years, some users may have mis-configured their proxy setup especially just for Piwik to detect the "last IP" correctly. Once we start reading the first IP from the list, their geo-location and other features depending on IP address may break -> solution would be to configure the Proxy headers correctly so the first IP returned is the IP of the client.
  • it is unclear whether it is correct to do so for others as well ie. HTTP_CLIENT_IP and HTTP_CF_CONNECTING_IP and HTTP_X_FORWARDED_HOST , but I assume that it is correct
  • the CI tests failures are not related to the changes in this PR
@tsteur commented on August 30th 2016 Member

Are you waiting for a user test here? LGTM if you think it's working as expected.

I think this is something that we should mention in https://github.com/piwik/piwik/issues/10454 as it may break users system when updating and it would be hard to find.

@mattab commented on September 19th 2016 Member

I think this is something that we should mention in #10454 as it may break users system when updating and it would be hard to find.

:+1:

This Pull Request was closed on September 19th 2016
Powered by GitHub Issue Mirror