From 6dbcdc1be097167c25a9b9a95956a9b765204216 Mon Sep 17 00:00:00 2001 From: Happy-melon Date: Mon, 21 Mar 2011 19:12:41 +0000 Subject: [PATCH] Blame hashar for this giant commit; he teased me for making so many smaller ones earlier... :D * Internalise $mAddress/$mUser, $mBy/$mByName, $mEnableAutoblock, $mId as getTarget(), getBlockers(), isAutoblocking(), getId(). * This required editing AbuseFilter and CheckUser backwards-incompatibly, so push the rest of the changes out to those extensions. * Attack the evil 14-parameter constructor and gratuitously-confusing newFromDB( $notVeryImportantParameter, $moreImportantParameter) * Reimplement the hack for bug 13611 in a slightly less fragile fashion; could still do with further cleanup, but then again the login frontend is its own can of worms... :S * Remove transitionary getTargetAndType() and newFromTargetAndType() methods * Some optimisation in parseTarget() * Fix the broken phpunit test mentioned in r84251 --- includes/Article.php | 3 +- includes/Block.php | 371 +++++++++++++------- includes/OutputPage.php | 4 +- includes/Title.php | 4 +- includes/User.php | 57 ++- includes/api/ApiBlock.php | 4 +- includes/api/ApiUnblock.php | 2 +- includes/specials/SpecialBlock.php | 40 +-- includes/specials/SpecialBlockme.php | 8 +- includes/specials/SpecialUnblock.php | 11 +- includes/specials/SpecialUserlogin.php | 30 +- tests/phpunit/includes/api/ApiBlockTest.php | 6 +- 12 files changed, 330 insertions(+), 210 deletions(-) diff --git a/includes/Article.php b/includes/Article.php index 6d3618de1b..560a26125f 100644 --- a/includes/Article.php +++ b/includes/Article.php @@ -1115,8 +1115,7 @@ class Article { if ( $ns == NS_USER || $ns == NS_USER_TALK ) { # Don't index user and user talk pages for blocked users (bug 11443) if ( !$this->mTitle->isSubpage() ) { - $block = new Block(); - if ( $block->load( $this->mTitle->getText() ) ) { + if ( Block::newFromTarget( null, $this->mTitle->getText() ) instanceof Block ) { return array( 'index' => 'noindex', 'follow' => 'nofollow' diff --git a/includes/Block.php b/includes/Block.php index aa2d98e294..a082968908 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -15,18 +15,32 @@ * FIXME: this whole class is a cesspit, needs a complete rewrite */ class Block { - /* public*/ var $mAddress, $mUser, $mBy, $mReason, $mTimestamp, $mAuto, $mId, $mExpiry, - $mEnableAutoblock, $mHideName, - $mByName, $mAngryAutoblock; + /* public*/ var $mUser, $mReason, $mTimestamp, $mAuto, $mExpiry, + $mHideName, + $mAngryAutoblock; protected + $mAddress, + $mId, + $mBy, + $mByName, $mFromMaster, $mRangeStart, $mRangeEnd, $mAnonOnly, + $mEnableAutoblock, $mBlockEmail, $mAllowUsertalk, $mCreateAccount; + /// @var User|String + protected $target; + + /// @var Block::TYPE_ constant. Can only be USER, IP or RANGE internally + protected $type; + + /// @var User + protected $blocker; + # TYPE constants const TYPE_USER = 1; const TYPE_IP = 2; @@ -34,10 +48,19 @@ class Block { const TYPE_AUTO = 4; const TYPE_ID = 5; + /** + * Constructor + * FIXME: Don't know what the best format to have for this constructor is, but fourteen + * optional parameters certainly isn't it. + */ function __construct( $address = '', $user = 0, $by = 0, $reason = '', $timestamp = 0, $auto = 0, $expiry = '', $anonOnly = 0, $createAccount = 0, $enableAutoblock = 0, $hideName = 0, $blockEmail = 0, $allowUsertalk = 0, $byName = false ) { + if( $timestamp === 0 ){ + $timestamp = wfTimestampNow(); + } + $this->mId = 0; # Expand valid IPv6 addresses $address = IP::sanitizeIP( $address ); @@ -139,99 +162,128 @@ class Block { * @param $user int The user ID, or zero for anonymous users * @param $killExpired bool Whether to delete expired rows while loading * @return Boolean: the user is blocked from editing - * + * @deprecated since 1.18 */ public function load( $address = '', $user = 0, $killExpired = true ) { - wfDebug( "Block::load: '$address', '$user', $killExpired\n" ); + wfDeprecated( __METHOD__ ); + if( $user ){ + $username = User::whoIs( $user ); + $block = self::newFromTarget( $username, $address ); + } else { + $block = self::newFromTarget( null, $address ); + } - $db = wfGetDB( $this->mFromMaster ? DB_MASTER : DB_SLAVE ); + if( $block instanceof Block ){ + # This is mildly evil, but hey, it's B/C :D + foreach( $block as $variable => $value ){ + $this->$variable = $value; + } + return true; + } else { + return false; + } + } - if ( 0 == $user && $address === '' ) { - # Invalid user specification, not blocked - $this->clear(); + /** + * Load a block from the database which affects the already-set $this->target: + * 1) A block directly on the given user or IP + * 2) A rangeblock encompasing the given IP (smallest first) + * 3) An autoblock on the given IP + * @param $vagueTarget User|String also search for blocks affecting this target. Doesn't + * make any sense to use TYPE_AUTO / TYPE_ID here + * @return Bool whether a relevant block was found + */ + protected function newLoad( $vagueTarget = null ) { + $db = wfGetDB( $this->mFromMaster ? DB_MASTER : DB_SLAVE ); - return false; + if( $this->type !== null ){ + $conds = array( + 'ipb_address' => array( (string)$this->target ), + ); + } else { + $conds = array( 'ipb_address' => array() ); } - # Try user block - if ( $user ) { - $res = $db->resultObject( $db->select( - 'ipblocks', - '*', - array( 'ipb_user' => $user ), - __METHOD__ - ) ); - - if ( $this->loadFromResult( $res, $killExpired ) ) { - return true; + if( $vagueTarget !== null ){ + list( $target, $type ) = self::parseTarget( $vagueTarget ); + switch( $type ) { + case self::TYPE_USER: + # Slightly wierd, but who are we to argue? + $conds['ipb_address'][] = (string)$target; + break; + + case self::TYPE_IP: + $conds['ipb_address'][] = (string)$target; + $conds[] = self::getRangeCond( IP::toHex( $target ) ); + $conds = $db->makeList( $conds, LIST_OR ); + break; + + case self::TYPE_RANGE: + list( $start, $end ) = IP::parseRange( $target ); + $conds['ipb_address'][] = (string)$target; + $conds[] = self::getRangeCond( $start, $end ); + $conds = $db->makeList( $conds, LIST_OR ); + break; + + default: + throw new MWException( "Tried to load block with invalid type" ); } } - # Try IP block - # TODO: improve performance by merging this query with the autoblock one - # Slightly tricky while handling killExpired as well - if ( $address !== '' ) { - $conds = array( 'ipb_address' => $address, 'ipb_auto' => 0 ); - $res = $db->resultObject( $db->select( - 'ipblocks', - '*', - $conds, - __METHOD__ - ) ); - - if ( $this->loadFromResult( $res, $killExpired ) ) { - if ( $user && $this->mAnonOnly ) { - # Block is marked anon-only - # Whitelist this IP address against autoblocks and range blocks - # (but not account creation blocks -- bug 13611) - if ( !$this->mCreateAccount ) { - $this->clear(); - } + $res = $db->select( 'ipblocks', '*', $conds, __METHOD__ ); - return false; - } else { - return true; - } - } - } + # This result could contain a block on the user, a block on the IP, and a russian-doll + # set of rangeblocks. We want to choose the most specific one, so keep a leader board. + $bestRow = null; - # Try range block - if ( $this->loadRange( $address, $killExpired, $user ) ) { - if ( $user && $this->mAnonOnly ) { - # Respect account creation blocks on logged-in users -- bug 13611 - if ( !$this->mCreateAccount ) { - $this->clear(); - } + # Lower will be better + $bestBlockScore = 100; - return false; - } else { - return true; - } - } + # This is begging for $this = $bestBlock, but that's not allowed in PHP :( + $bestBlockPreventsEdit = null; - # Try autoblock - if ( $address ) { - $conds = array( 'ipb_address' => $address, 'ipb_auto' => 1 ); + foreach( $res as $row ){ + $block = Block::newFromRow( $row ); - if ( $user ) { - $conds['ipb_anon_only'] = 0; + # Don't use expired blocks + if( $block->deleteIfExpired() ){ + continue; } - $res = $db->resultObject( $db->select( - 'ipblocks', - '*', - $conds, - __METHOD__ - ) ); + # Don't use anon only blocks on users + if( $this->type == self::TYPE_USER && !$block->isHardblock() ){ + continue; + } - if ( $this->loadFromResult( $res, $killExpired ) ) { - return true; + if( $block->getType() == self::TYPE_RANGE ){ + # This is the number of bits that are allowed to vary in the block, give + # or take some floating point errors + $end = wfBaseconvert( $block->getRangeEnd(), 16, 10 ); + $start = wfBaseconvert( $block->getRangeStart(), 16, 10 ); + $size = log( $end - $start + 1, 2 ); + + # This has the nice property that a /32 block is ranked equally with a + # single-IP block, which is exactly what it is... + $score = self::TYPE_RANGE - 1 + ( $size / 128 ); + + } else { + $score = $block->getType(); + } + + if( $score < $bestBlockScore ){ + $bestBlockScore = $score; + $bestRow = $row; + $bestBlockPreventsEdit = $block->prevents( 'edit' ); } } - # Give up - $this->clear(); - return false; + if( $bestRow !== null ){ + $this->initFromRow( $bestRow ); + $this->prevents( 'edit', $bestBlockPreventsEdit ); + return true; + } else { + return false; + } } /** @@ -367,11 +419,13 @@ class Block { * @param $row ResultWrapper: a row from the ipblocks table */ protected function initFromRow( $row ) { - $this->mAddress = $row->ipb_address; + list( $this->target, $this->type ) = self::parseTarget( $row->ipb_address ); + + $this->setTarget( $row->ipb_address ); + $this->setBlocker( User::newFromId( $row->ipb_by ) ); + $this->mReason = $row->ipb_reason; $this->mTimestamp = wfTimestamp( TS_MW, $row->ipb_timestamp ); - $this->mUser = $row->ipb_user; - $this->mBy = $row->ipb_by; $this->mAuto = $row->ipb_auto; $this->mAnonOnly = $row->ipb_anon_only; $this->mCreateAccount = $row->ipb_create_account; @@ -381,17 +435,21 @@ class Block { $this->mHideName = $row->ipb_deleted; $this->mId = $row->ipb_id; $this->mExpiry = $row->ipb_expiry; - - if ( isset( $row->user_name ) ) { - $this->mByName = $row->user_name; - } else { - $this->mByName = $row->ipb_by_text; - } - $this->mRangeStart = $row->ipb_range_start; $this->mRangeEnd = $row->ipb_range_end; } + /** + * Create a new Block object from a database row + * @param $row ResultWrapper row from the ipblocks table + * @return Block + */ + public static function newFromRow( $row ){ + $block = new Block; + $block->initFromRow( $row ); + return $block; + } + /** * Once $mAddress has been set, get the range they came from. * Wrapper for IP::parseRange @@ -812,6 +870,14 @@ class Block { return $this->mByName; } + /** + * Get the block ID + * @return int + */ + public function getId() { + return $this->mId; + } + /** * Get/set the SELECT ... FOR UPDATE flag * @deprecated since 1.18 @@ -840,6 +906,10 @@ class Block { return !$y; } + public function isAutoblocking( $x = null ) { + return wfSetVar( $this->mEnableAutoblock, $x ); + } + /** * Get/set whether the Block prevents a given action * @param $action String @@ -849,7 +919,7 @@ class Block { public function prevents( $action, $x = null ) { switch( $action ) { case 'edit': - # TODO Not actually quite this simple (bug 13611 etc) + # For now... return true; case 'createaccount': @@ -1000,46 +1070,47 @@ class Block { /** * Given a target and the target's type, get an existing Block object if possible. - * Note that passing an IP address will get an applicable rangeblock if the IP is - * not individually blocked but falls within that range - * TODO: check that that fallback handles nested rangeblocks nicely (should return - * smallest one) - * @param $target String|User|Int a block target, which may be one of several types: + * @param $specificTarget String|User|Int a block target, which may be one of several types: * * A user to block, in which case $target will be a User * * An IP to block, in which case $target will be a User generated by using * User::newFromName( $ip, false ) to turn off name validation * * An IP range, in which case $target will be a String "123.123.123.123/18" etc - * * The ID of an existing block, in which case $target will be an Int - * @param $type Block::TYPE_ constant the type of block as described above - * @return Block|null (null if the target is not blocked) + * * The ID of an existing block, in the format "#12345" (since pure numbers are valid + * usernames + * Calling this with a user, IP address or range will not select autoblocks, and will + * only select a block where the targets match exactly (so looking for blocks on + * 1.2.3.4 will not select 1.2.0.0/16 or even 1.2.3.4/32) + * @param $vagueTarget String|User|Int as above, but we will search for *any* block which + * affects that target (so for an IP address, get ranges containing that IP; and also + * get any relevant autoblocks) + * @param $fromMaster Bool whether to use the DB_MASTER database + * @return Block|null (null if no relevant block could be found). The target and type + * of the returned Block will refer to the actual block which was found, which might + * not be the same as the target you gave if you used $vagueTarget! */ - public static function newFromTargetAndType( $target, $type ) { - if ( $target instanceof User ) { - if ( $type == Block::TYPE_IP ) { - return Block::newFromDB( $target->getName(), 0 ); - } elseif ( $type == Block::TYPE_USER ) { - return Block::newFromDB( '', $target->getId() ); - } else { - # Should be unreachable; - return null; - } + public static function newFromTarget( $specificTarget, $vagueTarget = null, $fromMaster = false ) { + list( $target, $type ) = self::parseTarget( $specificTarget ); + if( $type == Block::TYPE_ID || $type == Block::TYPE_AUTO ){ + return Block::newFromID( $target ); - } elseif ( $type == Block::TYPE_RANGE ) { - return Block::newFromDB( $target, 0 ); + } elseif( in_array( $type, array( Block::TYPE_USER, Block::TYPE_IP, Block::TYPE_RANGE, null ) ) ) { + $block = new Block(); + $block->fromMaster( $fromMaster ); - } elseif ( $type == Block::TYPE_ID || $type == Block::TYPE_AUTO ) { - return Block::newFromID( $target ); + if( $type !== null ){ + $block->setTarget( $target ); + } + if( $block->newLoad( $vagueTarget ) ){ + return $block; + } else { + return null; + } } else { return null; } } - public static function newFromTarget( $target ) { - list( $target, $type ) = self::parseTarget( $target ); - return self::newFromTargetAndType( $target, $type ); - } - /** * From an existing Block, get the target and the type of target. Note that it is * always safe to treat the target as a string; for User objects this will return @@ -1049,6 +1120,17 @@ class Block { public static function parseTarget( $target ) { $target = trim( $target ); + # We may have been through this before + if( $target instanceof User ){ + if( IP::isValid( $target->getName() ) ){ + return self::TYPE_IP; + } else { + return self::TYPE_USER; + } + } elseif( $target === null ){ + return array( null, null ); + } + $userObj = User::newFromName( $target ); if ( $userObj instanceof User ) { # Note that since numbers are valid usernames, a $target of "12345" will be @@ -1079,30 +1161,61 @@ class Block { } /** - * Get the target and target type for this particular Block. Note that for autoblocks, + * Get the type of target for this particular block + * @return Block::TYPE_ constant, will never be TYPE_ID + */ + public function getType() { + return $this->mAuto + ? self::TYPE_AUTO + : $this->type; + } + + /** + * Get the target for this particular Block. Note that for autoblocks, * this returns the unredacted name; frontend functions need to call $block->getRedactedName() * in this situation. - * @return array( User|String, Block::TYPE_ constant ) - * FIXME: this should be an integral part of the Block member variables + * @return User|String */ - public function getTargetAndType() { - list( $target, $type ) = self::parseTarget( $this->mAddress ); + public function getTarget() { + return $this->target; + } - # Check whether it's an autoblock - if ( $this->mAuto ) { - $type = self::TYPE_AUTO; + /** + * Set the target for this block, and update $this->type accordingly + * @param $target Mixed + */ + public function setTarget( $target ){ + list( $this->target, $this->type ) = self::parseTarget( $target ); + + $this->mAddress = (string)$this->target; + if( $this->type == self::TYPE_USER ){ + if( $this->target instanceof User ){ + # Cheat + $this->mUser = $this->target->getID(); + } else { + $this->mUser = User::idFromName( $this->target ); + } + } else { + $this->mUser = 0; } - - return array( $target, $type ); } - public function getType() { - list( /*...*/, $type ) = $this->getTargetAndType(); - return $type; + /** + * Get the user who implemented this block + * @return User + */ + public function getBlocker(){ + return $this->blocker; } - public function getTarget() { - list( $target, /*...*/ ) = $this->getTargetAndType(); - return $target; + /** + * Set the user who implemented (or will implement) this block + * @param $user User + */ + public function setBlocker( User $user ){ + $this->blocker = $user; + + $this->mBy = $user->getID(); + $this->mByName = $user->getName(); } } diff --git a/includes/OutputPage.php b/includes/OutputPage.php index f57695be1b..60533c5285 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -1910,7 +1910,7 @@ class OutputPage { $link = '[[' . $wgContLang->getNsText( NS_USER ) . ":{$name}|{$name}]]"; - $blockid = $wgUser->mBlock->mId; + $blockid = $wgUser->mBlock->getId(); $blockExpiry = $wgLang->formatExpiry( $wgUser->mBlock->mExpiry ); @@ -1922,7 +1922,7 @@ class OutputPage { /* $ip returns who *is* being blocked, $intended contains who was meant to be blocked. * This could be a username, an IP range, or a single IP. */ - $intended = $wgUser->mBlock->mAddress; + $intended = $wgUser->mBlock->getTarget(); $this->addWikiMsg( $msg, $link, $reason, $ip, $name, $blockid, $blockExpiry, diff --git a/includes/Title.php b/includes/Title.php index 601e54cd51..15bd8f4ebf 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -1562,7 +1562,7 @@ class Title { } $link = '[[' . $wgContLang->getNsText( NS_USER ) . ":{$name}|{$name}]]"; - $blockid = $block->mId; + $blockid = $block->getId(); $blockExpiry = $user->mBlock->mExpiry; $blockTimestamp = $wgLang->timeanddate( wfTimestamp( TS_MW, $user->mBlock->mTimestamp ), true ); if ( $blockExpiry == 'infinity' ) { @@ -1571,7 +1571,7 @@ class Title { $blockExpiry = $wgLang->timeanddate( wfTimestamp( TS_MW, $blockExpiry ), true ); } - $intended = $user->mBlock->mAddress; + $intended = $user->mBlock->getTarget(); $errors[] = array( ( $block->mAuto ? 'autoblockedtext' : 'blockedtext' ), $link, $reason, $ip, $name, $blockid, $blockExpiry, $intended, $blockTimestamp ); diff --git a/includes/User.php b/includes/User.php index 2b0a5997a2..8cf872d7ff 100644 --- a/includes/User.php +++ b/includes/User.php @@ -1129,42 +1129,26 @@ class User { $this->mHideName = 0; $this->mAllowUsertalk = 0; - # Check if we are looking at an IP or a logged-in user - if ( $this->isAllowed( 'ipblock-exempt' ) ) { - # Exempt from all types of IP-block - $ip = ''; - } elseif ( $this->isIP( $this->getName() ) ) { - $ip = $this->getName(); + # We only need to worry about passing the IP address to the Block generator if the + # user is not immune to autoblocks/hardblocks, and they are the current user so we + # know which IP address they're actually coming from + if ( !$this->isAllowed( 'ipblock-exempt' ) && $this->getID() == $wgUser->getID() ) { + $ip = wfGetIP(); } else { - # Check if we are looking at the current user - # If we don't, and the user is logged in, we don't know about - # his IP / autoblock status, so ignore autoblock of current user's IP - if ( $this->getID() != $wgUser->getID() ) { - $ip = ''; - } else { - # Get IP of current user - $ip = wfGetIP(); - } + $ip = null; } # User/IP blocking - $this->mBlock = new Block(); - $this->mBlock->fromMaster( !$bFromSlave ); - if ( $this->mBlock->load( $ip , $this->mId ) ) { + $this->mBlock = Block::newFromTarget( $this->getName(), $ip, !$bFromSlave ); + if ( $this->mBlock instanceof Block ) { wfDebug( __METHOD__ . ": Found block.\n" ); - $this->mBlockedby = $this->mBlock->mBy; - if( $this->mBlockedby == 0 ) - $this->mBlockedby = $this->mBlock->mByName; + $this->mBlockedby = $this->mBlock->getBlocker()->getName(); $this->mBlockreason = $this->mBlock->mReason; $this->mHideName = $this->mBlock->mHideName; $this->mAllowUsertalk = !$this->mBlock->prevents( 'editownusertalk' ); if ( $this->isLoggedIn() && $wgUser->getID() == $this->getID() ) { $this->spreadBlock(); } - } else { - // Bug 13611: don't remove mBlock here, to allow account creation blocks to - // apply to users. Note that the existence of $this->mBlock is not used to - // check for edit blocks, $this->mBlockedby is instead. } # Proxy blocking @@ -1374,7 +1358,7 @@ class User { */ function isBlocked( $bFromSlave = true ) { // hacked from false due to horrible probs on site $this->getBlockedStatus( $bFromSlave ); - return $this->mBlockedby !== 0; + return $this->mBlock instanceof Block && $this->mBlock->prevents( 'edit' ); } /** @@ -1427,7 +1411,7 @@ class User { */ function getBlockId() { $this->getBlockedStatus(); - return ( $this->mBlock ? $this->mBlock->mId : false ); + return ( $this->mBlock ? $this->mBlock->getId() : false ); } /** @@ -2735,7 +2719,7 @@ class User { return; } - $userblock = Block::newFromDB( '', $this->mId ); + $userblock = Block::newFromTarget( $this->getName() ); if ( !$userblock ) { return; } @@ -2797,11 +2781,24 @@ class User { /** * Get whether the user is explicitly blocked from account creation. - * @return Bool + * @return Bool|Block */ function isBlockedFromCreateAccount() { $this->getBlockedStatus(); - return $this->mBlock && $this->mBlock->prevents( 'createaccount' ); + if( $this->mBlock && $this->mBlock->prevents( 'createaccount' ) ){ + return $this->mBlock; + } + + # bug 13611: if the IP address the user is trying to create an account from is + # blocked with createaccount disabled, prevent new account creation there even + # when the user is logged in + static $accBlock = false; + if( $accBlock === false ){ + $accBlock = Block::newFromTarget( null, wfGetIP() ); + } + return $accBlock instanceof Block && $accBlock->prevents( 'createaccount' ) + ? $accBlock + : false; } /** diff --git a/includes/api/ApiBlock.php b/includes/api/ApiBlock.php index 634367d21c..98d88acf94 100644 --- a/includes/api/ApiBlock.php +++ b/includes/api/ApiBlock.php @@ -98,11 +98,11 @@ class ApiBlock extends ApiBase { $this->dieUsageMsg( $retval ); } - list( $target, $type ) = SpecialBlock::getTargetAndType( $params['user'] ); + list( $target, /*...*/ ) = SpecialBlock::getTargetAndType( $params['user'] ); $res['user'] = $params['user']; $res['userID'] = $target instanceof User ? $target->getId() : 0; - $block = Block::newFromTargetAndType( $target, $type ); + $block = Block::newFromTarget( $target ); if( $block instanceof Block ){ $res['expiry'] = $block->mExpiry == wfGetDB( DB_SLAVE )->getInfinity() ? 'infinite' diff --git a/includes/api/ApiUnblock.php b/includes/api/ApiUnblock.php index 9a1bf336f4..d10ca8f07d 100644 --- a/includes/api/ApiUnblock.php +++ b/includes/api/ApiUnblock.php @@ -82,7 +82,7 @@ class ApiUnblock extends ApiBase { $this->dieUsageMsg( $retval[0] ); } - $res['id'] = $block->mId; + $res['id'] = $block->getId(); $res['user'] = $block->getType() == Block::TYPE_AUTO ? '' : $block->getTarget(); $res['reason'] = $params['reason']; $this->getResult()->addValue( null, $this->getModuleName(), $res ); diff --git a/includes/specials/SpecialBlock.php b/includes/specials/SpecialBlock.php index c26f0d40b1..36a050f12f 100644 --- a/includes/specials/SpecialBlock.php +++ b/includes/specials/SpecialBlock.php @@ -201,16 +201,16 @@ class SpecialBlock extends SpecialPage { protected function maybeAlterFormDefaults( &$fields ){ $fields['Target']['default'] = (string)$this->target; - $block = Block::newFromTargetAndType( $this->target, $this->type ); + $block = Block::newFromTarget( $this->target ); if( $block instanceof Block && !$block->mAuto # The block exists and isn't an autoblock && ( $this->type != Block::TYPE_RANGE # The block isn't a rangeblock - || $block->mAddress == $this->target ) # or if it is, the range is what we're about to block + || $block->getTarget() == $this->target ) # or if it is, the range is what we're about to block ) { $fields['HardBlock']['default'] = $block->isHardblock(); $fields['CreateAccount']['default'] = $block->prevents( 'createaccount' ); - $fields['AutoBlock']['default'] = $block->mEnableAutoblock; + $fields['AutoBlock']['default'] = $block->isAutoblocking(); if( isset( $fields['DisableEmail'] ) ){ $fields['DisableEmail']['default'] = $block->prevents( 'sendemail' ); } @@ -531,24 +531,18 @@ class SpecialBlock extends SpecialPage { } } - # Create block object. Note that for a user block, ipb_address is only for display purposes - # FIXME: Why do we need to pass fourteen optional parameters to do this!?! - $block = new Block( - $target, # IP address or User name - $userId, # User id - $wgUser->getId(), # Blocker id - $data['Reason'][0], # Reason string - wfTimestampNow(), # Block Timestamp - 0, # Is this an autoblock (no) - self::parseExpiryInput( $data['Expiry'] ), # Expiry time - !$data['HardBlock'], # Block anon only - $data['CreateAccount'], - $data['AutoBlock'], - $data['HideUser'] - ); - + # Create block object. + $block = new Block(); + $block->setTarget( $target ); + $block->setBlocker( $wgUser ); + $block->mReason = $data['Reason'][0]; + $block->mExpiry = self::parseExpiryInput( $data['Expiry'] ); + $block->prevents( 'createaccount', $data['CreateAccount'] ); $block->prevents( 'editownusertalk', ( !$wgBlockAllowsUTEdit || $data['DisableUTEdit'] ) ); $block->prevents( 'sendemail', $data['DisableEmail'] ); + $block->isHardblock( $data['HardBlock'] ); + $block->isAutoblocking( $data['AutoBlock'] ); + $block->mHideName = $data['HideUser']; if( !wfRunHooks( 'BlockIp', array( &$block, &$wgUser ) ) ) { return array( 'hookaborted' ); @@ -566,7 +560,7 @@ class SpecialBlock extends SpecialPage { # This returns direct blocks before autoblocks/rangeblocks, since we should # be sure the user is blocked by now it should work for our purposes - $currentBlock = Block::newFromTargetAndType( $target, $type ); + $currentBlock = Block::newFromTarget( $target ); if( $block->equals( $currentBlock ) ) { return array( 'ipb_already_blocked' ); @@ -611,7 +605,7 @@ class SpecialBlock extends SpecialPage { # Block constructor sanitizes certain block options on insert $data['BlockEmail'] = $block->prevents( 'sendemail' ); - $data['AutoBlock'] = $block->mEnableAutoblock; + $data['AutoBlock'] = $block->isAutoblocking(); # Prepare log parameters $logParams = array(); @@ -641,8 +635,8 @@ class SpecialBlock extends SpecialPage { public static function getSuggestedDurations( $lang = null ){ $a = array(); $msg = $lang === null - ? wfMessage( 'ipboptions' )->inContentLanguage() - : wfMessage( 'ipboptions' )->inLanguage( $lang ); + ? wfMessage( 'ipboptions' )->inContentLanguage()->text() + : wfMessage( 'ipboptions' )->inLanguage( $lang )->text(); if( $msg == '-' ){ return array(); diff --git a/includes/specials/SpecialBlockme.php b/includes/specials/SpecialBlockme.php index f5131f5f24..407476679d 100644 --- a/includes/specials/SpecialBlockme.php +++ b/includes/specials/SpecialBlockme.php @@ -48,10 +48,12 @@ class SpecialBlockme extends UnlistedSpecialPage { if ( !$user->isLoggedIn() ) { $user->addToDatabase(); } - $id = $user->getId(); - $reason = wfMsg( 'proxyblockreason' ); - $block = new Block( $ip, 0, $id, $reason, wfTimestampNow() ); + $block = new Block(); + $block->setTarget( $ip ); + $block->setBlocker( $user ); + $block->mReason = wfMsg( 'proxyblockreason' ); + $block->insert(); $wgOut->addWikiMsg( 'proxyblocksuccess' ); diff --git a/includes/specials/SpecialUnblock.php b/includes/specials/SpecialUnblock.php index 418cc5ee0f..3eba8b769e 100644 --- a/includes/specials/SpecialUnblock.php +++ b/includes/specials/SpecialUnblock.php @@ -49,7 +49,7 @@ class SpecialUnblock extends SpecialPage { } list( $this->target, $this->type ) = SpecialBlock::getTargetAndType( $par, $wgRequest ); - $this->block = Block::newFromTargetAndType( $this->target, $this->type ); + $this->block = Block::newFromTarget( $this->target ); # bug 15810: blocked admins should have limited access here. This won't allow sysops # to remove autoblocks on themselves, but they should have ipblock-exempt anyway @@ -168,7 +168,7 @@ class SpecialUnblock extends SpecialPage { # unblock the whole range. list( $target, $type ) = SpecialBlock::getTargetAndType( $target ); if( $block->getType() == Block::TYPE_RANGE && $type == Block::TYPE_IP ) { - $range = $block->mAddress; + $range = $block->getTarget(); return array( array( 'ipb_blocked_as_range', $target, $range ) ); } @@ -185,7 +185,12 @@ class SpecialUnblock extends SpecialPage { # Unset _deleted fields as needed if( $block->mHideName ) { - RevisionDeleteUser::unsuppressUserName( $block->mAddress, $block->mUser ); + # Something is deeply FUBAR if this is not a User object, but who knows? + $id = $block->getTarget() instanceof User + ? $block->getTarget()->getID() + : User::idFromName( $block->getTarget() ); + + RevisionDeleteUser::unsuppressUserName( $block->getTarget(), $id ); } # Make log entry diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php index 26edc63a41..323f7b64c2 100644 --- a/includes/specials/SpecialUserlogin.php +++ b/includes/specials/SpecialUserlogin.php @@ -304,7 +304,7 @@ class LoginForm extends SpecialPage { $wgOut->permissionRequired( 'createaccount' ); return false; } elseif ( $wgUser->isBlockedFromCreateAccount() ) { - $this->userBlockedMessage(); + $this->userBlockedMessage( $wgUser->isBlockedFromCreateAccount() ); return false; } @@ -918,9 +918,15 @@ class LoginForm extends SpecialPage { } } - /** */ - function userBlockedMessage() { - global $wgOut, $wgUser; + /** + * Output a message that informs the user that they cannot create an account because + * there is a block on them or their IP which prevents account creation. Note that + * User::isBlockedFromCreateAccount(), which gets this block, ignores the 'hardblock' + * setting on blocks (bug 13611). + * @param $block Block the block causing this error + */ + function userBlockedMessage( Block $block ) { + global $wgOut; # Let's be nice about this, it's likely that this feature will be used # for blocking large numbers of innocent people, e.g. range blocks on @@ -932,14 +938,18 @@ class LoginForm extends SpecialPage { $wgOut->setPageTitle( wfMsg( 'cantcreateaccounttitle' ) ); - $ip = wfGetIP(); - $blocker = User::whoIs( $wgUser->mBlock->mBy ); - $block_reason = $wgUser->mBlock->mReason; - + $block_reason = $block->mReason; if ( strval( $block_reason ) === '' ) { $block_reason = wfMsg( 'blockednoreason' ); } - $wgOut->addWikiMsg( 'cantcreateaccount-text', $ip, $block_reason, $blocker ); + + $wgOut->addWikiMsg( + 'cantcreateaccount-text', + $block->getTarget(), + $block_reason, + $block->getBlocker()->getName() + ); + $wgOut->returnToMain( false ); } @@ -963,7 +973,7 @@ class LoginForm extends SpecialPage { $wgOut->readOnlyPage(); return; } elseif ( $wgUser->isBlockedFromCreateAccount() ) { - $this->userBlockedMessage(); + $this->userBlockedMessage( $wgUser->isBlockedFromCreateAccount() ); return; } elseif ( count( $permErrors = $titleObj->getUserPermissionsErrors( 'createaccount', $wgUser, true ) )>0 ) { $wgOut->showPermissionsErrorPage( $permErrors, 'createaccount' ); diff --git a/tests/phpunit/includes/api/ApiBlockTest.php b/tests/phpunit/includes/api/ApiBlockTest.php index 5c8eee3d10..60b80c73c8 100644 --- a/tests/phpunit/includes/api/ApiBlockTest.php +++ b/tests/phpunit/includes/api/ApiBlockTest.php @@ -53,12 +53,12 @@ class ApiBlockTest extends ApiTestSetup { 'user' => 'UTBlockee', 'reason' => 'Some reason', 'token' => $pageinfo['blocktoken'] ), $data ); - - $block = Block::newFromDB('UTBlockee'); + + $block = Block::newFromTarget('UTBlockee'); $this->assertTrue( !is_null( $block ), 'Block is valid' ); - $this->assertEquals( 'UTBlockee', $block->mAddress ); + $this->assertEquals( 'UTBlockee', (string)$block->getTarget() ); $this->assertEquals( 'Some reason', $block->mReason ); $this->assertEquals( 'infinity', $block->mExpiry ); -- 2.20.1