From: Tim Starling Date: Thu, 1 Dec 2005 10:37:47 +0000 (+0000) Subject: Faster IP blocks. Requires schema change. X-Git-Tag: 1.6.0~1087 X-Git-Url: http://git.cyclocoop.org/url?a=commitdiff_plain;h=55671d7040e820552344dab96caf40c25085259a;p=lhc%2Fweb%2Fwiklou.git Faster IP blocks. Requires schema change. --- diff --git a/includes/Block.php b/includes/Block.php index a64dcc37b4..8277746ccd 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -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 index cc3ab90d3b..0000000000 --- a/includes/BlockCache.php +++ /dev/null @@ -1,153 +0,0 @@ -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 ); -} diff --git a/includes/ProxyTools.php b/includes/ProxyTools.php index 5e0f6dde7f..991ec36874 100644 --- a/includes/ProxyTools.php +++ b/includes/ProxyTools.php @@ -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 ); +} + ?> diff --git a/includes/Setup.php b/includes/Setup.php index a5964eff17..adacedeaeb 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -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(); diff --git a/includes/User.php b/includes/User.php index 144c7832c1..294bac7899 100644 --- a/includes/User.php +++ b/includes/User.php @@ -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 index 0000000000..c31e2d9cc9 --- /dev/null +++ b/maintenance/archives/patch-ipb_range_start.sql @@ -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 '%/%'; diff --git a/maintenance/tables.sql b/maintenance/tables.sql index 018e7ebf46..278ae1649e 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -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; diff --git a/maintenance/updaters.inc b/maintenance/updaters.inc index baadae3913..97efd978f5 100644 --- a/maintenance/updaters.inc +++ b/maintenance/updaters.inc @@ -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 ) {