r82093 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r82092‎ | r82093 | r82094 >
Date:23:24, 13 February 2011
Author:aaron
Status:ok
Tags:
Comment:
* (bug 27353) IPv6 address ending in "::WORD" was not recognized
* Moved down 'contains no "::"' alternative for clarity (and possibly use frequency too)
* Added more IPv6 tests
Modified paths:
  • /trunk/phase3/includes/IP.php (modified) (history)
  • /trunk/phase3/tests/phpunit/includes/IPTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/IPTest.php
——@@ -39,17 +39,20 @@
4040 $this->assertFalse( IP::isIPv6( 'fc:100:::' ), 'IPv6 ending with a ":::"' );
4141 $this->assertFalse( IP::isIPv6( 'fc:300' ), 'IPv6 with only 2 words' );
4242 $this->assertFalse( IP::isIPv6( 'fc:100:300' ), 'IPv6 with only 3 words' );
 43+
4344 $this->assertTrue( IP::isIPv6( 'fc:100::' ) );
4445 $this->assertTrue( IP::isIPv6( 'fc:100:a::' ) );
4546 $this->assertTrue( IP::isIPv6( 'fc:100:a:d::' ) );
4647 $this->assertTrue( IP::isIPv6( 'fc:100:a:d:1::' ) );
4748 $this->assertTrue( IP::isIPv6( 'fc:100:a:d:1:e::' ) );
4849 $this->assertTrue( IP::isIPv6( 'fc:100:a:d:1:e:ac::' ) );
 50+
4951 $this->assertFalse( IP::isIPv6( 'fc:100:a:d:1:e:ac:0::' ), 'IPv6 with 8 words ending with "::"' );
5052 $this->assertFalse( IP::isIPv6( 'fc:100:a:d:1:e:ac:0:1::' ), 'IPv6 with 9 words ending with "::"' );
5153
5254 $this->assertFalse( IP::isIPv6( ':::' ) );
5355 $this->assertFalse( IP::isIPv6( '::0:' ), 'IPv6 ending in a lone ":"' );
 56+
5457 $this->assertTrue( IP::isIPv6( '::' ), 'IPv6 zero address' );
5558 $this->assertTrue( IP::isIPv6( '::0' ) );
5659 $this->assertTrue( IP::isIPv6( '::fc' ) );
——@@ -59,18 +62,24 @@
6063 $this->assertTrue( IP::isIPv6( '::fc:100:a:d:1' ) );
6164 $this->assertTrue( IP::isIPv6( '::fc:100:a:d:1:e' ) );
6265 $this->assertTrue( IP::isIPv6( '::fc:100:a:d:1:e:ac' ) );
 66+
6367 $this->assertFalse( IP::isIPv6( '::fc:100:a:d:1:e:ac:0' ), 'IPv6 with "::" and 8 words' );
6468 $this->assertFalse( IP::isIPv6( '::fc:100:a:d:1:e:ac:0:1' ), 'IPv6 with 9 words' );
6569
6670 $this->assertFalse( IP::isIPv6( ':fc::100' ), 'IPv6 starting with lone ":"' );
6771 $this->assertFalse( IP::isIPv6( 'fc::100:' ), 'IPv6 ending with lone ":"' );
6872 $this->assertFalse( IP::isIPv6( 'fc:::100' ), 'IPv6 with ":::" in the middle' );
 73+
6974 $this->assertTrue( IP::isIPv6( 'fc::100' ), 'IPv6 with "::" and 2 words' );
7075 $this->assertTrue( IP::isIPv6( 'fc::100:a' ), 'IPv6 with "::" and 3 words' );
7176 $this->assertTrue( IP::isIPv6( 'fc::100:a:d', 'IPv6 with "::" and 4 words' ) );
7277 $this->assertTrue( IP::isIPv6( 'fc::100:a:d:1' ), 'IPv6 with "::" and 5 words' );
7378 $this->assertTrue( IP::isIPv6( 'fc::100:a:d:1:e' ), 'IPv6 with "::" and 6 words' );
7479 $this->assertTrue( IP::isIPv6( 'fc::100:a:d:1:e:ac' ), 'IPv6 with "::" and 7 words' );
 80+ $this->assertTrue( IP::isIPv6( '2001::df'), 'IPv6 with "::" and 2 words' );
 81+ $this->assertTrue( IP::isIPv6( '2001:5c0:1400:a::df'), 'IPv6 with "::" and 5 words' );
 82+ $this->assertTrue( IP::isIPv6( '2001:5c0:1400:a::df:2'), 'IPv6 with "::" and 6 words' );
 83+
7584 $this->assertFalse( IP::isIPv6( 'fc::100:a:d:1:e:ac:0' ), 'IPv6 with "::" and 8 words' );
7685 $this->assertFalse( IP::isIPv6( 'fc::100:a:d:1:e:ac:0:1' ), 'IPv6 with 9 words' );
7786
——@@ -111,6 +120,26 @@
112121 $this->assertTrue( IP::isValid( $ip ) , "$ip is a valid IPv6 address" );
113122 }
114123 }
 124+ // test with some abbreviations
 125+ $this->assertFalse( IP::isValid( ':fc:100::' ), 'IPv6 starting with lone ":"' );
 126+ $this->assertFalse( IP::isValid( 'fc:100:::' ), 'IPv6 ending with a ":::"' );
 127+ $this->assertFalse( IP::isValid( 'fc:300' ), 'IPv6 with only 2 words' );
 128+ $this->assertFalse( IP::isValid( 'fc:100:300' ), 'IPv6 with only 3 words' );
 129+
 130+ $this->assertTrue( IP::isValid( 'fc:100::' ) );
 131+ $this->assertTrue( IP::isValid( 'fc:100:a:d:1:e::' ) );
 132+ $this->assertTrue( IP::isValid( 'fc:100:a:d:1:e:ac::' ) );
 133+
 134+ $this->assertTrue( IP::isValid( 'fc::100' ), 'IPv6 with "::" and 2 words' );
 135+ $this->assertTrue( IP::isValid( 'fc::100:a' ), 'IPv6 with "::" and 3 words' );
 136+ $this->assertTrue( IP::isValid( '2001::df'), 'IPv6 with "::" and 2 words' );
 137+ $this->assertTrue( IP::isValid( '2001:5c0:1400:a::df'), 'IPv6 with "::" and 5 words' );
 138+ $this->assertTrue( IP::isValid( '2001:5c0:1400:a::df:2'), 'IPv6 with "::" and 6 words' );
 139+ $this->assertTrue( IP::isValid( 'fc::100:a:d:1' ), 'IPv6 with "::" and 5 words' );
 140+ $this->assertTrue( IP::isValid( 'fc::100:a:d:1:e:ac' ), 'IPv6 with "::" and 7 words' );
 141+
 142+ $this->assertFalse( IP::isValid( 'fc:100:a:d:1:e:ac:0::' ), 'IPv6 with 8 words ending with "::"' );
 143+ $this->assertFalse( IP::isValid( 'fc:100:a:d:1:e:ac:0:1::' ), 'IPv6 with 9 words ending with "::"' );
115144 }
116145
117146 public function testInvalidIPs() {
Index: trunk/phase3/includes/IP.php
——@@ -39,17 +39,18 @@
4040 ':(?::|(?::' . RE_IPV6_WORD . '){1,7})' .
4141 '|' . // ends with "::" (except "::")
4242 RE_IPV6_WORD . '(?::' . RE_IPV6_WORD . '){0,6}::' .
43 - '|' . // contains no "::"
44 - RE_IPV6_WORD . '(?::' . RE_IPV6_WORD . '){7}' .
45 - '|' . // contains one "::" in the middle and 2 words
46 - RE_IPV6_WORD . '::' . RE_IPV6_WORD .
47 - '|' . // contains one "::" in the middle and 3+ words (awkward regex for PCRE 4.0+)
 43+ '|' . // contains one "::" in the middle, ending in "::WORD"
 44+ RE_IPV6_WORD . '(?::' . RE_IPV6_WORD . '){0,5}' . '::' . RE_IPV6_WORD .
 45+ '|' . // contains one "::" in the middle, not ending in "::WORD" (regex for PCRE 4.0+)
4846 RE_IPV6_WORD . '(?::(?P<abn>:(?P<iabn>))?' . RE_IPV6_WORD . '(?!:(?P=abn))){1,5}' .
4947 ':' . RE_IPV6_WORD . '(?P=iabn)' .
5048 // NOTE: (?!(?P=abn)) fails iff "::" used twice; (?P=iabn) passes iff a "::" was found.
51 - // RegExp (PCRE 7.2+ only) for last 2 cases that allows easy regex concatenation:
52 - #RE_IPV6_WORD . '(?::((?(-1)|:))?' . RE_IPV6_WORD . '){1,6}(?(-2)|^)' .
 49+ '|' . // contains no "::"
 50+ RE_IPV6_WORD . '(?::' . RE_IPV6_WORD . '){7}' .
5351 ')'
 52+ // NOTE: With PCRE 7.2+, we can combine the last two cases into one regex:
 53+ // RE_IPV6_WORD . '(?::((?(-1)|:))?' . RE_IPV6_WORD . '){1,6}(?(-2)|^)'
 54+ // This also improves regex concatenation by using relative references.
5455 );
5556 // An IPv6 block is an IP address and a prefix (d1 to d128)
5657 define( 'RE_IPV6_BLOCK', RE_IPV6_ADD . '\/' . RE_IPV6_PREFIX );

Follow-up revisions

RevisionCommit summaryAuthorDate
r82095Follow-up r82093: fixed comment :)aaron23:30, 13 February 2011
r85436MFT r82093, r82095 (dunno why they didn't merge cleanly before...)demon14:08, 5 April 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r77067*(bug 25920) Moved forward ref to a back ref to really get v6 regex to compil...aaron10:49, 21 November 2010

Status & tagging log