Make WordPress Core

Opened 18 years ago

Closed 17 years ago

#4198 closed defect (bug) (wontfix)

$_SERVER['REMOTE_ADDR'] is a bad environmental variable for some people

Reported by: technosailor's profile technosailor Owned by: rob1n's profile rob1n
Milestone: Priority: normal
Severity: normal Version: 2.2
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

If a blog is behind a proxy/cache such as squid, the REMOTE_ADDR environmental variable is set to the IP of the proxy. Sometimes this causes problems with IP comment flooding functionality as an example, but also with some plugins that check IP addresses and portions of the core such as comment IP logging. Attached is a patch that checks for HTTP_X_FORWARDED_FOR which is the server variable that is set in PHP when a squid proxy is present. I put the function in pluggable as this variable _might_ be different for other proxies. I have no access to other proxies to test.

Attachments (3)

get-user-real-ip-api.diff (1.6 KB) - added by technosailor 18 years ago.
Replacement
get-user-real-ip-api.2.diff (1.6 KB) - added by technosailor 18 years ago.
Replacement
4198.diff (477 bytes) - added by rob1n 18 years ago.

Download all attachments as: .zip

Change History (11)

#1 @technosailor
18 years ago

Suggest committing for the 2.0.x branch and 2.1.x branch as well.

#2 @foolswisdom
18 years ago

  • Keywords has-patch added
  • Milestone changed from 2.2 to 2.3
  • Version set to 2.2

#3 @rob1n
18 years ago

  • Keywords 2nd-opinion added

Makes sense to me. I'd go for just 2.0, 2.2 and trunk. 2.1 is going to be discontinued once 2.2 is out, but 2.0 is still there for Debianic purposes.

@technosailor
18 years ago

Replacement

@technosailor
18 years ago

Replacement

#4 @technosailor
18 years ago

Someone wanna help me understand whats wrong with my diff now that HTML display won't work? I'm confused and willing to adjust what I'm doing if I know what it is I'm doing.

#5 @andy
18 years ago

I am not in favor of this solution as a core patch. Most blogs are not so affected and would see more harm than good from this patch.

Those who use proxies know who they are and should handle it themselves as needed. I suggest adding a plugin or modifying your own wp-config.php to include something like the following:

if ( !empty( $_SERVER['HTTP_X_FORWARDED_FOR'] ) )
    $_SERVER['REMOTE_ADDR'] = $_SERVER['HTTP_X_FORWARDED_FOR'];

@rob1n
18 years ago

#6 @rob1n
18 years ago

  • Keywords 2nd-opinion removed
  • Owner changed from technosailor to rob1n
  • Status changed from new to assigned

#7 @hovenko
17 years ago

The patch attached to this bug report will (at least might) break functionality that depends on REMOTE_ADDR. Also it does not handle more than one proxy server.

When using two or more proxy servers you will get a comma separated list with all the IP addresses. Eks: "client, proxy1, proxy2".

Bug report #4602 adds a new function, wp_get_ip_address(), that solves this.

#8 @markjaquith
17 years ago

  • Milestone 2.3 deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

This needs to be handled on a server-by-server basis, for reasons stated above.

Note: See TracTickets for help on using tickets.