Skip to content

fix: Request::getIPAddress() #6820

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

Merged
merged 7 commits into from
Nov 7, 2022

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Nov 6, 2022

Description

  • use header() instead of getServer() for header values
    • getIPAddress() does not work with OpenSwoole.

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • [] User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 6, 2022
@kenjis kenjis marked this pull request as draft November 6, 2022 01:26
@MGatner
Copy link
Member

MGatner commented Nov 6, 2022

Meant to "leave a comment".

@kenjis kenjis force-pushed the fix-Request-getIPAddress branch from 3a224c1 to ead7757 Compare November 7, 2022 07:45
@AmitSonkhiya
Copy link

AmitSonkhiya commented Nov 7, 2022

I have copied this code snippet for the getIPAddress() method and it is now correctly determined the IP address based on x-forwarded-for.
Just I had to add 127.0.0.1 in the $proxyIPs property of the App class since the server resides in the internet space.
It would be great if x-real-ip could be checked as well in the method.

@kenjis kenjis marked this pull request as ready for review November 7, 2022 23:17
@kenjis
Copy link
Member Author

kenjis commented Nov 7, 2022

@AmitSonkhiya Thank you for testing!

@kenjis kenjis merged commit 76000ab into codeigniter4:develop Nov 7, 2022
@kenjis kenjis deleted the fix-Request-getIPAddress branch November 7, 2022 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants