Skip to content
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

Resolves #2070 Add new x-remote-addr var to the get_ip() function #3400

Open
wants to merge 1 commit into
base: feature
Choose a base branch
from
Open

Resolves #2070 Add new x-remote-addr var to the get_ip() function #3400

wants to merge 1 commit into from

Conversation

effone
Copy link
Member

@effone effone commented Aug 18, 2018

Attempt to resolve #2070

Task List:

  • Modify get_ip( ) function to handle various headers
  • Use PHP filters (and flags) to validate IP in place of REGEX
  • Cover CloudFlare IP override
  • Add ACP Setting for users to select desired headers

@dvz @euantorano (or some expert/s) please review the ordering in $filter_headers array. This is important as the first match will be considered to return.

@effone effone added b:1.8 Branch: 1.8.x s:in-progress Status: In Progress. Some work completed labels Aug 18, 2018
@effone effone requested review from euantorano and dvz August 18, 2018 09:34
@effone
Copy link
Member Author

effone commented Aug 18, 2018

The ACP settings will be created after review / confirmation for considered headers and their check sequence.

@dvz
Copy link
Member

dvz commented Aug 19, 2018

A setting should be used as the first and only source for IP addresses, perhaps with a fallback to $_SERVER['REMOTE_ADDR']. The value in chosen header could be validated when settings are saved.

The actual list for attempts could be included in the installation process - see https://community.mybb.com/thread-219338.html

$filter_headers = array(
'HTTP_X_FORWARDED_FOR',
'HTTP_CLIENT_IP',
'HTTP_CF_CONNECTING_IP ', // Cloudflare Proxy
Copy link

@skmedix skmedix Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove space from the end of the string

Suggested change
'HTTP_CF_CONNECTING_IP ', // Cloudflare Proxy
'HTTP_CF_CONNECTING_IP', // Cloudflare Proxy

@Eldenroot
Copy link
Contributor

@effone - will you finish this one? Thx

@effone
Copy link
Member Author

effone commented Nov 9, 2018

Yes @Eldenroot , need to rewrite the PR, cause the approach outlined by @dvz is different from what it is now.

@Ben-MyBB
Copy link
Member

@effone Ping! Reminder about this PR :)

@effone
Copy link
Member Author

effone commented May 29, 2019

The actual list for attempts could be included in the installation process

Failed to understand this part @dvz

@dvz
Copy link
Member

dvz commented May 29, 2019

Fixed the link (https://community.mybb.com/thread-219338.html) - when the installer is rewritten, the header name can be detected automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x s:in-progress Status: In Progress. Some work completed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Add new x-remote-addr var to the get_ip() function
5 participants