Anomie has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/62445


Change subject: API: Fix IPv6 handling in list=blocks
......................................................................

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
---
M RELEASE-NOTES-1.22
M includes/api/ApiQueryBlocks.php
2 files changed, 43 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/45/62445/1

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index ad62c80..f138b95 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -102,6 +102,10 @@
   stored in the database.
 * (bug 47219) Allow specifying change type of Wikipedia feed items
 * prop=imageinfo now allows setting iiurlheight without setting iiurlwidth
+* (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
--- a/includes/api/ApiQueryBlocks.php
+++ b/includes/api/ApiQueryBlocks.php
@@ -91,18 +91,30 @@
                        $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' );
                        }
-                       $prefix = substr( $lower, 0, 4 );
+
+                       # 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' );
+                       }
+
+                       # 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 @@
        }
 
        public function getParamDescription() {
+               global $wgBlockCIDRLimit;
                $p = $this->getModulePrefix();
                return array(
                        'start' => 'The timestamp to start enumerating from',
@@ -302,8 +315,12 @@
                        '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 @@
        }
 
        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' ),

-- 
To view, visit https://gerrit.wikimedia.org/r/62445
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I02471bb32c3a217004d07a79d9f98b62133b31ef
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Anomie <bjor...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to