From c9649314f710f722be2070f6f1bcae10189ba8f6 Mon Sep 17 00:00:00 2001
From: Florian Schmidt <florian.schmidt.stargatewissen@gmail.com>
Date: Sun, 16 Apr 2017 01:52:23 +0200
Subject: [PATCH] Use IPSet for checking of wgProxyList

Instead of using array_* functions, use the IPSet for checking, if a
specific IP address matches a set of addresses.

This also deprecates a backward-compatibility functionality, that
the wgProxyList array could also be an associative array, where the blocked
IP address is set as a key of the array insted of a value. All IP address
keys will be mved to values on-the-fly, however a deprecation warning will
be emitted. A notice in the Release notes was added, too.

Bug: T161580
Change-Id: I69d9534942c415ab044177969ecd54160079b593
---
 RELEASE-NOTES-1.30                       |  4 ++-
 includes/DefaultSettings.php             |  2 +-
 includes/user/User.php                   | 34 +++++++++++++-----
 tests/phpunit/includes/user/UserTest.php | 46 ++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30
index 3c1ae8d17c..244bd16d61 100644
--- a/RELEASE-NOTES-1.30
+++ b/RELEASE-NOTES-1.30
@@ -39,7 +39,9 @@ changes to languages because of Phabricator reports.
 * …
 
 === Other changes in 1.30 ===
-* …
+* The use of an associative array for $wgProxyList, where the IP address is in
+  the key instead of the value, is deprecated (e.g. [ '127.0.0.1' => 'value' ]).
+  Please convert these arrays to indexed/sequential ones (e.g. [ '127.0.0.1' ]).
 
 == Compatibility ==
 MediaWiki 1.30 requires PHP 5.5.9 or later. There is experimental support for
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 299757c8a6..ac2261c541 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -5938,7 +5938,7 @@ $wgSecretKey = false;
  *
  * This can have the following formats:
  * - An array of addresses, either in the values
- *   or the keys (for backward compatibility)
+ *   or the keys (for backward compatibility, deprecated since 1.30)
  * - A string, in that case this is the path to a file
  *   containing the list of IP addresses, one per line
  */
diff --git a/includes/user/User.php b/includes/user/User.php
index 3edd49f783..94da031d7d 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -20,6 +20,7 @@
  * @file
  */
 
+use IPSet\IPSet;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Session\SessionManager;
 use MediaWiki\Session\Token;
@@ -1854,18 +1855,33 @@ class User implements IDBAccessObject {
 			$wgProxyList = array_map( 'trim', file( $wgProxyList ) );
 		}
 
-		if ( is_array( $wgProxyList ) ) {
-			if (
-				// Look for IP as value
-				array_search( $ip, $wgProxyList ) !== false ||
-				// Look for IP as key (for backwards-compatility)
-				array_key_exists( $ip, $wgProxyList )
-			) {
-				return true;
+		$resultProxyList = [];
+		$deprecatedIPEntries = [];
+
+		// backward compatibility: move all ip addresses in keys to values
+		foreach ( $wgProxyList as $key => $value ) {
+			$keyIsIP = IP::isIPAddress( $key );
+			$valueIsIP = IP::isIPAddress( $value );
+			if ( $keyIsIP && !$valueIsIP ) {
+				$deprecatedIPEntries[] = $key;
+				$resultProxyList[] = $key;
+			} elseif ( $keyIsIP && $valueIsIP ) {
+				$deprecatedIPEntries[] = $key;
+				$resultProxyList[] = $key;
+				$resultProxyList[] = $value;
+			} else {
+				$resultProxyList[] = $value;
 			}
 		}
 
-		return false;
+		if ( $deprecatedIPEntries ) {
+			wfDeprecated(
+				'IP addresses in the keys of $wgProxyList (found the following IP addresses in keys: ' .
+				implode( ', ', $deprecatedIPEntries ) . ', please move them to values)', '1.30' );
+		}
+
+		$proxyListIPSet = new IPSet( $resultProxyList );
+		return $proxyListIPSet->match( $ip );
 	}
 
 	/**
diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index 776dda12e3..a596851c0e 100644
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -934,4 +934,50 @@ class UserTest extends MediaWikiTestCase {
 
 		$this->assertFalse( $user->getExperienceLevel() );
 	}
+
+	public static function provideIsLocallBlockedProxy() {
+		return [
+			[ '1.2.3.4', '1.2.3.4' ],
+			[ '1.2.3.4', '1.2.3.0/16' ],
+		];
+	}
+
+	/**
+	 * @dataProvider provideIsLocallBlockedProxy
+	 * @covers User::isLocallyBlockedProxy
+	 */
+	public function testIsLocallyBlockedProxy( $ip, $blockListEntry ) {
+		$this->setMwGlobals(
+			'wgProxyList', []
+		);
+		$this->assertFalse( User::isLocallyBlockedProxy( $ip ) );
+
+		$this->setMwGlobals(
+			'wgProxyList',
+			[
+				$blockListEntry
+			]
+		);
+		$this->assertTrue( User::isLocallyBlockedProxy( $ip ) );
+
+		$this->setMwGlobals(
+			'wgProxyList',
+			[
+				'test' => $blockListEntry
+			]
+		);
+		$this->assertTrue( User::isLocallyBlockedProxy( $ip ) );
+
+		$this->hideDeprecated(
+			'IP addresses in the keys of $wgProxyList (found the following IP ' .
+			'addresses in keys: ' . $blockListEntry . ', please move them to values)'
+		);
+		$this->setMwGlobals(
+			'wgProxyList',
+			[
+				$blockListEntry => 'test'
+			]
+		);
+		$this->assertTrue( User::isLocallyBlockedProxy( $ip ) );
+	}
 }
-- 
2.20.1