Faster IP blocks. Requires schema change.
authorTim Starling <tstarling@users.mediawiki.org>
Thu, 1 Dec 2005 10:37:47 +0000 (10:37 +0000)
committerTim Starling <tstarling@users.mediawiki.org>
Thu, 1 Dec 2005 10:37:47 +0000 (10:37 +0000)
includes/Block.php
includes/BlockCache.php [deleted file]
includes/ProxyTools.php
includes/Setup.php
includes/User.php
maintenance/archives/patch-ipb_range_start.sql [new file with mode: 0644]
maintenance/tables.sql
maintenance/updaters.inc

index a64dcc3..8277746 100644 (file)
@@ -17,15 +17,16 @@ define ( 'EB_RANGE_ONLY', 4 );
  * loaded or filled. It is not load-on-demand. There are no accessors.
  *
  * To use delete(), you only need to fill $mAddress
- * Globals used: $wgBlockCache, $wgAutoblockExpiry
+ * Globals used: $wgAutoblockExpiry, $wgAntiLockFlags
  *
  * @todo This could be used everywhere, but it isn't.
  * @package MediaWiki
  */
 class Block
 {
-       /* public*/ var $mAddress, $mUser, $mBy, $mReason, $mTimestamp, $mAuto, $mId, $mExpiry;
-       /* private */ var $mNetworkBits, $mIntegerAddr, $mForUpdate, $mByName;
+       /* public*/ var $mAddress, $mUser, $mBy, $mReason, $mTimestamp, $mAuto, $mId, $mExpiry,
+                           $mRangeStart, $mRangeEnd;
+       /* private */ var $mNetworkBits, $mIntegerAddr, $mForUpdate, $mFromMaster, $mByName;
 
        function Block( $address = '', $user = '', $by = 0, $reason = '',
                $timestamp = '' , $auto = 0, $expiry = '' )
@@ -43,6 +44,7 @@ class Block
                }
 
                $this->mForUpdate = false;
+               $this->mFromMaster = false;
                $this->mByName = false;
                $this->initialiseRange();
        }
@@ -63,19 +65,14 @@ class Block
        }
 
        /**
-        * Get a ban from the DB, with either the given address or the given username
+        * Get the DB object and set the reference parameter to the query options
         */
-       function load( $address = '', $user = 0, $killExpired = true )
+       function &getDBOptions( &$options ) 
        {
                global $wgAntiLockFlags;
-               $fname = 'Block::load';
-               wfDebug( "Block::load: '$address', '$user', $killExpired\n" );
-
-               $ret = false;
-               $killed = false;
-               if ( $this->forUpdate() ) {
+               if ( $this->mForUpdate || $this->mFromMaster ) {
                        $db =& wfGetDB( DB_MASTER );
-                       if ( $wgAntiLockFlags & ALF_NO_BLOCK_LOCK ) {
+                       if ( !$this->mForUpdate || ($wgAntiLockFlags & ALF_NO_BLOCK_LOCK) ) {
                                $options = '';
                        } else {
                                $options = 'FOR UPDATE';
@@ -84,15 +81,33 @@ class Block
                        $db =& wfGetDB( DB_SLAVE );
                        $options = '';
                }
+               return $db;
+       }
+
+       /**
+        * Get a ban from the DB, with either the given address or the given username
+        */
+       function load( $address = '', $user = 0, $killExpired = true )
+       {
+               $fname = 'Block::load';
+               wfDebug( "Block::load: '$address', '$user', $killExpired\n" );
+
+               $options = '';
+               $db =& $this->getDBOptions( $options );
+
+               $ret = false;
+               $killed = false;
                $ipblocks = $db->tableName( 'ipblocks' );
 
-               if ( 0 == $user && $address=='' ) {
-                       $sql = "SELECT * from $ipblocks $options";
-               } elseif ($address=="") {
+               if ( 0 == $user && $address == '' ) {
+                       # Invalid user specification, not blocked
+                       $this->clear();
+                       return false;
+               } elseif ( $address == '' ) {
                        $sql = "SELECT * FROM $ipblocks WHERE ipb_user={$user} $options";
-               } elseif ($user=="") {
-                       $sql = "SELECT * FROM $ipblocks WHERE ipb_address='" . $db->strencode( $address ) . "' $options";
-               } elseif ( $options=='' ) {
+               } elseif ( $user == '' ) {
+                       $sql = "SELECT * FROM $ipblocks WHERE ipb_address=" . $db->addQuotes( $address ) . " $options";
+               } elseif ( $options == '' ) {
                        # If there are no options (e.g. FOR UPDATE), use a UNION
                        # so that the query can make efficient use of indices
                        $sql = "SELECT * FROM $ipblocks WHERE ipb_address='" . $db->strencode( $address ) .
@@ -105,10 +120,7 @@ class Block
                }
 
                $res = $db->query( $sql, $fname );
-               if ( 0 == $db->numRows( $res ) ) {
-                       # User is not blocked
-                       $this->clear();
-               } else {
+               if ( 0 != $db->numRows( $res ) ) {
                        # Get first block
                        $row = $db->fetchObject( $res );
                        $this->initFromRow( $row );
@@ -137,9 +149,72 @@ class Block
                        }
                }
                $db->freeResult( $res );
+
+               # No blocks found yet? Try looking for range blocks
+               if ( !$ret && $address != '' ) {
+                       $ret = $this->loadRange( $address, $killExpired );
+               }
+               if ( !$ret ) {
+                       $this->clear();
+               }
+
                return $ret;
        }
 
+       /**
+        * Search the database for any range blocks matching the given address, and 
+        * load the row if one is found.
+        */
+       function loadRange( $address, $killExpired = true )
+       {
+               $fname = 'Block::loadRange';
+
+               $iaddr = wfIP2Hex( $address );
+               if ( $iaddr === false ) {
+                       # Invalid address
+                       return false;
+               }
+
+               # Only scan ranges which start in this /16, this improves search speed
+               # Blocks should not cross a /16 boundary.
+               $range = substr( $iaddr, 0, 4 );
+               
+               $options = '';
+               $db =& $this->getDBOptions( $options );
+               $ipblocks = $db->tableName( 'ipblocks' );
+               $sql = "SELECT * FROM $ipblocks WHERE ipb_range_start LIKE '$range%' ".
+                       "AND ipb_range_start <= '$iaddr' AND ipb_range_end >= '$iaddr' $options";
+               $res = $db->query( $sql, $fname );
+               $row = $db->fetchObject( $res );
+
+               $success = false;
+               if ( $row ) {
+                       # Found a row, initialise this object
+                       $this->initFromRow( $row );
+
+                       # Is it expired?
+                       if ( !$killExpired || !$this->deleteIfExpired() ) {
+                               # No, return true
+                               $success = true;
+                       } 
+               }
+               
+               $db->freeResult( $res );
+               return $success;
+       }
+
+       /**
+        * Determine if a given integer IPv4 address is in a given CIDR network
+        */
+       function isAddressInRange( $addr, $range ) {
+               list( $network, $bits ) = wfParseCIDR( $range );
+               if ( $network !== false && $addr >> ( 32 - $bits ) == $network >> ( 32 - $bits ) ) {
+                       return true;
+               } else {
+                       return false;
+               }
+       }
+
        function initFromRow( $row )
        {
                $this->mAddress = $row->ipb_address;
@@ -157,24 +232,21 @@ class Block
                } else {
                        $this->mByName = false;
                }
-
-               $this->initialiseRange();
+               $this->mRangeStart = $row->ipb_range_start;
+               $this->mRangeEnd = $row->ipb_range_end;
        }
 
        function initialiseRange()
        {
+               $this->mRangeStart = '';
+               $this->mRangeEnd = '';
                if ( $this->mUser == 0 ) {
-                       $rangeParts = explode( '/', $this->mAddress );
-                       if ( count( $rangeParts ) == 2 ) {
-                               $this->mNetworkBits = $rangeParts[1];
-                       } else {
-                               $this->mNetworkBits = 32;
+                       list( $network, $bits ) = wfParseCIDR( $this->mAddress );
+                       if ( $network !== false ) {
+                               $this->mRangeStart = sprintf( '%08X', $network );
+                               $this->mRangeEnd = sprintf( '%08X', $network + (1 << (32 - $bits)) - 1 );
                        }
-                       $this->mIntegerAddr = ip2long( $rangeParts[0] );
-               } else {
-                       $this->mNetworkBits = false;
-                       $this->mIntegerAddr = false;
-               }
+               }       
        }
 
        /**
@@ -199,7 +271,7 @@ class Block
                        $options = '';
                }
                if ( $flags & EB_RANGE_ONLY ) {
-                       $cond = " AND ipb_address LIKE '%/%'";
+                       $cond = " AND ipb_range_start <> ''";
                } else {
                        $cond = '';
                }
@@ -215,7 +287,7 @@ class Block
 
                while ( $row = $db->fetchObject( $res ) ) {
                        $block->initFromRow( $row );
-                       if ( ( $flags & EB_RANGE_ONLY ) && $block->getNetworkBits() == 32 ) {
+                       if ( ( $flags & EB_RANGE_ONLY ) && $block->mRangeStart == '' ) {
                                continue;
                        }
 
@@ -247,7 +319,6 @@ class Block
                        $condition = array( 'ipb_address' => $this->mAddress );
                }
                $dbw->delete( 'ipblocks', $condition, $fname );
-               $this->clearCache();
        }
 
        function insert()
@@ -267,10 +338,10 @@ class Block
                                'ipb_expiry' => $this->mExpiry ?
                                        $dbw->timestamp($this->mExpiry) :
                                        $this->mExpiry,
+                               'ipb_range_start' => $this->mRangeStart,
+                               'ipb_range_end' => $this->mRangeEnd,
                        ), 'Block::insert'
                );
-
-               $this->clearCache();
        }
 
        function deleteIfExpired()
@@ -319,19 +390,10 @@ class Block
                                        'ipb_address' => $this->mAddress
                                ), 'Block::updateTimestamp'
                        );
-
-                       $this->clearCache();
-               }
-       }
-
-       /* private */ function clearCache()
-       {
-               global $wgBlockCache;
-               if ( is_object( $wgBlockCache ) ) {
-                       $wgBlockCache->loadFromDB();
                }
        }
 
+       /*
        function getIntegerAddr()
        {
                return $this->mIntegerAddr;
@@ -340,7 +402,7 @@ class Block
        function getNetworkBits()
        {
                return $this->mNetworkBits;
-       }
+       }*/
 
        function getByName()
        {
@@ -354,6 +416,10 @@ class Block
                return wfSetVar( $this->mForUpdate, $x );
        }
 
+       function fromMaster( $x = NULL ) {
+               return wfSetVar( $this->mFromMaster, $x );
+       }
+
        /* static */ function getAutoblockExpiry( $timestamp )
        {
                global $wgAutoblockExpiry;
@@ -365,7 +431,7 @@ class Block
                $parts = explode( '/', $range );
                if ( count( $parts ) == 2 ) {
                        $shift = 32 - $parts[1];
-                       $ipint = ip2long( $parts[0] );
+                       $ipint = wfIP2Unsigned( $parts[0] );
                        $ipint = $ipint >> $shift << $shift;
                        $newip = long2ip( $ipint );
                        $range = "$newip/{$parts[1]}";
diff --git a/includes/BlockCache.php b/includes/BlockCache.php
deleted file mode 100644 (file)
index cc3ab90..0000000
+++ /dev/null
@@ -1,153 +0,0 @@
-<?php
-/**
- * Contain the blockcache class
- * @package Cache
- */
-
-/**
- * Object for fast lookup of IP blocks
- * Represents a memcached value, and in some sense, the entire ipblocks table
- * @package MediaWiki
- */
-class BlockCache
-{
-       var $mData = false, $mMemcKey;
-
-       /**
-        * Constructor
-        * Create a new BlockCache object
-        *
-        * @param Boolean $deferLoad   specifies whether to immediately load the data from memcached.
-        * @param String $dbName       specifies the memcached dbName prefix to be used. Defaults to $wgDBname.
-        */
-       function BlockCache( $deferLoad = false, $dbName = '' ) {
-               global $wgDBname;
-
-               if ( $dbName == '' ) {
-                       $dbName = $wgDBname;
-               }
-
-               $this->mMemcKey = $dbName.':ipblocks';
-
-               if ( !$deferLoad ) {
-                       $this->load();
-               }
-       }
-
-       /**
-        * Load the blocks from the database and save them to memcached
-        *  @param bool $bFromSlave Whether to load data from slaves or master
-        */
-       function loadFromDB( $bFromSlave = false ) {
-               global $wgUseMemCached, $wgMemc;
-               $fname = 'BlockCache::loadFromDB';
-               wfProfileIn( $fname );
-
-               $this->mData = array();
-               # Selecting FOR UPDATE is a convenient way to serialise the memcached and DB operations,
-               # which is necessary even though we don't update the DB
-               if ( !$bFromSlave ) {
-                       Block::enumBlocks( 'wfBlockCacheInsert', '', EB_FOR_UPDATE | EB_RANGE_ONLY );
-                       #$wgMemc->set( $this->mMemcKey, $this->mData, 0 );
-               } else {
-                       Block::enumBlocks( 'wfBlockCacheInsert', '', EB_RANGE_ONLY );
-               }
-               wfProfileOut( $fname );
-       }
-               
-       /**
-        * Load the cache from memcached or, if that's not possible, from the DB
-        */
-       function load( $bFromSlave ) {
-               global $wgUseMemCached, $wgMemc;
-
-               if ( $this->mData === false) {
-                       $this->loadFromDB( $bFromSlave );
-/*
-               // Memcache disabled for performance issues.
-                       # Try memcached
-                       if ( $wgUseMemCached ) {
-                               $this->mData = $wgMemc->get( $this->mMemcKey );
-                       }
-
-                       if ( !is_array( $this->mData ) ) {
-                               $this->loadFromDB( $bFromSlave );
-                       }*/
-               }
-       }
-
-       /**
-        * Add a block to the cache
-        *
-        * @param Object &$block   Reference to a "Block" object.
-        */
-       function insert( &$block ) {
-               if ( $block->mUser == 0 ) {
-                       $nb = $block->getNetworkBits();
-                       $ipint = $block->getIntegerAddr();
-                       $index = $ipint >> ( 32 - $nb );
-
-                       if ( !array_key_exists( $nb, $this->mData ) ) {
-                               $this->mData[$nb] = array();
-                       }
-               
-                       $this->mData[$nb][$index] = 1;
-               }
-       }
-       
-       /**
-        * Find out if a given IP address is blocked
-        *
-        * @param String $ip   IP address
-        * @param bool $bFromSlave True means to load check against slave, else check against master.
-        */
-       function get( $ip, $bFromSlave ) {
-               $fname = 'BlockCache::get';
-               wfProfileIn( $fname );
-
-               $this->load( $bFromSlave );
-               $ipint = ip2long( $ip );
-               $blocked = false;
-
-               foreach ( $this->mData as $networkBits => $blockInts ) {
-                       if ( array_key_exists( $ipint >> ( 32 - $networkBits ), $blockInts ) ) {
-                               $blocked = true;
-                               break;
-                       }
-               }
-               if ( $blocked ) {
-                       # Clear low order bits
-                       if ( $networkBits != 32 ) {
-                               $ip .= '/'.$networkBits;
-                               $ip = Block::normaliseRange( $ip );
-                       }
-                       $block = new Block();
-                       $block->forUpdate( $bFromSlave );
-                       $block->load( $ip );
-               } else {
-                       $block = false;
-               }
-
-               wfProfileOut( $fname );
-               return $block;
-       }
-
-       /**
-        * Clear the local cache
-        * There was once a clear() to clear memcached too, but I deleted it
-        */
-       function clearLocal() {
-               $this->mData = false;
-       }
-}
-
-/**
- * Add a block to the global $wgBlockCache
- *
- * @param Object $block  A "Block"-object
- * @param Any    $tag    unused
- */
-function wfBlockCacheInsert( $block, $tag ) {
-       global $wgBlockCache;
-       $wgBlockCache->insert( $block );
-}
index 5e0f6dd..991ec36 100644 (file)
@@ -55,7 +55,10 @@ function wfGetIP() {
        return $ip;
 }
 
-/** */
+/** 
+ * Given an IP address in dotted-quad notation, returns an unsigned integer.
+ * Like ip2long() except that it actually works and has a consistent error return value.
+ */
 function wfIP2Unsigned( $ip ) {
        $n = ip2long( $ip );
        if ( $n == -1 || $n === false ) { # Return value on error depends on PHP version
@@ -66,6 +69,17 @@ function wfIP2Unsigned( $ip ) {
        return $n;
 }
 
+/**
+ * Return a zero-padded hexadecimal representation of an IP address
+ */
+function wfIP2Hex( $ip ) {
+       $n = wfIP2Unsigned( $ip );
+       if ( $n !== false ) {
+               $n = sprintf( '%08X', $n );
+       }
+       return $n;
+}
+
 /**
  * Determine if an IP address really is an IP address, and if it is public, 
  * i.e. not RFC 1918 or similar
@@ -143,4 +157,22 @@ function wfProxyCheck() {
        }
 }
 
+/**
+ * Convert a network specification in CIDR notation to an integer network and a number of bits
+ */
+function wfParseCIDR( $range ) {
+       $parts = explode( '/', $range, 2 );
+       if ( count( $parts ) != 2 ) {
+               return array( false, false );
+       }
+       $network = wfIP2Unsigned( $parts[0] );
+       if ( $network !== false && is_numeric( $parts[1] ) && $parts[1] >= 0 && $parts[1] <= 32 ) {
+               $bits = $parts[1];
+       } else {
+               $network = false;
+               $bits = false;
+       }
+       return array( $network, $bits );
+}
+
 ?>
index a5964ef..adacede 100644 (file)
@@ -62,7 +62,6 @@ require_once( 'Article.php' );
 require_once( 'MagicWord.php' );
 require_once( 'Block.php' );
 require_once( 'MessageCache.php' );
-require_once( 'BlockCache.php' );
 require_once( 'Parser.php' );
 require_once( 'ParserCache.php' );
 require_once( 'WebRequest.php' );
@@ -269,11 +268,6 @@ wfProfileIn( $fname.'-OutputPage' );
 $wgOut = new OutputPage();
 
 wfProfileOut( $fname.'-OutputPage' );
-wfProfileIn( $fname.'-BlockCache' );
-
-$wgBlockCache = new BlockCache( true );
-
-wfProfileOut( $fname.'-BlockCache' );
 wfProfileIn( $fname.'-misc2' );
 
 $wgDeferredUpdateList = array();
index 144c783..294bac7 100644 (file)
@@ -372,14 +372,9 @@ class User {
         * @param bool $bFromSlave Specify whether to check slave or master. To improve performance,
         *  non-critical checks are done against slaves. Check when actually saving should be done against
         *  master.
-        *
-        * Note that even if $bFromSlave is false, the check is done first against slave, then master.
-        * The logic is that if blocked on slave, we'll assume it's either blocked on master or
-        * just slightly outta sync and soon corrected - safer to block slightly more that less.
-        * And it's cheaper to check slave first, then master if needed, than master always.
         */
        function getBlockedStatus( $bFromSlave = true ) {
-               global $wgBlockCache, $wgProxyList, $wgEnableSorbs, $wgProxyWhitelist;
+               global $wgProxyList, $wgEnableSorbs, $wgProxyWhitelist;
 
                if ( -1 != $this->mBlockedby ) {
                        wfDebug( "User::getBlockedStatus: already loaded.\n" );
@@ -395,7 +390,7 @@ class User {
 
                # User/IP blocking
                $block = new Block();
-               $block->forUpdate( $bFromSlave );
+               $block->fromMaster( !$bFromSlave );
                if ( $block->load( $ip , $this->mId ) ) {
                        wfDebug( "$fname: Found block.\n" );
                        $this->mBlockedby = $block->mBy;
@@ -407,23 +402,6 @@ class User {
                        wfDebug( "$fname: No block.\n" );
                }
 
-               # Range blocking
-               if ( !$this->mBlockedby ) {
-                       # Check first against slave, and optionally from master.
-                       wfDebug( "$fname: Checking range blocks\n" );
-                       $block = $wgBlockCache->get( $ip, true );
-                       if ( !$block && !$bFromSlave )
-                               {
-                               # Not blocked: check against master, to make sure.
-                               $wgBlockCache->clearLocal( );
-                               $block = $wgBlockCache->get( $ip, false );
-                               }
-                       if ( $block !== false ) {
-                               $this->mBlockedby = $block->mBy;
-                               $this->mBlockreason = $block->mReason;
-                       }
-               }
-
                # Proxy blocking
                if ( !$this->isSysop() && !in_array( $ip, $wgProxyWhitelist ) ) {
 
diff --git a/maintenance/archives/patch-ipb_range_start.sql b/maintenance/archives/patch-ipb_range_start.sql
new file mode 100644 (file)
index 0000000..c31e2d9
--- /dev/null
@@ -0,0 +1,25 @@
+-- Add the range handling fields
+ALTER TABLE /*$wgDBprefix*/ipblocks 
+  ADD ipb_range_start varchar(32) NOT NULL default '',
+  ADD ipb_range_end varchar(32) NOT NULL default '',
+  ADD INDEX ipb_range (ipb_range_start(8), ipb_range_end(8));
+
+
+-- Initialise fields
+-- Only range blocks match ipb_address LIKE '%/%', this fact is used in the code already
+UPDATE /*$wgDBprefix*/ipblocks 
+  SET 
+    ipb_range_start = LPAD(HEX( 
+        (SUBSTRING_INDEX(ipb_address, '.', 1) << 24)
+      + (SUBSTRING_INDEX(SUBSTRING_INDEX(ipb_address, '.', 2), '.', -1) << 16)
+      + (SUBSTRING_INDEX(SUBSTRING_INDEX(ipb_address, '.', 3), '.', -1) << 24)
+      + (SUBSTRING_INDEX(SUBSTRING_INDEX(ipb_address, '/', 1), '.', -1)) ), 8, '0' ),
+
+    ipb_range_end = LPAD(HEX( 
+        (SUBSTRING_INDEX(ipb_address, '.', 1) << 24)
+      + (SUBSTRING_INDEX(SUBSTRING_INDEX(ipb_address, '.', 2), '.', -1) << 16)
+      + (SUBSTRING_INDEX(SUBSTRING_INDEX(ipb_address, '.', 3), '.', -1) << 24)
+      + (SUBSTRING_INDEX(SUBSTRING_INDEX(ipb_address, '/', 1), '.', -1))
+      + ((1 << (32 - SUBSTRING_INDEX(ipb_address, '/', -1))) - 1) ), 8, '0' )
+
+  WHERE ipb_address LIKE '%/%';
index 018e7eb..278ae16 100644 (file)
@@ -502,10 +502,16 @@ CREATE TABLE /*$wgDBprefix*/ipblocks (
   
   -- Time at which the block will expire.
   ipb_expiry char(14) binary NOT NULL default '',
-
+  
+  -- Start and end of an address range, in hexadecimal
+  -- Size chosen to allow IPv6
+  ipb_range_start varchar(32) NOT NULL default '',
+  ipb_range_end varchar(32) NOT NULL default '',
+  
   PRIMARY KEY ipb_id (ipb_id),
   INDEX ipb_address (ipb_address),
-  INDEX ipb_user (ipb_user)
+  INDEX ipb_user (ipb_user),
+  INDEX ipb_range (ipb_range_start(8), ipb_range_end(8)),
 
 ) TYPE=InnoDB;
 
index baadae3..97efd97 100644 (file)
@@ -49,7 +49,8 @@ $wgNewFields = array(
        array( 'image',         'img_media_type',   'patch-img_media_type.sql' ),
        array( 'validate',      'val_ip',           'patch-val_ip.sql' ),
        array( 'site_stats',    'ss_total_pages',   'patch-ss_total_articles.sql' ),
-       array( 'interwiki',     'iw_trans',         'patch-interwiki-trans.sql' ),
+       array( 'interwiki',     'iw_trans',         'patch-interwiki-trans.sql' ),
+       array( 'ipblocks',      'ipb_range_start',  'patch-ipb_range_start.sql' ),
 );
 
 function rename_table( $from, $to, $patch ) {