Moved wfGetIP() to WebRequest::getIP():
authorAlexandre Emsenhuber <ialex@users.mediawiki.org>
Thu, 18 Aug 2011 20:03:30 +0000 (20:03 +0000)
committerAlexandre Emsenhuber <ialex@users.mediawiki.org>
Thu, 18 Aug 2011 20:03:30 +0000 (20:03 +0000)
* Changed all calls in core to the latter
* Also marked wfGetForwardedFor() as deprecated
* Moved wfGetIP() tests to WebRequestTest

17 files changed:
api.php
includes/Autopromote.php
includes/EditPage.php
includes/Exception.php
includes/ProxyTools.php
includes/RecentChange.php
includes/SkinLegacy.php
includes/Title.php
includes/User.php
includes/WebRequest.php
includes/specials/SpecialBlockme.php
includes/specials/SpecialPasswordReset.php
includes/specials/SpecialUserlogin.php
includes/specials/SpecialVersion.php
tests/phpunit/includes/ProxyTools/README [deleted file]
tests/phpunit/includes/ProxyTools/wfGetIPTest.php [deleted file]
tests/phpunit/includes/WebRequestTest.php

diff --git a/api.php b/api.php
index 39d27dd..7aef1eb 100644 (file)
--- a/api.php
+++ b/api.php
@@ -127,7 +127,7 @@ if ( $wgAPIRequestLog ) {
        $items = array(
                        wfTimestamp( TS_MW ),
                        $endtime - $starttime,
-                       wfGetIP(),
+                       $wgRequest->getIP(),
                        $_SERVER['HTTP_USER_AGENT']
        );
        $items[] = $wgRequest->wasPosted() ? 'POST' : 'GET';
index 83f3c20..a233603 100644 (file)
@@ -166,9 +166,9 @@ class Autopromote {
                                $groups = array_slice( $cond, 1 );
                                return count( array_intersect( $groups, $user->getGroups() ) ) == count( $groups );
                        case APCOND_ISIP:
-                               return $cond[1] == wfGetIP();
+                               return $cond[1] == $user->getRequest()->getIP();
                        case APCOND_IPINRANGE:
-                               return IP::isInRange( wfGetIP(), $cond[1] );
+                               return IP::isInRange( $user->getRequest()->getIP(), $cond[1] );
                        case APCOND_BLOCKED:
                                return $user->isBlocked();
                        case APCOND_ISBOT:
index bf79b09..28032da 100644 (file)
@@ -856,7 +856,7 @@ class EditPage {
         * @return int one of the constants describing the result
         */
        function internalAttemptSave( &$result, $bot = false ) {
-               global $wgFilterCallback, $wgUser, $wgParser;
+               global $wgFilterCallback, $wgUser, $wgRequest, $wgParser;
                global $wgMaxArticleSize;
 
                wfProfileIn( __METHOD__  );
@@ -888,7 +888,7 @@ class EditPage {
                }
                if ( $match !== false ) {
                        $result['spam'] = $match;
-                       $ip = wfGetIP();
+                       $ip = $wgRequest->getIP();
                        $pdbk = $this->mTitle->getPrefixedDBkey();
                        $match = str_replace( "\n", '', $match );
                        wfDebugLog( 'SpamRegex', "$ip spam regex hit [[$pdbk]]: \"$match\"" );
index 5a972ca..7660060 100644 (file)
@@ -352,7 +352,7 @@ class ThrottledError extends ErrorPageError {
  */
 class UserBlockedError extends ErrorPageError {
        public function __construct( Block $block ){
-               global $wgLang;
+               global $wgLang, $wgRequest;
 
                $blockerUserpage = $block->getBlocker()->getUserPage();
                $link = "[[{$blockerUserpage->getPrefixedText()}|{$blockerUserpage->getText()}]]";
@@ -372,7 +372,7 @@ class UserBlockedError extends ErrorPageError {
                        array(
                                $link,
                                $reason,
-                               wfGetIP(),
+                               $wgRequest->getIP(),
                                $block->getBlocker()->getName(),
                                $block->getId(),
                                $wgLang->formatExpiry( $block->mExpiry ),
index ab9beff..83f026f 100644 (file)
@@ -8,29 +8,13 @@
 /**
  * Extracts the XFF string from the request header
  * Note: headers are spoofable
+ *
+ * @deprecated in 1.19; use $wgRequest->getHeader( 'X-Forwarded-For' ) instead.
  * @return string
  */
 function wfGetForwardedFor() {
-       $apacheHeaders = function_exists( 'apache_request_headers' ) ? apache_request_headers() : null;
-       if( is_array( $apacheHeaders ) ) {
-               // More reliable than $_SERVER due to case and -/_ folding
-               $set = array();
-               foreach ( $apacheHeaders as $tempName => $tempValue ) {
-                       $set[ strtoupper( $tempName ) ] = $tempValue;
-               }
-               $index = strtoupper ( 'X-Forwarded-For' );
-       } else {
-               // Subject to spoofing with headers like X_Forwarded_For
-               $set = $_SERVER;
-               $index = 'HTTP_X_FORWARDED_FOR';
-       }
-
-       #Try to see if XFF is set
-       if( isset( $set[$index] ) ) {
-               return $set[$index];
-       } else {
-               return null;
-       }
+       global $wgRequest;
+       return $wgRequest->getHeader( 'X-Forwarded-For' );
 }
 
 /**
@@ -42,88 +26,20 @@ function wfGetForwardedFor() {
  */
 function wfGetAgent() {
        wfDeprecated( __FUNCTION__ );
-       if( function_exists( 'apache_request_headers' ) ) {
-               // More reliable than $_SERVER due to case and -/_ folding
-               $set = array();
-               foreach ( apache_request_headers() as $tempName => $tempValue ) {
-                       $set[ strtoupper( $tempName ) ] = $tempValue;
-               }
-               $index = strtoupper ( 'User-Agent' );
-       } else {
-               // Subject to spoofing with headers like X_Forwarded_For
-               $set = $_SERVER;
-               $index = 'HTTP_USER_AGENT';
-       }
-       if( isset( $set[$index] ) ) {
-               return $set[$index];
-       } else {
-               return '';
-       }
+       global $wgRequest;
+       return $wgRequest->getHeader( 'User-Agent' );
 }
 
 /**
  * Work out the IP address based on various globals
  * For trusted proxies, use the XFF client IP (first of the chain)
- * @param $reset boolean Used to reset the internal static variable
- * tracking the IP address. Set to anything non empty to reset it, for
- * example: wfGetIP( 'reset' ); (default: false).
+ *
+ * @deprecated in 1.19; call $wgRequest->getIP() directly.
  * @return string
  */
-function wfGetIP( $reset = false ) {
-       global $wgUsePrivateIPs, $wgCommandLineMode;
-       static $ip = false;
-
-       if( $reset ) {
-               $ip = false;
-       }
-
-       # Return cached result
-       if ( !empty( $ip ) ) {
-               return $ip;
-       }
-
-       /* collect the originating ips */
-       # Client connecting to this webserver
-       if ( isset( $_SERVER['REMOTE_ADDR'] ) ) {
-               $ip = IP::canonicalize( $_SERVER['REMOTE_ADDR'] );
-       } elseif( $wgCommandLineMode ) {
-               $ip = '127.0.0.1';
-       }
-
-       # Append XFF
-       $forwardedFor = wfGetForwardedFor();
-       if ( $forwardedFor !== null ) {
-               $ipchain = array_map( 'trim', explode( ',', $forwardedFor ) );
-               $ipchain = array_reverse( $ipchain );
-               if ( $ip ) {
-                       array_unshift( $ipchain, $ip );
-               }
-
-               # Step through XFF list and find the last address in the list which is a trusted server
-               # Set $ip to the IP address given by that trusted server, unless the address is not sensible (e.g. private)
-               foreach ( $ipchain as $i => $curIP ) {
-                       $curIP = IP::canonicalize( $curIP );
-                       if ( wfIsTrustedProxy( $curIP ) ) {
-                               if ( isset( $ipchain[$i + 1] ) ) {
-                                       if( $wgUsePrivateIPs || IP::isPublic( $ipchain[$i + 1 ] ) ) {
-                                               $ip = $ipchain[$i + 1];
-                                       }
-                               }
-                       } else {
-                               break;
-                       }
-               }
-       }
-
-       # Allow extensions to improve our guess
-       wfRunHooks( 'GetIP', array( &$ip ) );
-
-       if( !$ip ) {
-               throw new MWException( "Unable to determine IP" );
-       }
-
-       wfDebug( "IP: $ip\n" );
-       return $ip;
+function wfGetIP() {
+       global $wgRequest;
+       return $wgRequest->getIP();
 }
 
 /**
@@ -148,14 +64,14 @@ function wfIsTrustedProxy( $ip ) {
  */
 function wfProxyCheck() {
        global $wgBlockOpenProxies, $wgProxyPorts, $wgProxyScriptPath;
-       global $wgMemc, $wgProxyMemcExpiry;
+       global $wgMemc, $wgProxyMemcExpiry, $wgRequest;
        global $wgProxyKey;
 
        if ( !$wgBlockOpenProxies ) {
                return;
        }
 
-       $ip = wfGetIP();
+       $ip = $wgRequest->getIP();
 
        # Get MemCached key
        $mcKey = wfMemcKey( 'proxy', 'ip', $ip );
index 518444d..a3deb91 100644 (file)
@@ -361,8 +361,9 @@ class RecentChange {
         */
        public static function notifyEdit( $timestamp, &$title, $minor, &$user, $comment, $oldId,
                $lastTimestamp, $bot, $ip='', $oldSize=0, $newSize=0, $newId=0, $patrol=0 ) {
+               global $wgRequest;
                if( !$ip ) {
-                       $ip = wfGetIP();
+                       $ip = $wgRequest->getIP();
                        if( !$ip ) $ip = '';
                }
 
@@ -424,8 +425,9 @@ class RecentChange {
         */
        public static function notifyNew( $timestamp, &$title, $minor, &$user, $comment, $bot,
                $ip='', $size=0, $newId=0, $patrol=0 ) {
+               global $wgRequest;
                if( !$ip ) {
-                       $ip = wfGetIP();
+                       $ip = $wgRequest->getIP();
                        if( !$ip ) $ip = '';
                }
 
@@ -484,7 +486,7 @@ class RecentChange {
                $overRedir = false ) {
                global $wgRequest;
                if( !$ip ) {
-                       $ip = wfGetIP();
+                       $ip = $wgRequest->getIP();
                        if( !$ip ) $ip = '';
                }
 
@@ -565,7 +567,7 @@ class RecentChange {
                $type, $action, $target, $logComment, $params, $newId=0 ) {
                global $wgRequest;
                if( !$ip ) {
-                       $ip = wfGetIP();
+                       $ip = $wgRequest->getIP();
                        if( !$ip ) {
                                $ip = '';
                        }
index 5311634..6fbb465 100644 (file)
@@ -889,7 +889,7 @@ class LegacyTemplate extends BaseTemplate {
        }
 
        function nameAndLogin() {
-               global $wgUser, $wgLang, $wgContLang;
+               global $wgUser, $wgLang, $wgRequest, $wgContLang;
 
                $logoutPage = $wgContLang->specialPage( 'Userlogout' );
 
@@ -897,7 +897,7 @@ class LegacyTemplate extends BaseTemplate {
 
                if ( $wgUser->isAnon() ) {
                        if ( $this->getSkin()->showIPinHeader() ) {
-                               $name = wfGetIP();
+                               $name = $wgRequest->getIP();
 
                                $talkLink = Linker::link( $wgUser->getTalkPage(),
                                        $wgLang->getNsText( NS_TALK ) );
index c58fff8..f9a61e6 100644 (file)
@@ -1562,7 +1562,7 @@ class Title {
                        if ( $reason == '' ) {
                                $reason = wfMsg( 'blockednoreason' );
                        }
-                       $ip = wfGetIP();
+                       $ip = $user->getRequest()->getIP();
 
                        if ( is_numeric( $id ) ) {
                                $name = User::whoIs( $id );
index 47e2462..6befcd2 100644 (file)
@@ -1259,7 +1259,7 @@ class User {
                # user is not immune to autoblocks/hardblocks, and they are the current user so we
                # know which IP address they're actually coming from
                if ( !$this->isAllowed( 'ipblock-exempt' ) && $this->getID() == $wgUser->getID() ) {
-                       $ip = wfGetIP();
+                       $ip = $this->getRequest()->getIP();
                } else {
                        $ip = null;
                }
@@ -1409,7 +1409,7 @@ class User {
         */
        public function isPingLimitable() {
                global $wgRateLimitsExcludedIPs;
-               if( in_array( wfGetIP(), $wgRateLimitsExcludedIPs ) ) {
+               if( in_array( $this->getRequest()->getIP(), $wgRateLimitsExcludedIPs ) ) {
                        // No other good way currently to disable rate limits
                        // for specific IPs. :P
                        // But this is a crappy hack and should die.
@@ -1450,7 +1450,7 @@ class User {
                $limits = $wgRateLimits[$action];
                $keys = array();
                $id = $this->getId();
-               $ip = wfGetIP();
+               $ip = $this->getRequet()->getIP();
                $userLimit = false;
 
                if( isset( $limits['anon'] ) && $id == 0 ) {
@@ -1607,7 +1607,7 @@ class User {
                if( IP::isIPAddress( $this->getName() ) ) {
                        $ip = $this->getName();
                } elseif( !$ip ) {
-                       $ip = wfGetIP();
+                       $ip = $this->getRequest()->getIP();
                }
                $blocked = false;
                wfRunHooks( 'UserIsBlockedGlobally', array( &$this, $ip, &$blocked ) );
@@ -1685,7 +1685,7 @@ class User {
                        $this->load();
                        if ( $this->mName === false ) {
                                # Clean up IPs
-                               $this->mName = IP::sanitizeIP( wfGetIP() );
+                               $this->mName = IP::sanitizeIP( $this->getRequest()->getIP() );
                        }
                        return $this->mName;
                }
@@ -2943,7 +2943,7 @@ class User {
                        return;
                }
 
-               $userblock->doAutoblock( wfGetIP() );
+               $userblock->doAutoblock( $this->getRequest()->getIP() );
        }
 
        /**
@@ -3012,7 +3012,7 @@ class User {
                # blocked with createaccount disabled, prevent new account creation there even
                # when the user is logged in
                if( $this->mBlockedFromCreateAccount === false ){
-                       $this->mBlockedFromCreateAccount = Block::newFromTarget( null, wfGetIP() );
+                       $this->mBlockedFromCreateAccount = Block::newFromTarget( null, $this->getRequest()->getIP() );
                }
                return $this->mBlockedFromCreateAccount instanceof Block && $this->mBlockedFromCreateAccount->prevents( 'createaccount' )
                        ? $this->mBlockedFromCreateAccount
@@ -3228,7 +3228,7 @@ class User {
 
                return $this->sendMail( wfMsg( 'confirmemail_subject' ),
                        wfMsg( $message,
-                               wfGetIP(),
+                               $this->getRequest()->getIP(),
                                $this->getName(),
                                $url,
                                $wgLang->timeanddate( $expiration, false ),
index 6c20324..48472c9 100644 (file)
@@ -44,6 +44,12 @@ class WebRequest {
         */
        private $response;
 
+       /**
+        * Cached client IP address
+        * @var String
+        */
+       private $ip;
+
        public function __construct() {
                /// @todo FIXME: This preemptive de-quoting can interfere with other web libraries
                ///        and increases our memory footprint. It would be cleaner to do on
@@ -962,6 +968,72 @@ HTML;
                arsort( $langs, SORT_NUMERIC );
                return $langs;
        }
+
+       /**
+        * Fetch the raw IP from the request
+        *
+        * @return String
+        */
+       protected function getRawIP() {
+               if ( isset( $_SERVER['REMOTE_ADDR'] ) ) {
+                       return IP::canonicalize( $_SERVER['REMOTE_ADDR'] );
+               } else {
+                       return null;
+               }
+       }
+
+       /**
+        * Work out the IP address based on various globals
+        * For trusted proxies, use the XFF client IP (first of the chain)
+        * @return string
+        */
+       public function getIP() {
+               global $wgUsePrivateIPs;
+
+               # Return cached result
+               if ( $this->ip !== null ) {
+                       return $this->ip;
+               }
+
+               # collect the originating ips
+               $ip = $this->getRawIP();
+
+               # Append XFF
+               $forwardedFor = $this->getHeader( 'X-Forwarded-For' );
+               if ( $forwardedFor !== false ) {
+                       $ipchain = array_map( 'trim', explode( ',', $forwardedFor ) );
+                       $ipchain = array_reverse( $ipchain );
+                       if ( $ip ) {
+                               array_unshift( $ipchain, $ip );
+                       }
+
+                       # Step through XFF list and find the last address in the list which is a trusted server
+                       # Set $ip to the IP address given by that trusted server, unless the address is not sensible (e.g. private)
+                       foreach ( $ipchain as $i => $curIP ) {
+                               $curIP = IP::canonicalize( $curIP );
+                               if ( wfIsTrustedProxy( $curIP ) ) {
+                                       if ( isset( $ipchain[$i + 1] ) ) {
+                                               if ( $wgUsePrivateIPs || IP::isPublic( $ipchain[$i + 1 ] ) ) {
+                                                       $ip = $ipchain[$i + 1];
+                                               }
+                                       }
+                               } else {
+                                       break;
+                               }
+                       }
+               }
+
+               # Allow extensions to improve our guess
+               wfRunHooks( 'GetIP', array( &$ip ) );
+
+               if ( !$ip ) {
+                       throw new MWException( "Unable to determine IP" );
+               }
+
+               wfDebug( "IP: $ip\n" );
+               $this->ip = $ip;
+               return $ip;
+       }
 }
 
 /**
@@ -1165,4 +1237,8 @@ class FauxRequest extends WebRequest {
        public function checkUrlExtension( $extWhitelist = array() ) {
                return true;
        }
+
+       protected function getRawIP() {
+               return '127.0.0.1';
+       }
 }
index 4074766..66be933 100644 (file)
@@ -38,7 +38,7 @@ class SpecialBlockme extends UnlistedSpecialPage {
                $this->setHeaders();
                $this->outputHeader();
 
-               $ip = wfGetIP();
+               $ip = $wgRequest->getIP();
                if( !$wgBlockOpenProxies || $wgRequest->getText( 'ip' ) != md5( $ip . $wgProxyKey ) ) {
                        $wgOut->addWikiMsg( 'proxyblocker-disabled' );
                        return;
index b427403..476ab05 100644 (file)
@@ -162,7 +162,7 @@ class SpecialPasswordReset extends FormSpecialPage {
 
                // We need to have a valid IP address for the hook, but per bug 18347, we should
                // send the user's name if they're logged in.
-               $ip = wfGetIP();
+               $ip = $this->getRequest()->getIP();
                if ( !$ip ) {
                        return array( 'badipaddress' );
                }
index a933041..ee4d617 100644 (file)
@@ -251,7 +251,7 @@ class LoginForm extends SpecialPage {
         * @private
         */
        function addNewAccountInternal() {
-               global $wgUser, $wgOut;
+               global $wgUser, $wgOut, $wgRequest;
                global $wgMemc, $wgAccountCreationThrottle;
                global $wgAuth, $wgMinimalPasswordLength;
                global $wgEmailConfirmToEdit;
@@ -308,7 +308,7 @@ class LoginForm extends SpecialPage {
                        return false;
                }
 
-               $ip = wfGetIP();
+               $ip = $wgRequest->getIP();
                if ( $wgUser->isDnsBlacklisted( $ip, true /* check $wgProxyWhitelist */ ) ) {
                        $this->mainLoginForm( wfMsg( 'sorbs_create_account_reason' ) . ' (' . htmlspecialchars( $ip ) . ')' );
                        return false;
@@ -588,12 +588,12 @@ class LoginForm extends SpecialPage {
         * @return Bool|Integer The integer hit count or True if it is already at the limit
         */
        public static function incLoginThrottle( $username ) {
-               global $wgPasswordAttemptThrottle, $wgMemc;
+               global $wgPasswordAttemptThrottle, $wgMemc, $wgRequest;
                $username = trim( $username ); // sanity
 
                $throttleCount = 0;
                if ( is_array( $wgPasswordAttemptThrottle ) ) {
-                       $throttleKey = wfMemcKey( 'password-throttle', wfGetIP(), md5( $username ) );
+                       $throttleKey = wfMemcKey( 'password-throttle', $wgRequest->getIP(), md5( $username ) );
                        $count = $wgPasswordAttemptThrottle['count'];
                        $period = $wgPasswordAttemptThrottle['seconds'];
 
@@ -616,10 +616,10 @@ class LoginForm extends SpecialPage {
         * @return void
         */
        public static function clearLoginThrottle( $username ) {
-               global $wgMemc;
+               global $wgMemc, $wgRequest;
                $username = trim( $username ); // sanity
 
-               $throttleKey = wfMemcKey( 'password-throttle', wfGetIP(), md5( $username ) );
+               $throttleKey = wfMemcKey( 'password-throttle', $wgRequest->getIP(), md5( $username ) );
                $wgMemc->delete( $throttleKey );
        }
 
@@ -682,7 +682,7 @@ class LoginForm extends SpecialPage {
        }
 
        function processLogin() {
-               global $wgUser;
+               global $wgUser, $wgRequest, $wgLang;
 
                switch ( $this->authenticateUserData() ) {
                        case self::SUCCESS:
@@ -697,7 +697,7 @@ class LoginForm extends SpecialPage {
                                self::clearLoginToken();
 
                                // Reset the throttle
-                               $key = wfMemcKey( 'password-throttle', wfGetIP(), md5( $this->mUsername ) );
+                               $key = wfMemcKey( 'password-throttle', $wgRequest->getIP(), md5( $this->mUsername ) );
                                global $wgMemc;
                                $wgMemc->delete( $key );
 
@@ -705,7 +705,6 @@ class LoginForm extends SpecialPage {
                                        /* Replace the language object to provide user interface in
                                         * correct language immediately on this first page load.
                                         */
-                                       global $wgLang, $wgRequest;
                                        $code = $wgRequest->getVal( 'uselang', $wgUser->getOption( 'language' ) );
                                        $wgLang = Language::factory( $code );
                                        return $this->successfulLogin();
@@ -779,12 +778,12 @@ class LoginForm extends SpecialPage {
         * @private
         */
        function mailPasswordInternal( $u, $throttle = true, $emailTitle = 'passwordremindertitle', $emailText = 'passwordremindertext' ) {
-               global $wgServer, $wgScript, $wgUser, $wgNewPasswordExpiry;
+               global $wgServer, $wgScript, $wgUser, $wgNewPasswordExpiry, $wgRequest;
 
                if ( $u->getEmail() == '' ) {
                        return Status::newFatal( 'noemail', $u->getName() );
                }
-               $ip = wfGetIP();
+               $ip = $wgRequest->getIP();
                if( !$ip ) {
                        return Status::newFatal( 'badipaddress' );
                }
index 6dba691..4651fe6 100644 (file)
@@ -502,7 +502,7 @@ class SpecialVersion extends SpecialPage {
         * @return String: HTML fragment
         */
        private function IPInfo() {
-               $ip =  str_replace( '--', ' - ', htmlspecialchars( wfGetIP() ) );
+               $ip =  str_replace( '--', ' - ', htmlspecialchars( $this->getRequest()->getIP() ) );
                return "<!-- visited from $ip -->\n" .
                        "<span style='display:none'>visited from $ip</span>";
        }
diff --git a/tests/phpunit/includes/ProxyTools/README b/tests/phpunit/includes/ProxyTools/README
deleted file mode 100644 (file)
index 20b29bf..0000000
+++ /dev/null
@@ -1,2 +0,0 @@
-This directory hold tests for includes/ProxyTools.php file
-which is a pile of functions.
diff --git a/tests/phpunit/includes/ProxyTools/wfGetIPTest.php b/tests/phpunit/includes/ProxyTools/wfGetIPTest.php
deleted file mode 100644 (file)
index baa3d5d..0000000
+++ /dev/null
@@ -1,88 +0,0 @@
-<?php
-/*
- * Unit tests for wfGetIP()
- * 
- * FIXME: r94558 cause some test to throw an exception cause
- * wgCommandLine is not set :-)  Resetting the static variable
- * enlight a bug that is hidden by a previous call to wfGetIP().
- *
- * TODO: need to cover the part dealing with XFF headers.
- */
-class wfGetIPTest extends MediaWikiTestCase {
-
-       /** helper */
-       public function assertIP( $expected, $msg = '' ) {
-               $this->assertEquals(
-                       $expected,
-                       wfGetIP( 'reset' ),
-                       $msg );
-       }
-
-       /** helper */
-       public function assertIPNot( $expected, $msg = '' ) {
-               $this->assertNotEquals(
-                       $expected,
-                       wfGetIP( 'reset' ),
-                       $msg );
-       }
-
-       function testGetLoopbackAddressWhenInCommandLine() {
-               global $wgCommandLineMode;
-               $save = $wgCommandLineMode;
-
-               $wgCommandLineMode = true;
-               $this->assertIP( '127.0.0.1' );
-
-               # restore global
-               $wgCommandLineMode = $save;
-       }
-
-       function testGetFromServerRemoteAddr() {
-               global $_SERVER;
-               $save = null;
-               if( array_key_exists( 'REMOTE_ADDR', $_SERVER ) ) {
-                       $save = $_SERVER['REMOTE_ADDR'];
-               } else {
-                       $clearRemoteAddr = true;
-               }
-
-               # Starting assertions
-               $_SERVER['REMOTE_ADDR'] = '192.0.2.77'; # example IP - RFC 5737
-               $this->assertIP( '192.0.2.77' );
-
-               /*
-               #### wfGetIP() does not support IPv6 yet :((
-               $_SERVER['REMOTE_ADDR'] = '2001:db8:0:77';
-               $this->assertIP( '2001:db8:0:77' );
-               */
-
-               # restore global
-               if( $clearRemoteAddr ) {
-                       unset( $_SERVER['REMOTE_ADDR'] );
-               } else {
-                       $_SERVER['REMOTE_ADDR'] = $save;
-               }
-               # Restore internal static altered above
-               wfGetIP( 'reset' );
-       }
-
-       /**
-        * @expectedException MWException
-        * @group Broken
-        * This is broken since it alter $wgCommandLineMode which is needed
-        * by our exception handler.  Until someone find a correct way to handle
-        * this case, the test should stay in the Broken group.
-        * 
-        */
-       function testLackOfRemoteAddrThrowAnException() {
-               global $wgCommandLineMode;
-               $save = $wgCommandLineMode;
-
-               # PHPUnit runs from the command line
-               $wgCommandLineMode = false;
-               # Next call throw an exception about lacking an IP
-               wfGetIP( 'reset' );
-
-               $wgCommandLineMode = $save;
-       }
-}
index 1cfbd3f..ff8bf34 100644 (file)
@@ -85,4 +85,102 @@ class WebRequestTest extends MediaWikiTestCase {
                        ),
                );
        }
+
+       /**
+        * @dataProvider provideGetIP
+        */
+       function testGetIP( $expected, $input, $squid, $private, $description ) {
+               global $wgSquidServersNoPurge, $wgUsePrivateIPs;
+               $oldServer = $_SERVER;
+               $_SERVER = $input;
+               $wgSquidServersNoPurge = $squid;
+               $wgUsePrivateIPs = $private;
+               $request = new WebRequest();
+               $result = $request->getIP();
+               $_SERVER = $oldServer;
+               $this->assertEquals( $expected, $result, $description );
+       }
+
+       function provideGetIP() {
+               return array(
+                       array(
+                               '127.0.0.1',
+                               array(
+                                       'REMOTE_ADDR' => '127.0.0.1'
+                               ),
+                               array(),
+                               false,
+                               'Simple IPv4'
+                       ),
+                       array(
+                               '::1',
+                               array(
+                                       'REMOTE_ADDR' => '::1'
+                               ),
+                               array(),
+                               false,
+                               'Simple IPv6'
+                       ),
+                       array(
+                               '12.0.0.3',
+                               array(
+                                       'REMOTE_ADDR' => '12.0.0.1',
+                                       'HTTP_X_FORWARDED_FOR' => '12.0.0.3, 12.0.0.2'
+                               ),
+                               array( '12.0.0.1', '12.0.0.2' ),
+                               false,
+                               'With X-Forwaded-For'
+                       ),
+                       array(
+                               '12.0.0.1',
+                               array(
+                                       'REMOTE_ADDR' => '12.0.0.1',
+                                       'HTTP_X_FORWARDED_FOR' => '12.0.0.3, 12.0.0.2'
+                               ),
+                               array(),
+                               false,
+                               'With X-Forwaded-For and disallowed server'
+                       ),
+                       array(
+                               '12.0.0.2',
+                               array(
+                                       'REMOTE_ADDR' => '12.0.0.1',
+                                       'HTTP_X_FORWARDED_FOR' => '12.0.0.3, 12.0.0.2'
+                               ),
+                               array( '12.0.0.1' ),
+                               false,
+                               'With multiple X-Forwaded-For and only one allowed server'
+                       ),
+                       array(
+                               '12.0.0.2',
+                               array(
+                                       'REMOTE_ADDR' => '12.0.0.2',
+                                       'HTTP_X_FORWARDED_FOR' => '10.0.0.3, 12.0.0.2'
+                               ),
+                               array( '12.0.0.1', '12.0.0.2' ),
+                               false,
+                               'With X-Forwaded-For and private IP'
+                       ),
+                       array(
+                               '10.0.0.3',
+                               array(
+                                       'REMOTE_ADDR' => '12.0.0.2',
+                                       'HTTP_X_FORWARDED_FOR' => '10.0.0.3, 12.0.0.2'
+                               ),
+                               array( '12.0.0.1', '12.0.0.2' ),
+                               true,
+                               'With X-Forwaded-For and private IP (allowed)'
+                       ),
+               );
+       }
+
+       /**
+        * @expectedException MWException
+        */
+       function testGetIpLackOfRemoteAddrThrowAnException() {
+               var_dump( $_SERVER );
+               $request = new WebRequest();
+               # Next call throw an exception about lacking an IP
+               $request->getIP();
+       }
 }