r94577 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r94576‎ | r94577 | r94578 >
Date:21:45, 15 August 2011
Author:hashar
Status:reverted (Comments)
Tags:
Comment:
back off r94558:
- reverts ./includes/ProxyTools.php
- marks tests broken

In our test suite, the first call to wfGetIP() set the static variable.
Hence the remaining of the code is only tested on the first call to it.
Resetting the static variable enlight a bug somewhere in our code where
we are calling wfGetIP() but can not reliably get an IP, somehow REMOTE_ADDR
does not exist and $wgCommandLineMode is disabled.
Will have to track this bug further when I got time.
Modified paths:
  • /trunk/phase3/includes/ProxyTools.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/ProxyTools/wfGetIPTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/ProxyTools/wfGetIPTest.php
——@@ -1,6 +1,10 @@
22 <?php
33 /*
44 * Unit tests for wfGetIP()
 5+ *
 6+ * FIXME: r94558 cause some test to throw an exception cause
 7+ * wgCommandLine is not set :-) Resetting the static variable
 8+ * enlight a bug that is hidden by a previous call to wfGetIP().
59 *
610 * TODO: need to cover the part dealing with XFF headers.
711 */
——@@ -24,6 +28,9 @@
2529 $msg );
2630 }
2731
 32+ /**
 33+ * @group Broken
 34+ */
2835 function testGetLoopbackAddressWhenInCommandLine() {
2936 global $wgCommandLineMode;
3037 $save = $wgCommandLineMode;
——@@ -67,6 +74,7 @@
6875
6976 /**
7077 * @expectedException MWException
 78+ * @group Broken
7179 */
7280 function testLackOfRemoteAddrThrowAnException() {
7381 global $wgCommandLineMode;
Index: trunk/phase3/includes/ProxyTools.php
——@@ -64,18 +64,12 @@
6565 /**
6666 * Work out the IP address based on various globals
6767 * For trusted proxies, use the XFF client IP (first of the chain)
68 - * @param $reset boolean Used to reset the internal static variable
69 - * tracking the IP address. (default: false)
7068 * @return string
7169 */
72 -function wfGetIP( $reset = false ) {
 70+function wfGetIP() {
7371 global $wgUsePrivateIPs, $wgCommandLineMode;
7472 static $ip = false;
7573
76 - if( $reset ) {
77 - $ip = false;
78 - }
79 -
8074 # Return cached result
8175 if ( !empty( $ip ) ) {
8276 return $ip;

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r94558Tests for wfGetIP() follow up r89407...hashar20:16, 15 August 2011

Comments

#Comment by Hashar (talk | contribs)   21:50, 15 August 2011

safer to back off this change to includes/ProxyTools.php :-) Next time I will run the full test suite before committing.

#Comment by Hashar (talk | contribs)   14:16, 16 August 2011

r94558 reintroduced with r94638 which is a reversion of this r94577.

Status & tagging log