Make WordPress Core

Opened 16 years ago

Closed 14 years ago

#4602 closed enhancement (duplicate)

Function for getting the clients IP address

Reported by: hovenko's profile hovenko Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: 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 (1)

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

Download all attachments as: .zip

Change History (13)

#1 @hovenko
16 years ago

  • Keywords has-patch added

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

#2 @Otto42
16 years ago

Related ticket: #4198

Suggest combining these two tickets into one.

#3 follow-up: @markjaquith
16 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.

#4 @Otto42
16 years ago

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.

#5 in reply to: ↑ 3 @hovenko
16 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.

#6 @ryan
16 years ago

  • Milestone changed from 2.3 to 2.4

#7 @ffemtcj
15 years ago

  • Milestone changed from 2.5 to 2.6

#8 follow-up: @pishmishy
15 years ago

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

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.

#9 in reply to: ↑ 8 @Otto42
15 years ago

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

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.

#10 @DD32
15 years ago

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.

#12 @Denis-de-Bernardy
14 years ago

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