r41902 - Code Review

From MediaWiki.org

Jump to: navigation, search
Repository:MediaWiki
Revision:r41901 | r41902 (on ViewVC) | r41903 >
Date:21:49, 9 October 2008
Author:aaron
Status:ok (Comments)
Tags:
Comment:(bug 14634) Show range blocks for IPs
Modified paths:

Diff [purge]

Index: trunk/phase3/includes/specials/SpecialIpblocklist.php
===================================================================
--- trunk/phase3/includes/specials/SpecialIpblocklist.php	(revision 41901)
+++ trunk/phase3/includes/specials/SpecialIpblocklist.php	(revision 41902)
@@ -78,6 +78,7 @@
 		$this->hideuserblocks = $wgRequest->getBool( 'hideuserblocks' );
 		$this->hidetempblocks = $wgRequest->getBool( 'hidetempblocks' );
 		$this->hideaddressblocks = $wgRequest->getBool( 'hideaddressblocks' );
+		$this->scanRange = $wgRequest->getBool( 'range' );
 	}
 
 	/**
@@ -162,8 +163,7 @@
 	 * @return array array(message key, parameters) on failure, empty array on success
 	 */
 
-	static function doUnblock(&$id, &$ip, &$reason, &$range = null)
-	{
+	static function doUnblock(&$id, &$ip, &$reason, &$range = null) {
 		if ( $id ) {
 			$block = Block::newFromID( $id );
 			if ( !$block ) {
@@ -245,10 +245,20 @@
 			// No extra conditions
 		} elseif ( substr( $this->ip, 0, 1 ) == '#' ) {
 			$conds['ipb_id'] = substr( $this->ip, 1 );
+		// Single IPs
 		} elseif ( IP::isIPAddress($this->ip) && strpos($this->ip,'/') === false ) {
-			$conds['ipb_address'] = IP::sanitizeIP($this->ip);
+			if( $this->scanRange && $iaddr = IP::toHex($this->ip) ) {
+				# Only scan ranges which start in this /16, this improves search speed
+				# Blocks should not cross a /16 boundary.
+				$range = substr( $iaddr, 0, 4 );
+				$conds[] = "(ipb_address = '" . IP::sanitizeIP($this->ip) . "') OR 
+					(ipb_range_start LIKE '$range%' AND ipb_range_start <= '$iaddr' AND ipb_range_end >= '$iaddr')";
+			} else {
+				$conds['ipb_address'] = IP::sanitizeIP($this->ip);
+			}
 			$conds['ipb_auto'] = 0;
-		} elseif( IP::isIPAddress($this->ip) ) {
+		// IP range
+		} elseif ( IP::isIPAddress($this->ip) ) {
 			$conds['ipb_address'] = Block::normaliseRange( $this->ip );
 			$conds['ipb_auto'] = 0;
 		} else {
@@ -298,8 +308,9 @@
 				Xml::openElement( 'fieldset' ) .
 				Xml::element( 'legend', null, wfMsg( 'ipblocklist-legend' ) ) .
 				Xml::inputLabel( wfMsg( 'ipblocklist-username' ), 'ip', 'ip', /* size */ false, $this->ip ) .
-				'&nbsp;' .
-				Xml::submitButton( wfMsg( 'ipblocklist-submit' ) ) .
+				'<br/>' . 
+				Xml::checkLabel( wfMsg('ipblocklist-scanrange'), 'range', 'range', $this->scanRange ) .
+				'&nbsp;' . Xml::submitButton( wfMsg( 'ipblocklist-submit' ) ) .
 				Xml::closeElement( 'fieldset' )
 			);
 	}
Index: trunk/phase3/languages/messages/MessagesEn.php
===================================================================
--- trunk/phase3/languages/messages/MessagesEn.php	(revision 41901)
+++ trunk/phase3/languages/messages/MessagesEn.php	(revision 41902)
@@ -2566,6 +2566,7 @@
 'ipblocklist-sh-userblocks'       => '$1 account blocks',
 'ipblocklist-sh-tempblocks'       => '$1 temporary blocks',
 'ipblocklist-sh-addressblocks'    => '$1 single IP blocks',
+'ipblocklist-scanrange'           => 'For IPs, include all blocks that affect the address',
 'ipblocklist-summary'             => '', # do not translate or duplicate this message to other languages
 'ipblocklist-submit'              => 'Search',
 'blocklistline'                   => '$1, $2 blocked $3 ($4)',

Follow-up revisions

RevisionCommit summaryAuthorDate
r42011Tweaks to r41902 "(bug 14634) Show range blocks for IPs"brion01:44, 13 October 2008

Comments

#Comment by Brion VIBBER (Talk | contribs)   01:44, 13 October 2008

Updated a bit in 42011:

  • Remove the "scan range blocks" checkbox -- doing the range check is sane default behavior, and should Just Work. :)
  • Paranoia SQL escaping. We may "know" that the strings will never include something escapable, but then again someone might make a mistake one day. By ensuring we escape them, we have one less thing to worry about, and one less thing to double-check every time we look at this code.

Note that a search for a particular range block currently does not turn up larger range blocks that include it, which it probably should.

Views
Toolbox