r76267 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r76266‎ | r76267 | r76268 >
Date:20:31, 7 November 2010
Author:aaron
Status:ok (Comments)
Tags:
Comment:
* Fixed RE_IPV6_ADD for IP networks ending in "::", like "abcd::/y" or "a::/y"
* Fixed formatHex() for IPv6 by handling prefix properly
* hextoOctet -> hexToOctet
* Assorted code cleanups (mostly with $bits/$network)
* Improved various code comments/docs
Modified paths:
  • /trunk/phase3/includes/IP.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/IP.php
@@ -36,12 +36,15 @@
3737 // An IPv6 block is an IP address and a prefix (d1 to d128)
3838 define( 'RE_IPV6_PREFIX', '(12[0-8]|1[01][0-9]|[1-9]?\d)');
3939 // An IPv6 IP is made up of 8 octets. However abbreviations like "::" can be used.
40 -// This is lax! Number of octets/double colons validation not done.
 40+// This is lax! The number of colon groups is checked (1 to 7) but
 41+// the number of double colons is not validated (must be 0 to 1).
4142 define( 'RE_IPV6_ADD',
4243 '(' .
43 - ':(:' . RE_IPV6_WORD . '){1,7}' . // IPs that start with ":"
 44+ ':(:' . RE_IPV6_WORD . '){1,7}' . // starts with "::"
4445 '|' .
45 - RE_IPV6_WORD . '(:{1,2}' . RE_IPV6_WORD . '|::$){1,7}' . // IPs that don't start with ":"
 46+ RE_IPV6_WORD . '(:{1,2}' . RE_IPV6_WORD . '){0,6}::' . // ends with "::"
 47+ '|' .
 48+ RE_IPV6_WORD . '(:{1,2}' . RE_IPV6_WORD . '){1,7}' . // neither of the above
4649 ')'
4750 );
4851 define( 'RE_IPV6_BLOCK', RE_IPV6_ADD . '\/' . RE_IPV6_PREFIX );
@@ -60,9 +63,9 @@
6164 */
6265 class IP {
6366 /**
64 - * Given a string, determine if it as valid IP
65 - * Unlike isValid(), this looks for networks too
66 - * @param $ip IP address.
 67+ * Given a string, determine if it as valid IP.
 68+ * Note: Unlike isValid(), this looks for networks too.
 69+ * @param $ip string possible IP address
6770 * @return string
6871 */
6972 public static function isIPAddress( $ip ) {
@@ -70,30 +73,40 @@
7174 return false;
7275 }
7376 if ( is_array( $ip ) ) {
74 - throw new MWException( 'invalid value passed to ' . __METHOD__ );
 77+ throw new MWException( 'invalid value passed to ' . __METHOD__ );
7578 }
76 - // IPv6 IPs with two "::" strings are ambiguous and thus invalid
77 - return preg_match( '/^' . IP_ADDRESS_STRING . '$/', $ip ) && ( substr_count( $ip, '::' ) < 2 );
 79+ return preg_match( '/^' . IP_ADDRESS_STRING . '$/', $ip )
 80+ && ( substr_count( $ip, '::' ) <= 1 ); // IPv6 IPs with 2+ "::" are ambiguous
7881 }
7982
 83+ /**
 84+ * Given a string, determine if it as valid IP in IPv6 only.
 85+ * Note: Unlike isValid(), this looks for networks too.
 86+ * @param $ip string possible IP address
 87+ * @return string
 88+ */
8089 public static function isIPv6( $ip ) {
8190 if ( !$ip ) {
8291 return false;
8392 }
84 - if( is_array( $ip ) ) {
 93+ if ( is_array( $ip ) ) {
8594 throw new MWException( 'invalid value passed to ' . __METHOD__ );
8695 }
87 - $doubleColons = substr_count( $ip, '::' );
88 - // IPv6 IPs with two "::" strings are ambiguous and thus invalid
8996 return preg_match( '/^' . RE_IPV6_ADD . '(\/' . RE_IPV6_PREFIX . '|)$/', $ip )
90 - && ( $doubleColons == 1 || substr_count( $ip, ':' ) == 7 );
 97+ && ( substr_count( $ip, '::' ) <= 1 ); // IPv6 IPs with 2+ "::" are ambiguous
9198 }
9299
 100+ /**
 101+ * Given a string, determine if it as valid IP in IPv4 only.
 102+ * Note: Unlike isValid(), this looks for networks too.
 103+ * @param $ip string possible IP address
 104+ * @return string
 105+ */
93106 public static function isIPv4( $ip ) {
94107 if ( !$ip ) {
95108 return false;
96109 }
97 - return preg_match( '/^' . RE_IP_ADD . '(\/' . RE_IP_PREFIX . '|)$/', $ip);
 110+ return preg_match( '/^' . RE_IP_ADD . '(\/' . RE_IP_PREFIX . '|)$/', $ip );
98111 }
99112
100113 /**
@@ -117,9 +130,10 @@
118131 if ( count( $parts ) != 2 ) {
119132 return false;
120133 }
121 - $network = self::toUnsigned( $parts[0] );
122 - if ( $network !== false && is_numeric( $parts[1] ) && $parts[1] >= 0 && $parts[1] <= 32 ) {
123 - $bits = $parts[1] + 96;
 134+ list( $network, $bits ) = $parts;
 135+ $network = self::toUnsigned( $network );
 136+ if ( $network !== false && is_numeric( $bits ) && $bits >= 0 && $bits <= 32 ) {
 137+ $bits += 96;
124138 return self::toOctet( $network ) . "/$bits";
125139 } else {
126140 return false;
@@ -186,7 +200,10 @@
187201 $extra = ':';
188202 $pad = 8; // 6+2 (due to '::')
189203 }
190 - $ip = str_replace( '::', str_repeat( $repeat, $pad - substr_count( $ip, ':' ) ) . $extra, $ip );
 204+ $ip = str_replace( '::',
 205+ str_repeat( $repeat, $pad - substr_count( $ip, ':' ) ) . $extra,
 206+ $ip
 207+ );
191208 }
192209 // Remove leading zereos from each bloc as needed
193210 $ip = preg_replace( '/(^|:)0+' . RE_IPV6_WORD . '/', '$1$2', $ip );
@@ -213,21 +230,23 @@
214231
215232 /**
216233 * Convert an IPv4 or IPv6 hexadecimal representation back to readable format
 234+ * @param $hex string number, with "v6-" prefix if it is IPv6
 235+ * @return string quad-dotted (IPv4) or octet notation (IPv6)
217236 */
218237 public static function formatHex( $hex ) {
219 - if ( substr( $hex, 0, 3 ) == 'v6-' ) {
220 - return self::hexToOctet( $hex );
221 - } else {
 238+ if ( substr( $hex, 0, 3 ) == 'v6-' ) { // IPv6
 239+ return self::hexToOctet( substr( $hex, 3 ) );
 240+ } else { // IPv4
222241 return self::hexToQuad( $hex );
223242 }
224243 }
225244
226245 /**
227 - * Given a hexadecimal number, returns to an IPv6 address in octet notation
 246+ * Converts a hexadecimal number to an IPv6 address in octet notation
228247 * @param $ip_hex string hex IP
229 - * @return string
 248+ * @return string (of format a:b:c:d:e:f:g:h)
230249 */
231 - public static function hextoOctet( $ip_hex ) {
 250+ public static function hexToOctet( $ip_hex ) {
232251 // Convert to padded uppercase hex
233252 $ip_hex = str_pad( strtoupper( $ip_hex ), 32, '0' );
234253 // Separate into 8 octets
@@ -241,12 +260,11 @@
242261 }
243262
244263 /**
245 - * Converts a hexadecimal number to an IPv4 address in octet notation
 264+ * Converts a hexadecimal number to an IPv4 address in quad-dotted notation
246265 * @param $ip string Hex IP
247 - * @return string
 266+ * @return string (of format a.b.c.d)
248267 */
249268 public static function hexToQuad( $ip ) {
250 - // Converts a hexadecimal IP to nnn.nnn.nnn.nnn format
251269 $s = '';
252270 for ( $i = 0; $i < 4; $i++ ) {
253271 if ( $s !== '' ) {
@@ -258,34 +276,35 @@
259277 }
260278
261279 /**
262 - * Convert a network specification in IPv6 CIDR notation to an integer network and a number of bits
 280+ * Convert a network specification in IPv6 CIDR notation to an
 281+ * integer network and a number of bits
263282 * @return array(string, int)
264283 */
265284 public static function parseCIDR6( $range ) {
266 - # Expand any IPv6 IP
 285+ # Explode into <expanded IP,range>
267286 $parts = explode( '/', IP::sanitizeIP( $range ), 2 );
268287 if ( count( $parts ) != 2 ) {
269288 return array( false, false );
270289 }
271 - $network = self::toUnsigned6( $parts[0] );
272 - if ( $network !== false && is_numeric( $parts[1] ) && $parts[1] >= 0 && $parts[1] <= 128 ) {
273 - $bits = $parts[1];
 290+ list( $network, $bits ) = $parts;
 291+ $network = self::toUnsigned6( $network );
 292+ if ( $network !== false && is_numeric( $bits ) && $bits >= 0 && $bits <= 128 ) {
274293 if ( $bits == 0 ) {
275 - $network = 0;
 294+ $network = "0";
276295 } else {
277 - # Native 32 bit functions WONT work here!!!
278 - # Convert to a padded binary number
 296+ # Native 32 bit functions WONT work here!!!
 297+ # Convert to a padded binary number
279298 $network = wfBaseConvert( $network, 10, 2, 128 );
280 - # Truncate the last (128-$bits) bits and replace them with zeros
 299+ # Truncate the last (128-$bits) bits and replace them with zeros
281300 $network = str_pad( substr( $network, 0, $bits ), 128, 0, STR_PAD_RIGHT );
282 - # Convert back to an integer
 301+ # Convert back to an integer
283302 $network = wfBaseConvert( $network, 2, 10 );
284303 }
285304 } else {
286305 $network = false;
287306 $bits = false;
288307 }
289 - return array( $network, $bits );
 308+ return array( $network, (int)$bits );
290309 }
291310
292311 /**
@@ -301,8 +320,8 @@
302321 public static function parseRange6( $range ) {
303322 # Expand any IPv6 IP
304323 $range = IP::sanitizeIP( $range );
 324+ // CIDR notation...
305325 if ( strpos( $range, '/' ) !== false ) {
306 - # CIDR
307326 list( $network, $bits ) = self::parseCIDR6( $range );
308327 if ( $network === false ) {
309328 $start = $end = false;
@@ -318,8 +337,8 @@
319338 $start = "v6-$start";
320339 $end = "v6-$end";
321340 }
 341+ // Explicit range notation...
322342 } elseif ( strpos( $range, '-' ) !== false ) {
323 - # Explicit range
324343 list( $start, $end ) = array_map( 'trim', explode( '-', $range, 2 ) );
325344 $start = self::toUnsigned6( $start );
326345 $end = self::toUnsigned6( $end );
@@ -478,16 +497,16 @@
479498
480499 /**
481500 * Convert a network specification in CIDR notation to an integer network and a number of bits
482 - * @return array(string, int)
 501+ * @return array(int, int)
483502 */
484503 public static function parseCIDR( $range ) {
485504 $parts = explode( '/', $range, 2 );
486505 if ( count( $parts ) != 2 ) {
487506 return array( false, false );
488507 }
489 - $network = self::toSigned( $parts[0] );
490 - if ( $network !== false && is_numeric( $parts[1] ) && $parts[1] >= 0 && $parts[1] <= 32 ) {
491 - $bits = $parts[1];
 508+ list( $network, $bits ) = $parts;
 509+ $network = self::toSigned( $network );
 510+ if ( $network !== false && is_numeric( $bits ) && $bits >= 0 && $bits <= 32 ) {
492511 if ( $bits == 0 ) {
493512 $network = 0;
494513 } else {

Follow-up revisions

RevisionCommit summaryAuthorDate
r76275Similar to r76267 but for JS. Should finish bug 24293.aaron22:46, 7 November 2010
r76514* Followed-up r76267:...aaron12:18, 11 November 2010

Comments

#Comment by Aaron Schulz (talk | contribs)   17:15, 9 November 2010

Note: a bit more work still on the way with isAddress stuff.

Status & tagging log