In a scenario where Caching mode is set to Normal and the Drupal instance is behind a proxy ip_address function reports the Proxy server's IP address.
Setup:
| Client | --> | Proxy | --> | Origin |
I did not test in the setup where Caching mode is disabled or set to Aggressive.
In the ip_address function, declaring $ip_address as static and then testing whether it is already set passes at all times and thus no attempt is made to find the client's IP.
In the patch I have introduced a new optional parameter $force that will force the function to reset $ip_address and hence calculate the client's IP. In index.php ip_address with $force set to true will be calledl Irrespective of any cache the ip will be re-calculated at every request, hence getting the correct client IP.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | ip_address_force_recalculation_8.patch | 1.96 KB | pancho |
| #7 | ip_address_force_recalculation.patch | 1.79 KB | pancho |
| #6 | reset-proxy-ip-219825-6.patch | 1.58 KB | cburschka |
| #4 | reset-proxy-ip-219825-4.patch | 1.58 KB | cburschka |
| #2 | proxy.patch | 1.61 KB | mohanjith |
Comments
Comment #1
keith.smith commentedI 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.
Comment #2
mohanjith commentedkeith.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.
Comment #3
cburschkaThere are a few points that still need to be changed.
1.)
Do what ever tois vague and confusing. I guess what you are doing isReset the cached client IP address.2.) The description of the
$forceparameter is indented one space too much - compare it with the line below@return.3.)
$forceis not descriptive. As far as I know, other functions with a resetting parameter name it$resetor$refresh.4.) The
ip_addressfunction 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 ofip_address(TRUE);.5.)
trueshould be capitalized according to code-style, I think.Comment #4
cburschkaHere is a revised patch that corrects the above points.
Also,
can be rewritten to
Comment #5
paddy_deburca commentedThe definition of the function still refers to
$force. Should this be now$reset?function ip_address($reset = FALSE) { ... }Paddy.
Comment #6
cburschkaRight.
Comment #7
panchoI'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.
Comment #8
panchoAnother version of #7, reflecting the contributions #2-#6.
Comment #9
heine commentedHow 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.
Comment #10
killes@www.drop.org commentedI 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.
Comment #11
neilotoole commentedI 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.