Ticket #4602 (closed enhancement: duplicate)

Opened 5 years ago

Last modified 3 years ago

Function for getting the clients IP address

Reported by: hovenko Owned by: anonymous
Priority: normal Milestone:
Component: General Version:
Severity: normal Keywords: has-patch
Cc:

Description

WordPress misses a function to get the IP address of the original client. It is critical to have a function like this when using plugins for voting, users online and when posting comments when WordPress runs on a server behind an HTTP proxy such as Varnish.

It checks if the header X-Forwarded-For is set (available from $_SERVER['HTTP_X_FORWARDED_FOR']) and uses the first address in this list. If it's not set, it uses $_SERVER['REMOTE_ADDR'] instead.

The function also applies a filter called "remote_ip_address" before returning the address.

Attachments

get_ip_address.patch Download (2.3 KB) - added by hovenko 5 years ago.

Change History

hovenko5 years ago

  • Keywords has-patch added

The patch also makes use of the function when new comments are posted.

Related ticket: #4198

Suggest combining these two tickets into one.

comment:3 follow-up: ↓ 5   markjaquith5 years ago

Andy Skelton noted that you can accomplish this without a function, by simply manipulating the superglobal "live" in a plugin.

Furthermore, that gives people the freedom to use HTTP_X_FORWARDED_FOR values or not, instead of forcing them to accept them.

To be fair, if you're behind a proxy, then you have no reason to not use the HTTP_X_FORWARDED_FOR, as the REMOTE_ADDR is *always* going to be the address of the proxy.

hovenko also brings up the valid point that andy's example code doesn't work for cases where you have multiple proxy servers.

This is a decent approach, using a single function to standardize the calls to get the remote IP address, and he also even adds a filter to the result in case you want to roll a different approach in a plugin.

I'd give it a +1.

comment:5 in reply to: ↑ 3   hovenko5 years ago

Replying to markjaquith:

Andy Skelton noted that you can accomplish this without a function, by simply manipulating the superglobal "live" in a plugin.

Manipulating it is not a good solution, far from it. In my world you should never manipulate the $_SERVER (and $_REQUEST and such) because that might/could/would break functionality in plugins or other 3rd party applications.

Furthermore, that gives people the freedom to use HTTP_X_FORWARDED_FOR values or not, instead of forcing them to accept them.

In contrast to the current implementation where WordPress forces users to use REMOTE_ADDR?

I believe WordPress should default to use HTTP_X_FORWARDED_FOR if it's set, to get correct author IP addresses on comments and posts. If someone don't want to use it they could override this behaviour by hooking on the "remote_ip_address". Overriding $_SERVERREMOTE_ADDR? will be just another hack to fix a bug.

If a new function is not wanted, at least a filter should be added and applied in those cases where the remote address is used, but that will be harder to use and maintain.

comment:6   ryan4 years ago

  • Milestone changed from 2.3 to 2.4
  • Milestone changed from 2.5 to 2.6

comment:8 follow-up: ↓ 9   pishmishy4 years ago

  • Status changed from new to closed
  • Resolution set to invalid
  • Milestone 2.6 deleted

I'm not sure that this function will get included if it's only to be used in plugins (you may as well include the function in your plugin).

If there are places where such a function may be useful in the core then please let me know otherwise I'll close this as invalid.

comment:9 in reply to: ↑ 8   Otto424 years ago

  • Status changed from closed to reopened
  • Resolution invalid deleted
  • Milestone set to 2.6

Replying to pishmishy:

I'm not sure that this function will get included if it's only to be used in plugins (you may as well include the function in your plugin).

If there are places where such a function may be useful in the core then please let me know otherwise I'll close this as invalid.

You didn't check the patch, did you? It does use the function in the core, specifically in getting the IP address of the commenters.

Reopening.

I think the patch might need refreshing, I think theres been a change to a few ip sanitizations since then.

Moving the IP sanitisation to the get_ip function would be a good move too IMO, it would simplify moving to IPv6 support internally too.

  • Status changed from reopened to closed
  • Resolution set to duplicate
  • Milestone 2.9 deleted
Note: See TracTickets for help on using tickets.