r76591 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r76590‎ | r76591 | r76592 >
Date:19:37, 12 November 2010
Author:aaron
Status:ok (Comments)
Tags:
Comment:
* Removed redundant check in toUnsigned6().
* MW requires PHP 5.1+, so the -1/false ip2long annoyance is gone. Also, ip2long("255.255.255.255") is -1 so no special case code is needed anymore.
* Removed toSigned() (not used outside IP.php). Due to the above points, ip2long() is totally equilivant.
* Moved some functions and consts around.
* Comment tweaks.
Modified paths:
  • /trunk/phase3/includes/IP.php (modified) (history)
  • /trunk/phase3/maintenance/tests/phpunit/includes/IPTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/tests/phpunit/includes/IPTest.php
@@ -135,10 +135,8 @@
136136 public function testip2longWrapper() {
137137 // fixme : add more tests ?
138138 $this->assertEquals( pow(2,32) - 1, IP::toUnsigned( '255.255.255.255' ));
139 - $this->assertEquals( -1 , IP::toSigned( '255.255.255.255' )) ;
140139 $i = 'IN.VA.LI.D';
141140 $this->assertFalse( IP::toUnSigned( $i ) );
142 - $this->assertFalse( IP::toSigned( $i ) );
143141 }
144142
145143 public function testPrivateIPs() {
@@ -199,8 +197,6 @@
200198 function testCIDRParsing() {
201199 $this->assertFalseCIDR( '192.0.2.0' , "missing mask" );
202200 $this->assertFalseCIDR( '192.0.2.0/', "missing bitmask" );
203 -
204 - // code calls IP::toSigned()
205201
206202 // Verify if statement
207203 $this->assertFalseCIDR( '256.0.0.0/32', "invalid net" );
Index: trunk/phase3/includes/IP.php
@@ -29,13 +29,11 @@
3030 // An IPv4 block is an IP address and a prefix (d1 to d32)
3131 define( 'RE_IP_PREFIX', '(3[0-2]|[12]?\d)' );
3232 define( 'RE_IP_BLOCK', RE_IP_ADD . '\/' . RE_IP_PREFIX );
33 -// For IPv6 canonicalization (NOT for strict validation; these are quite lax!)
34 -define( 'RE_IPV6_WORD', '([0-9A-Fa-f]{1,4})' );
35 -define( 'RE_IPV6_GAP', ':(?:0+:)*(?::(?:0+:)*)?' );
36 -define( 'RE_IPV6_V4_PREFIX', '0*' . RE_IPV6_GAP . '(?:ffff:)?' );
 33+
3734 // An IPv6 block is an IP address and a prefix (d1 to d128)
 35+define( 'RE_IPV6_WORD', '([0-9A-Fa-f]{1,4})' );
3836 define( 'RE_IPV6_PREFIX', '(12[0-8]|1[01][0-9]|[1-9]?\d)');
39 -// An IPv6 address is made up of 8 octets. However, the "::" abbreviations can be used.
 37+// An IPv6 address is made up of 8 words. However, the "::" abbreviations can be used.
4038 define( 'RE_IPV6_ADD',
4139 '(' . // starts with "::" (includes the address "::")
4240 '(::|:(:' . RE_IPV6_WORD . '){1,7})' .
@@ -48,6 +46,10 @@
4947 ')'
5048 );
5149 define( 'RE_IPV6_BLOCK', RE_IPV6_ADD . '\/' . RE_IPV6_PREFIX );
 50+// For IPv6 canonicalization (NOT for strict validation; these are quite lax!)
 51+define( 'RE_IPV6_GAP', ':(?:0+:)*(?::(?:0+:)*)?' );
 52+define( 'RE_IPV6_V4_PREFIX', '0*' . RE_IPV6_GAP . '(?:ffff:)?' );
 53+
5254 // This might be useful for regexps used elsewhere, matches any IPv6 or IPv6 address or network
5355 define( 'IP_ADDRESS_STRING',
5456 '(?:' .
@@ -63,8 +65,9 @@
6466 */
6567 class IP {
6668 /**
67 - * Given a string, determine if it as valid IP.
68 - * Note: Unlike isValid(), this looks for networks too.
 69+ * Determine if a string is as valid IP address or network (CIDR prefix).
 70+ * SIIT IPv4-translated addresses are rejected.
 71+ * Note: canonicalize() tries to convert translated addresses to IPv4.
6972 * @param string $ip possible IP address
7073 * @return bool
7174 */
@@ -93,6 +96,30 @@
9497 }
9598
9699 /**
 100+ * Validate an IP address. Ranges are NOT considered valid.
 101+ * SIIT IPv4-translated addresses are rejected.
 102+ * Note: canonicalize() tries to convert translated addresses to IPv4.
 103+ * @param string $ip
 104+ * @return boolean True if it is valid.
 105+ */
 106+ public static function isValid( $ip ) {
 107+ return ( preg_match( '/^' . RE_IP_ADD . '$/', $ip )
 108+ || preg_match( '/^' . RE_IPV6_ADD . '$/', $ip ) );
 109+ }
 110+
 111+ /**
 112+ * Validate an IP Block (valid address WITH a valid prefix).
 113+ * SIIT IPv4-translated addresses are rejected.
 114+ * Note: canonicalize() tries to convert translated addresses to IPv4.
 115+ * @param string $ipblock
 116+ * @return boolean True if it is valid.
 117+ */
 118+ public static function isValidBlock( $ipblock ) {
 119+ return ( preg_match( '/^' . RE_IPV6_BLOCK . '$/', $ipblock )
 120+ || preg_match( '/^' . RE_IPV4_BLOCK . '$/', $ipblock ) );
 121+ }
 122+
 123+ /**
97124 * Given an IP address in dotted-quad notation, returns an IPv6 octet.
98125 * See http://www.answers.com/topic/ipv4-compatible-address
99126 * IPs with the first 92 bits as zeros are reserved from IPv6
@@ -125,16 +152,9 @@
126153 return self::toOctet( self::toUnsigned( $ip ) );
127154 }
128155
129 - private static function toUnsigned6( $ip ) {
130 - if ( self::isIPv6( $ip ) ) {
131 - return wfBaseConvert( self::IPv6ToRawHex( $ip ), 16, 10 );
132 - }
133 - return false;
134 - }
135 -
136156 /**
137157 * Convert an IP into a nice standard form.
138 - * IPv6 addresses in octet notation are expanded to 8 octets.
 158+ * IPv6 addresses in octet notation are expanded to 8 words.
139159 * IPv4 addresses are just trimmed.
140160 * @param string $ip IP address in quad or octet form (CIDR or not).
141161 * @return string
@@ -190,8 +210,7 @@
191211 * @return string
192212 */
193213 public static function toOctet( $ip_int ) {
194 - $ip_hex = wfBaseConvert( $ip_int, 10, 16, 32, false ); // uppercase hex
195 - return self::hexToOctet( $ip_hex );
 214+ return self::hexToOctet( wfBaseConvert( $ip_int, 10, 16, 32, false ) );
196215 }
197216
198217 /**
@@ -213,9 +232,9 @@
214233 * @return string (of format a:b:c:d:e:f:g:h)
215234 */
216235 public static function hexToOctet( $ip_hex ) {
217 - // Convert to padded uppercase hex
 236+ // Pad hex to 32 chars (128 bits)
218237 $ip_hex = str_pad( strtoupper( $ip_hex ), 32, '0', STR_PAD_LEFT );
219 - // Separate into 8 octets
 238+ // Separate into 8 words
220239 $ip_oct = substr( $ip_hex, 0, 4 );
221240 for ( $n = 1; $n < 8; $n++ ) {
222241 $ip_oct .= ':' . substr( $ip_hex, 4 * $n, 4 );
@@ -231,7 +250,7 @@
232251 * @return string (of format a.b.c.d)
233252 */
234253 public static function hexToQuad( $ip_hex ) {
235 - // Convert to padded uppercase hex
 254+ // Pad hex to 8 chars (32 bits)
236255 $ip_hex = str_pad( strtoupper( $ip_hex ), 8, '0', STR_PAD_LEFT );
237256 // Separate into four quads
238257 $s = '';
@@ -245,113 +264,6 @@
246265 }
247266
248267 /**
249 - * Convert a network specification in IPv6 CIDR notation to an
250 - * integer network and a number of bits
251 - * @return array(string, int)
252 - */
253 - private static function parseCIDR6( $range ) {
254 - # Explode into <expanded IP,range>
255 - $parts = explode( '/', IP::sanitizeIP( $range ), 2 );
256 - if ( count( $parts ) != 2 ) {
257 - return array( false, false );
258 - }
259 - list( $network, $bits ) = $parts;
260 - $network = self::IPv6ToRawHex( $network );
261 - if ( $network !== false && is_numeric( $bits ) && $bits >= 0 && $bits <= 128 ) {
262 - if ( $bits == 0 ) {
263 - $network = "0";
264 - } else {
265 - # Native 32 bit functions WONT work here!!!
266 - # Convert to a padded binary number
267 - $network = wfBaseConvert( $network, 16, 2, 128 );
268 - # Truncate the last (128-$bits) bits and replace them with zeros
269 - $network = str_pad( substr( $network, 0, $bits ), 128, 0, STR_PAD_RIGHT );
270 - # Convert back to an integer
271 - $network = wfBaseConvert( $network, 2, 10 );
272 - }
273 - } else {
274 - $network = false;
275 - $bits = false;
276 - }
277 - return array( $network, (int)$bits );
278 - }
279 -
280 - /**
281 - * Given a string range in a number of formats, return the
282 - * start and end of the range in hexadecimal. For IPv6.
283 - *
284 - * Formats are:
285 - * 2001:0db8:85a3::7344/96 CIDR
286 - * 2001:0db8:85a3::7344 - 2001:0db8:85a3::7344 Explicit range
287 - * 2001:0db8:85a3::7344/96 Single IP
288 - * @return array(string, int)
289 - */
290 - private static function parseRange6( $range ) {
291 - # Expand any IPv6 IP
292 - $range = IP::sanitizeIP( $range );
293 - // CIDR notation...
294 - if ( strpos( $range, '/' ) !== false ) {
295 - list( $network, $bits ) = self::parseCIDR6( $range );
296 - if ( $network === false ) {
297 - $start = $end = false;
298 - } else {
299 - $start = wfBaseConvert( $network, 10, 16, 32, false );
300 - # Turn network to binary (again)
301 - $end = wfBaseConvert( $network, 10, 2, 128 );
302 - # Truncate the last (128-$bits) bits and replace them with ones
303 - $end = str_pad( substr( $end, 0, $bits ), 128, 1, STR_PAD_RIGHT );
304 - # Convert to hex
305 - $end = wfBaseConvert( $end, 2, 16, 32, false );
306 - # see toHex() comment
307 - $start = "v6-$start";
308 - $end = "v6-$end";
309 - }
310 - // Explicit range notation...
311 - } elseif ( strpos( $range, '-' ) !== false ) {
312 - list( $start, $end ) = array_map( 'trim', explode( '-', $range, 2 ) );
313 - $start = self::toUnsigned6( $start );
314 - $end = self::toUnsigned6( $end );
315 - if ( $start > $end ) {
316 - $start = $end = false;
317 - } else {
318 - $start = wfBaseConvert( $start, 10, 16, 32, false );
319 - $end = wfBaseConvert( $end, 10, 16, 32, false );
320 - }
321 - # see toHex() comment
322 - $start = "v6-$start";
323 - $end = "v6-$end";
324 - } else {
325 - # Single IP
326 - $start = $end = self::toHex( $range );
327 - }
328 - if ( $start === false || $end === false ) {
329 - return array( false, false );
330 - } else {
331 - return array( $start, $end );
332 - }
333 - }
334 -
335 - /**
336 - * Validate an IP address. Ranges are NOT considered valid.
337 - * @param string $ip
338 - * @return boolean True if it is valid.
339 - */
340 - public static function isValid( $ip ) {
341 - return ( preg_match( '/^' . RE_IP_ADD . '$/', $ip )
342 - || preg_match( '/^' . RE_IPV6_ADD . '$/', $ip ) );
343 - }
344 -
345 - /**
346 - * Validate an IP Block (valid address WITH a valid prefix).
347 - * @param string $ipblock
348 - * @return boolean True if it is valid.
349 - */
350 - public static function isValidBlock( $ipblock ) {
351 - return ( preg_match( '/^' . RE_IPV6_BLOCK . '$/', $ipblock )
352 - || preg_match( '/^' . RE_IPV4_BLOCK . '$/', $ipblock ) );
353 - }
354 -
355 - /**
356268 * Determine if an IP address really is an IP address, and if it is public,
357269 * i.e. not RFC 1918 or similar
358270 * Comes from ProxyTools.php
@@ -405,7 +317,7 @@
406318 if ( !$privateRanges ) {
407319 $privateRanges = array(
408320 array( 'fc::', 'fdff:ffff:ffff:ffff:ffff:ffff:ffff:ffff' ), # RFC 4193 (local)
409 - array( '0:0:0:0:0:0:0:1', '0:0:0:0:0:0:0:1' ), # loopback
 321+ array( '0:0:0:0:0:0:0:1', '0:0:0:0:0:0:0:1' ), # loopback
410322 );
411323 }
412324 $n = self::toHex( $ip );
@@ -443,35 +355,9 @@
444356 }
445357
446358 /**
447 - * Given an IP address in dotted-quad/octet notation, returns an unsigned integer.
448 - * Like ip2long() except that it actually works and has a consistent error return value.
449 - * Comes from ProxyTools.php
450 - * @param string $ip Quad dotted IP address.
451 - * @return mixed (string/int/false)
452 - */
453 - public static function toUnsigned( $ip ) {
454 - if ( self::isIPv6( $ip ) ) {
455 - $n = wfBaseConvert( self::IPv6ToRawHex( $ip ), 16, 10 );
456 - } else {
457 - if ( $ip == '255.255.255.255' ) {
458 - $n = -1;
459 - } else {
460 - $n = ip2long( $ip );
461 - if ( $n == -1 || $n === false ) { # Return value on error depends on PHP version
462 - $n = false;
463 - }
464 - }
465 - if ( $n < 0 ) {
466 - $n += pow( 2, 32 );
467 - }
468 - }
469 - return $n;
470 - }
471 -
472 - /**
473359 * Given an IPv6 address in octet notation, returns a pure hex string.
474360 * @param string $ip octet ipv6 IP address.
475 - * @return string hex (uppercase)
 361+ * @return string pure hex (uppercase)
476362 */
477363 private static function IPv6ToRawHex( $ip ) {
478364 $ip = self::sanitizeIP( $ip );
@@ -486,24 +372,31 @@
487373 }
488374
489375 /**
490 - * Convert a dotted-quad IP to a signed integer
491 - * @param string $ip
492 - * @return mixed (string/false)
 376+ * Given an IP address in dotted-quad/octet notation, returns an unsigned integer.
 377+ * Like ip2long() except that it actually works and has a consistent error return value.
 378+ * Comes from ProxyTools.php
 379+ * @param string $ip Quad dotted IP address.
 380+ * @return mixed (string/int/false)
493381 */
494 - public static function toSigned( $ip ) {
495 - if ( $ip == '255.255.255.255' ) {
496 - $n = -1;
 382+ public static function toUnsigned( $ip ) {
 383+ if ( self::isIPv6( $ip ) ) {
 384+ $n = wfBaseConvert( self::IPv6ToRawHex( $ip ), 16, 10 );
497385 } else {
498386 $n = ip2long( $ip );
499 - if ( $n == -1 ) {
500 - $n = false;
 387+ if ( $n < 0 ) {
 388+ $n += pow( 2, 32 );
501389 }
502390 }
503391 return $n;
504392 }
505393
 394+ private static function toUnsigned6( $ip ) {
 395+ return wfBaseConvert( self::IPv6ToRawHex( $ip ), 16, 10 );
 396+ }
 397+
506398 /**
507 - * Convert a network specification in CIDR notation to an integer network and a number of bits
 399+ * Convert a network specification in CIDR notation
 400+ * to an integer network and a number of bits
508401 * @param string $range (CIDR IP)
509402 * @return array(int, int)
510403 */
@@ -513,7 +406,7 @@
514407 return array( false, false );
515408 }
516409 list( $network, $bits ) = $parts;
517 - $network = self::toSigned( $network );
 410+ $network = ip2long( $network );
518411 if ( $network !== false && is_numeric( $bits ) && $bits >= 0 && $bits <= 32 ) {
519412 if ( $bits == 0 ) {
520413 $network = 0;
@@ -587,6 +480,93 @@
588481 }
589482
590483 /**
 484+ * Convert a network specification in IPv6 CIDR notation to an
 485+ * integer network and a number of bits
 486+ * @return array(string, int)
 487+ */
 488+ private static function parseCIDR6( $range ) {
 489+ # Explode into <expanded IP,range>
 490+ $parts = explode( '/', IP::sanitizeIP( $range ), 2 );
 491+ if ( count( $parts ) != 2 ) {
 492+ return array( false, false );
 493+ }
 494+ list( $network, $bits ) = $parts;
 495+ $network = self::IPv6ToRawHex( $network );
 496+ if ( $network !== false && is_numeric( $bits ) && $bits >= 0 && $bits <= 128 ) {
 497+ if ( $bits == 0 ) {
 498+ $network = "0";
 499+ } else {
 500+ # Native 32 bit functions WONT work here!!!
 501+ # Convert to a padded binary number
 502+ $network = wfBaseConvert( $network, 16, 2, 128 );
 503+ # Truncate the last (128-$bits) bits and replace them with zeros
 504+ $network = str_pad( substr( $network, 0, $bits ), 128, 0, STR_PAD_RIGHT );
 505+ # Convert back to an integer
 506+ $network = wfBaseConvert( $network, 2, 10 );
 507+ }
 508+ } else {
 509+ $network = false;
 510+ $bits = false;
 511+ }
 512+ return array( $network, (int)$bits );
 513+ }
 514+
 515+ /**
 516+ * Given a string range in a number of formats, return the
 517+ * start and end of the range in hexadecimal. For IPv6.
 518+ *
 519+ * Formats are:
 520+ * 2001:0db8:85a3::7344/96 CIDR
 521+ * 2001:0db8:85a3::7344 - 2001:0db8:85a3::7344 Explicit range
 522+ * 2001:0db8:85a3::7344/96 Single IP
 523+ * @return array(string, int)
 524+ */
 525+ private static function parseRange6( $range ) {
 526+ # Expand any IPv6 IP
 527+ $range = IP::sanitizeIP( $range );
 528+ // CIDR notation...
 529+ if ( strpos( $range, '/' ) !== false ) {
 530+ list( $network, $bits ) = self::parseCIDR6( $range );
 531+ if ( $network === false ) {
 532+ $start = $end = false;
 533+ } else {
 534+ $start = wfBaseConvert( $network, 10, 16, 32, false );
 535+ # Turn network to binary (again)
 536+ $end = wfBaseConvert( $network, 10, 2, 128 );
 537+ # Truncate the last (128-$bits) bits and replace them with ones
 538+ $end = str_pad( substr( $end, 0, $bits ), 128, 1, STR_PAD_RIGHT );
 539+ # Convert to hex
 540+ $end = wfBaseConvert( $end, 2, 16, 32, false );
 541+ # see toHex() comment
 542+ $start = "v6-$start";
 543+ $end = "v6-$end";
 544+ }
 545+ // Explicit range notation...
 546+ } elseif ( strpos( $range, '-' ) !== false ) {
 547+ list( $start, $end ) = array_map( 'trim', explode( '-', $range, 2 ) );
 548+ $start = self::toUnsigned6( $start );
 549+ $end = self::toUnsigned6( $end );
 550+ if ( $start > $end ) {
 551+ $start = $end = false;
 552+ } else {
 553+ $start = wfBaseConvert( $start, 10, 16, 32, false );
 554+ $end = wfBaseConvert( $end, 10, 16, 32, false );
 555+ }
 556+ # see toHex() comment
 557+ $start = "v6-$start";
 558+ $end = "v6-$end";
 559+ } else {
 560+ # Single IP
 561+ $start = $end = self::toHex( $range );
 562+ }
 563+ if ( $start === false || $end === false ) {
 564+ return array( false, false );
 565+ } else {
 566+ return array( $start, $end );
 567+ }
 568+ }
 569+
 570+ /**
591571 * Determine if a given IPv4/IPv6 address is in a given CIDR network
592572 * @param $addr The address to check against the given range.
593573 * @param $range The range to check the given address against.

Comments

#Comment by Platonides (talk | contribs)   18:09, 15 November 2010

Use of undefined constant RE_IPV4_BLOCK in includes/IP.php line 119

#Comment by Aaron Schulz (talk | contribs)   20:41, 15 November 2010

Typo fixed in r76701.

Status & tagging log