From 9947848ca1a71a3288c392f9a85d65fc682052d5 Mon Sep 17 00:00:00 2001 From: Chad Horohoe Date: Tue, 15 Dec 2015 11:37:21 -0800 Subject: [PATCH] Fix IP::toHex for IPv4 addresses with a double/triple 0 block Bug: T97897 Change-Id: I5c0a37be42ae2c5091ead487a6d19f6e0dd89b36 --- includes/utils/IP.php | 22 +++++++++++++++------ tests/phpunit/includes/utils/IPTest.php | 35 +++++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/includes/utils/IP.php b/includes/utils/IP.php index 871f71b..9390dd8 100644 --- a/includes/utils/IP.php +++ b/includes/utils/IP.php @@ -127,8 +127,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 @@ -138,8 +139,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 ); @@ -489,8 +498,9 @@ class IP { if ( self::isIPv6( $ip ) ) { $n = self::toUnsigned6( $ip ); } else { - // 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 1267268..bcba179 100644 --- a/tests/phpunit/includes/utils/IPTest.php +++ b/tests/phpunit/includes/utils/IPTest.php @@ -270,15 +270,37 @@ class IPTest extends MediaWikiTestCase { } /** - * Improve IP::sanitizeIP() code coverage - * @todo Most probably incomplete - */ - public function testSanitizeIP() { - $this->assertNull( IP::sanitizeIP( '' ) ); - $this->assertNull( IP::sanitizeIP( ' ' ) ); + * @covers IP::sanitizeIP + * @dataProvider provideSanitizeIP + */ + 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, ' ') + ); + } + + /** * @covers IP::toUnsigned * @dataProvider provideToUnsigned */ @@ -331,6 +353,7 @@ class IPTest extends MediaWikiTestCase { 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' ), -- 2.6.4