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