From: Marius Hoch <hoo@online.de>
Date: Sat, 2 May 2015 16:48:04 +0000 (+0200)
Subject: Fix IP::toHex for IPv4 addresses with a double/triple 0 block
X-Git-Tag: 1.31.0-rc.0~8634
X-Git-Url: http://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=47528dcf6a8a40ad0a1ea99fb49113aa9309b460

Fix IP::toHex for IPv4 addresses with a double/triple 0 block

Bug: T97897
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
Change-Id: I5c0a37be42ae2c5091ead487a6d19f6e0dd89b36
---

diff --git a/includes/utils/IP.php b/includes/utils/IP.php
index fd7030eb87..8abca5b9d3 100644
--- a/includes/utils/IP.php
+++ b/includes/utils/IP.php
@@ -132,8 +132,9 @@ class IP {
 
 	/**
 	 * Convert an IP into a verbose, uppercase, normalized form.
-	 * IPv6 addresses in octet notation are expanded to 8 words.
-	 * IPv4 addresses are just trimmed.
+	 * Both IPv4 and IPv6 addresses are trimmed. Additionally,
+	 * IPv6 addresses in octet notation are expanded to 8 words;
+	 * IPv4 addresses have leading zeros, in each octet, removed.
 	 *
 	 * @param string $ip IP address in quad or octet form (CIDR or not).
 	 * @return string
@@ -143,8 +144,16 @@ class IP {
 		if ( $ip === '' ) {
 			return null;
 		}
-		if ( self::isIPv4( $ip ) || !self::isIPv6( $ip ) ) {
-			return $ip; // nothing else to do for IPv4 addresses or invalid ones
+		/* If not an IP, just return trimmed value, since sanitizeIP() is called
+		 * in a number of contexts where usernames are supplied as input.
+		 */
+		if ( !self::isIPAddress( $ip ) ) {
+			return $ip;
+		}
+		if ( self::isIPv4( $ip ) ) {
+			// Remove leading 0's from octet representation of IPv4 address
+			$ip = preg_replace( '/(?:^|(?<=\.))0+(?=[1-9]|0\.|0$)/', '', $ip );
+			return $ip;
 		}
 		// Remove any whitespaces, convert to upper case
 		$ip = strtoupper( $ip );
@@ -399,8 +408,9 @@ class IP {
 		if ( self::isIPv6( $ip ) ) {
 			$n = 'v6-' . self::IPv6ToRawHex( $ip );
 		} elseif ( self::isIPv4( $ip ) ) {
-			// Bug 60035: an IP with leading 0's fails in ip2long sometimes (e.g. *.08)
-			$ip = preg_replace( '/(?<=\.)0+(?=[1-9])/', '', $ip );
+			// T62035/T97897: An IP with leading 0's fails in ip2long sometimes (e.g. *.08),
+			// also double/triple 0 needs to be changed to just a single 0 for ip2long.
+			$ip = self::sanitizeIP( $ip );
 			$n = ip2long( $ip );
 			if ( $n < 0 ) {
 				$n += pow( 2, 32 );
diff --git a/tests/phpunit/includes/utils/IPTest.php b/tests/phpunit/includes/utils/IPTest.php
index 04b8f48604..369e38bb1b 100644
--- a/tests/phpunit/includes/utils/IPTest.php
+++ b/tests/phpunit/includes/utils/IPTest.php
@@ -307,12 +307,34 @@ class IPTest extends PHPUnit_Framework_TestCase {
 	}
 
 	/**
-	 * Improve IP::sanitizeIP() code coverage
-	 * @todo Most probably incomplete
+	 * @covers IP::sanitizeIP
+	 * @dataProvider provideSanitizeIP
 	 */
-	public function testSanitizeIP() {
-		$this->assertNull( IP::sanitizeIP( '' ) );
-		$this->assertNull( IP::sanitizeIP( ' ' ) );
+	public function testSanitizeIP( $expected, $input ) {
+		$result = IP::sanitizeIP( $input );
+		$this->assertEquals( $expected, $result );
+	}
+
+	/**
+	 * Provider for IP::testSanitizeIP()
+	 */
+	public static function provideSanitizeIP() {
+		return array(
+			array( '0.0.0.0', '0.0.0.0' ),
+			array( '0.0.0.0', '00.00.00.00' ),
+			array( '0.0.0.0', '000.000.000.000' ),
+			array( '141.0.11.253', '141.000.011.253' ),
+			array( '1.2.4.5', '1.2.4.5' ),
+			array( '1.2.4.5', '01.02.04.05' ),
+			array( '1.2.4.5', '001.002.004.005' ),
+			array( '10.0.0.1', '010.0.000.1' ),
+			array( '80.72.250.4', '080.072.250.04' ),
+			array( 'Foo.1000.00', 'Foo.1000.00' ),
+			array( 'Bar.01', 'Bar.01' ),
+			array( 'Bar.010', 'Bar.010' ),
+			array( null, '' ),
+			array( null, ' ' )
+		);
 	}
 
 	/**
@@ -336,6 +358,7 @@ class IPTest extends PHPUnit_Framework_TestCase {
 			array( '80000000', '128.0.0.0' ),
 			array( 'DEADCAFE', '222.173.202.254' ),
 			array( 'FFFFFFFF', '255.255.255.255' ),
+			array( '8D000BFD', '141.000.11.253' ),
 			array( false, 'IN.VA.LI.D' ),
 			array( 'v6-00000000000000000000000000000001', '::1' ),
 			array( 'v6-20010DB885A3000000008A2E03707334', '2001:0db8:85a3:0000:0000:8a2e:0370:7334' ),