API: Fix IPv6 handling in list=blocks

The current handling of the bkip parameter assumes IPv4, and breaks for
IPv6 CIDR ranges (it also isn't working right for IPv6 non-CIDR, but not
in an obvious way).

This rewrite handles IPv6 correctly. It also necessarily adds validation
for the bkip parameter, which would formerly return (not very sensible)
results when passed invalid values.

Bug: 48129
Change-Id: I02471bb32c3a217004d07a79d9f98b62133b31ef

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 841de35..52ca63f 100644 (file)
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -174,6 +174,10 @@ production.
   image (sha1 and timestamp).
 * (bug 49239) action=parse now can parse in preview and section preview modes.
 * (bug 49259) action=patrol now accepts revision ids.
+* (bug 48129) list=blocks&bkip= now correctly handles IPv6 CIDR ranges and
+  honors $wgBlockCIDRLimit. Note any clients passing invalid values to bkip
+  will now receive an error, rather than the previous behavior listing all
+  user blocks.
 
 === Languages updated in 1.22===
 

diff --git a/includes/api/ApiQueryBlocks.php b/includes/api/ApiQueryBlocks.php
index 881269f..e3c27f5 100644 (file)
--- a/includes/api/ApiQueryBlocks.php
+++ b/includes/api/ApiQueryBlocks.php
@@ -91,18 +91,30 @@ class ApiQueryBlocks extends ApiQueryBase {
                        $this->addWhereFld( 'ipb_auto', 0 );
                }
                if ( isset( $params['ip'] ) ) {
-                       list( $ip, $range ) = IP::parseCIDR( $params['ip'] );
-                       if ( $ip && $range ) {
-                               // We got a CIDR range
-                               if ( $range < 16 ) {
-                                       $this->dieUsage( 'CIDR ranges broader than /16 are not accepted', 'cidrtoobroad' );
-                               }
-                               $lower = wfBaseConvert( $ip, 10, 16, 8, false );
-                               $upper = wfBaseConvert( $ip + pow( 2, 32 - $range ) - 1, 10, 16, 8, false );
+                       global $wgBlockCIDRLimit;
+                       if ( IP::isIPv4( $params['ip'] ) ) {
+                               $type = 'IPv4';
+                               $cidrLimit = $wgBlockCIDRLimit['IPv4'];
+                               $prefixLen = 0;
+                       } elseif ( IP::isIPv6( $params['ip'] ) ) {
+                               $type = 'IPv6';
+                               $cidrLimit = $wgBlockCIDRLimit['IPv6'];
+                               $prefixLen = 3; // IP::toHex output is prefixed with "v6-"
                        } else {
-                               $lower = $upper = IP::toHex( $params['ip'] );
+                               $this->dieUsage( 'IP parameter is not valid', 'param_ip' );
+                       }
+
+                       # Check range validity, if it's a CIDR
+                       list( $ip, $range ) = IP::parseCIDR( $params['ip'] );
+                       if ( $ip !== false && $range !== false && $range < $cidrLimit ) {
+                               $this->dieUsage( "$type CIDR ranges broader than /$cidrLimit are not accepted", 'cidrtoobroad' );
                        }
-                       $prefix = substr( $lower, 0, 4 );
+
+                       # Let IP::parseRange handle calculating $upper, instead of duplicating the logic here.
+                       list( $lower, $upper ) = IP::parseRange( $params['ip'] );
+
+                       # Extract the common prefix to any rangeblock affecting this IP/CIDR
+                       $prefix = substr( $lower, 0, $prefixLen + floor( $cidrLimit / 4 ) );
 
                        # Fairly hard to make a malicious SQL statement out of hex characters,
                        # but it is good practice to add quotes
@@ -295,6 +307,7 @@ class ApiQueryBlocks extends ApiQueryBase {
        }
 
        public function getParamDescription() {
+               global $wgBlockCIDRLimit;
                $p = $this->getModulePrefix();
                return array(
                        'start' => 'The timestamp to start enumerating from',
@@ -302,8 +315,12 @@ class ApiQueryBlocks extends ApiQueryBase {
                        'dir' => $this->getDirectionDescription( $p ),
                        'ids' => 'List of block IDs to list (optional)',
                        'users' => 'List of users to search for (optional)',
-                       'ip' => array( 'Get all blocks applying to this IP or CIDR range, including range blocks.',
-                                       'Cannot be used together with bkusers. CIDR ranges broader than /16 are not accepted' ),
+                       'ip' => array(
+                               'Get all blocks applying to this IP or CIDR range, including range blocks.',
+                               "Cannot be used together with bkusers. CIDR ranges broader than " .
+                                       "IPv4/{$wgBlockCIDRLimit['IPv4']} or IPv6/{$wgBlockCIDRLimit['IPv6']} " .
+                                       "are not accepted"
+                       ),
                        'limit' => 'The maximum amount of blocks to list',
                        'prop' => array(
                                'Which properties to get',
@@ -384,10 +401,19 @@ class ApiQueryBlocks extends ApiQueryBase {
        }
 
        public function getPossibleErrors() {
+               global $wgBlockCIDRLimit;
                return array_merge( parent::getPossibleErrors(),
                        $this->getRequireOnlyOneParameterErrorMessages( array( 'users', 'ip' ) ),
                        array(
-                               array( 'code' => 'cidrtoobroad', 'info' => 'CIDR ranges broader than /16 are not accepted' ),
+                               array(
+                                       'code' => 'cidrtoobroad',
+                                       'info' => "IPv4 CIDR ranges broader than /{$wgBlockCIDRLimit['IPv4']} are not accepted"
+                               ),
+                               array(
+                                       'code' => 'cidrtoobroad',
+                                       'info' => "IPv6 CIDR ranges broader than /{$wgBlockCIDRLimit['IPv6']} are not accepted"
+                               ),
+                               array( 'code' => 'param_ip', 'info' => 'IP parameter is not valid' ),
                                array( 'code' => 'param_user', 'info' => 'User parameter may not be empty' ),
                                array( 'code' => 'param_user', 'info' => 'User name user is not valid' ),
                                array( 'show' ),