Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

getRequestingIp() getting local ip address from $_SERVER #12

Closed
samhibberd opened this issue Oct 8, 2018 · 2 comments
Closed

getRequestingIp() getting local ip address from $_SERVER #12

samhibberd opened this issue Oct 8, 2018 · 2 comments

Comments

@samhibberd
Copy link

Hi Selvin,

We have noticed a couple of sites on one of our hosts running into an issue where the getRequestingIp() function is returning a local address '127.0.0.1' rather than the client IP, it looks to be related to inconsistencies between the values stored in $_SERVER, maybe if a proxy is used:

https://github.com/selvinortiz/craft-plugin-patrol/blob/6728e1996c7df4cdf49482841e01cce1f7728149/src/services/PatrolService.php#L286

It looks as though our server (with kyup on cloudways) reports the correct ip in both $_SERVER['HTTP_X_FORWARDED_FOR'] and $_SERVER['HTTP_X_REAL_IP'].

There looks to be a load of different approaches out there, not sure which is the most reliable.

Thanks,

Sam

@selvinortiz
Copy link
Contributor

Sam,
Thank you for bringing this up.

I initially addressed CloudFlare hosting environments but Patrol is being used in many more environments now and we definitely need to figure out how to support proxies, Heroku/Fortrabbit style environments, etc.

I've had limited time to work on my plugins lately and I know it's holding a few people back in some cases.

selvinortiz added a commit that referenced this issue Nov 23, 2018
- Fix issue #12 where the requesting IP could not be determined if behind some proxies
- Fix critical issue where Patrol would explode on sites without a proper baseUrl set
- Updated default value of baseUrl to `app.request.hostInfo`
- Updated documentation for baseUrl

> Thank you, Simon Davies and Chris Rowe for your feedback and PRs
@selvinortiz
Copy link
Contributor

@samhibberd The latest release should address this issue. If you learn otherwise in your testing, please let me know.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants