ip_address function doesn't detect the correct IP

I read your blog post at http://blog.mohanjith.net/2008/02/running-drupal-behind-reverse-proxy.html, which had a link to this issue. I don't have a configuration where I can test this, but note that there are a few very minor code style violations which will have to be addressed (namely, indentation and periods at the end of comments). There may be others as well -- see http://drupal.org/coding-standards.

keith.smith thanks for having a look at the patch.

I have read the coding standards documentation again and made the necessary changes to be compliant with the Drupal coding style. Hope I haven't looked over any other changes necessary.

Rerolled the patch with the changes against the latest HEAD.

There are a few points that still need to be changed.

1.) Do what ever to is vague and confusing. I guess what you are doing is Reset the cached client IP address.

2.) The description of the $force parameter is indented one space too much - compare it with the line below @return.

3.) $force is not descriptive. As far as I know, other functions with a resetting parameter name it $reset or $refresh.

4.) The ip_address function claims it is resets the static variable because that fixes a problem. Not true - it resets it because it was told to by a parameter. In line with the causality, the explanation of why this is needed should go to the function call of ip_address(TRUE);.

5.) true should be capitalized according to code-style, I think.

Here is a revised patch that corrects the above points.

Also,

can be rewritten to

The definition of the function still refers to $force. Should this be now $reset ?

function ip_address($reset = FALSE) { ... }

Paddy.

Right.

I'm certainly not an expert in reverse proxying, neither can I test this right now.
Still it seems to me the place where $ip_address is forcedly recalculated is not the right one. mohanjith is doing this after a full bootstrap, while in fact the recalculated IP address is already needed in DRUPAL_BOOTSTRAP_ACCESS.
I moved it there, corrected codestyle and improved the documentation.
Tested this with my means, so this certainly doesn't break environments without a reverse proxy. Otherwise, this remains to be tested.

edit: Unfortunately, I've been crossing all the posts 2-6. The idea remains to be valid.

Another version of #7, reflecting the contributions #2-#6.

How is a static cached in between requests?

The only reason I can see is that ip_address is called the first time before the variables (for variable_get) have been loaded.

I think this issue is bogus.

I've been discussing this with chx:

Despite variable_init not beeing called until DRUPAL_BOOTSTRAP_LATE_PAGE_CACHE, the config option for the ip option is already read from settings.php in DRUPAL_BOOTSTRAP_CONFIGURATION in conf_init.

Won't fix.

I ran into the issue as well, but killes is correct in stating that it's bogus. For the benefit of anybody else who might be scratching their heads: the reason I ran into this issue is because I set the 'reverse_proxy' and 'reverse_proxy_addresses' variables in the database (it was difficult to get access to the file system on the server). However, since the first invocation of ip_address() occurs in the bootstrap before the call to variable_init() (which actually reads the contents of the variables db table), the ip_address() function doesn't have access to any of the database variables. It only has access to values set on the global $conf object, which you can configure in.... your settings.php file.

In short, you can only configure Reverse Proxy settings from settings.php, NOT via the variables database table.