Improve behavior of IP::toUnsigned on Windows

On Windows (and 32-bit systems), pow( 2, 32 ) - 1 is a float, which makes
IP::toUnsigned return a float sometimes (instead of string, int, or false,
as is specified in the documentation).

This makes problems for some callers (e.g. I0c9a4ae7 had to modify
wfBaseConvert because of this, while I believe this change would have made
that change unnecessary).

So to improve that, and make the result correspond to the documentation,
we ensure floats are converted to strings.

Plus, more phpunit coverage of IP::toUnsigned and the related IP::toHex.

Change-Id: Ic8e4d9c65497e78960b03555eab0558a6af7d8d2

2 files changed, 60 insertions, 6 deletions

diff --git a/includes/IP.php b/includes/IP.php
index 1e0a4f934545..0943606e0432 100644
--- a/includes/IP.php
+++ b/includes/IP.php
@@ -492,6 +492,11 @@ class IP {
 			$n = ip2long( $ip );
 			if ( $n < 0 ) {
 				$n += pow( 2, 32 );
+				# On 32-bit platforms (and on Windows), 2^32 does not fit into an int,
+				# so $n becomes a float. We convert it to string instead.
+				if ( is_float ( $n ) ) {
+					$n = (string) $n;
+				}
 			}
 		}
 		return $n;
diff --git a/tests/phpunit/includes/IPTest.php b/tests/phpunit/includes/IPTest.php
index 7bc2938521f5..c19317972ecd 100644
--- a/tests/phpunit/includes/IPTest.php
+++ b/tests/phpunit/includes/IPTest.php
@@ -259,14 +259,63 @@ class IPTest extends MediaWikiTestCase {
 	}
 
 	/**
-	 * test wrapper around ip2long which might return -1 or false depending on PHP version
 	 * @covers IP::toUnsigned
+	 * @dataProvider provideToUnsigned
 	 */
-	public function testip2longWrapper() {
-		// @todo FIXME: Add more tests ?
-		$this->assertEquals( pow( 2, 32 ) - 1, IP::toUnsigned( '255.255.255.255' ) );
-		$i = 'IN.VA.LI.D';
-		$this->assertFalse( IP::toUnSigned( $i ) );
+	public function testToUnsigned( $expected, $input ) {
+		$result = IP::toUnsigned( $input );
+		$this->assertTrue( $result === false || is_string( $result ) || is_int( $result ) );
+		$this->assertEquals( $expected, $result );
+	}
+
+	/**
+	 * Provider for IP::testToUnsigned()
+	 */
+	public static function provideToUnsigned() {
+		return array(
+			array ( 1, '0.0.0.1' ),
+			array ( 16909060, '1.2.3.4' ),
+			array ( 2130706433, '127.0.0.1' ),
+			array ( '2147483648', '128.0.0.0' ),
+			array ( '3735931646', '222.173.202.254' ),
+			array ( pow( 2, 32 ) - 1, '255.255.255.255' ),
+			array ( false, 'IN.VA.LI.D' ),
+			array ( 1, '::1' ),
+			array ( '42540766452641154071740215577757643572', '2001:0db8:85a3:0000:0000:8a2e:0370:7334' ),
+			array ( '42540766452641154071740215577757643572', '2001:db8:85a3::8a2e:0370:7334' ),
+			array ( false, 'IN:VA::LI:D' ),
+			array ( false, ':::1' )
+		);
+	}
+
+	/**
+	 * @covers IP::toHex
+	 * @dataProvider provideToHex
+	 */
+	public function testToHex( $expected, $input ) {
+		$result = IP::toHex( $input );
+		$this->assertTrue( $result === false || is_string( $result ) );
+		$this->assertEquals( $expected, $result );
+	}
+
+	/**
+	 * Provider for IP::testToHex()
+	 */
+	public static function provideToHex() {
+		return array(
+			array ( '00000001', '0.0.0.1' ),
+			array ( '01020304', '1.2.3.4' ),
+			array ( '7F000001', '127.0.0.1' ),
+			array ( '80000000', '128.0.0.0' ),
+			array ( 'DEADCAFE', '222.173.202.254' ),
+			array ( 'FFFFFFFF', '255.255.255.255' ),
+			array ( false, 'IN.VA.LI.D' ),
+			array ( 'v6-00000000000000000000000000000001', '::1' ),
+			array ( 'v6-20010DB885A3000000008A2E03707334', '2001:0db8:85a3:0000:0000:8a2e:0370:7334' ),
+			array ( 'v6-20010DB885A3000000008A2E03707334', '2001:db8:85a3::8a2e:0370:7334' ),
+			array ( false, 'IN:VA::LI:D' ),
+			array ( false, ':::1' )
+		);
 	}
 
 	/**