ip_address() fails when client request IP and proxy IP are the same

Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."

Tagging as novice for the task of rerolling the Drupal 8.x patch.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

Thanks. :)

I am also using Varnish on D7, and found that Simpletests were frequently and sporadically failing, mostly due to 403 errors. There seemed to be some correlation to how long Varnish had been running, indicating an issue there.

The D7 patch above seems to have solved the problem. thanks!

Nevermind... still having the same errors, I don't think they are related to this patch...

If $untrusted is empty, wouldn't it be better to use the first element in $forwarded rather than just $ip_address?

Assume that you have two proxies, p1 and p2, and you make a request from p1 via p2 to the web server. In this case I think that ip_address() should return the IP address of p1. With you patch it returns the IP address of p2.

In addition to simpletests, another thing that can fail as a result of this is any runtime code that calls drupal_http_request() to make a request between two sites on the same server.

That's where I ran into it (with Deploy, where the drupal_http_request() call uses Services to log in to the site and therefore runs directly into the session saving problem described above too).

The above patch seemed to work fine, at least for a simple setup (i.e. with only one reverse proxy); for more complicated setups it seems like #6 might indeed be an issue.

Adding the backport tag.

7.x-ip_address.patch queued for re-testing.

Uploaded Josh Waihi's original D7 patch with different name. I hope this matches the naming convention for D7 now ...

Oh, it doesn't work that way. In any case the original D7 patch still applies cleanly for me locally.
Re-queueing the latest D8 patch then.

Sorry for the noise.

#2: 8.x-ip_address_reroll.patch queued for re-testing.

I agree with #6. This could indeed be an issue. I attached a new D8 version of a fix.

#10 patch solved an issue I had in D7.
https://drupal.org/node/1985612#comment-7596197

What happened to function ip_address()?

Is this still an issue in D8? If not back to D7.

Came here from @SocialNicheGuru's post on another thread.

Thank you for posting the patch in #10. It solved a Drupal 7 SQL error we would occasionally get from httpBL (https://www.drupal.org/project/httpbl) when running with Varnish:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'hostname' cannot be null: INSERT INTO {httpbl} (hostname, status, expire) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => [:db_insert_placeholder_1] => 0 [:db_insert_placeholder_2] => 1435337201 ) in _httpbl_cache_set() (line 607 of /sites/all/modules/httpbl/httpbl.module).

In Drupal 8, this is handled correctly by Symfony. The attached patch addresses #1327728-6: ip_address() fails when client request IP and proxy IP are the same for Drupal 7.

@Darren Oh, we've added patch #20 to our code. I'll let you know if we notice any issues after it has run for a bit. Thanks.

#20 works for me as well..i guess we need a test for it though

My name is Christian and I am currently checking this out.

Added patch with test.

thanks! can we have a test only patch to illustrate the failure?

Why not?

thanks! #24 looks good to go then

Marked for commit.

#24 is the patch to commit

Committed to 7.x - thanks!

Might be worth a followup to see if those tests should be ported to Drupal 8 also.

I created #2759995: Port tests to Drupal 8 for IP address detection when client request IP address and proxy IP address are the same as a child issue to look into porting the tests to Drupal 8.