Blame hashar for this giant commit; he teased me for making so many smaller ones...
authorHappy-melon <happy-melon@users.mediawiki.org>
Mon, 21 Mar 2011 19:12:41 +0000 (19:12 +0000)
committerHappy-melon <happy-melon@users.mediawiki.org>
Mon, 21 Mar 2011 19:12:41 +0000 (19:12 +0000)
* 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

12 files changed:
includes/Article.php
includes/Block.php
includes/OutputPage.php
includes/Title.php
includes/User.php
includes/api/ApiBlock.php
includes/api/ApiUnblock.php
includes/specials/SpecialBlock.php
includes/specials/SpecialBlockme.php
includes/specials/SpecialUnblock.php
includes/specials/SpecialUserlogin.php
tests/phpunit/includes/api/ApiBlockTest.php

index 6d3618d..560a261 100644 (file)
@@ -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'
index aa2d98e..a082968 100644 (file)
  * 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... <evil laugh>
                                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();
        }
 }
index f57695b..60533c5 100644 (file)
@@ -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,
index 601e54c..15bd8f4 100644 (file)
@@ -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 );
index 2b0a599..8cf872d 100644 (file)
@@ -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;
        }
 
        /**
index 634367d..98d88ac 100644 (file)
@@ -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'
index 9a1bf33..d10ca8f 100644 (file)
@@ -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 );
index c26f0d4..36a050f 100644 (file)
@@ -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();
index f5131f5..4074766 100644 (file)
@@ -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' );
index 418cc5e..3eba8b7 100644 (file)
@@ -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
index 26edc63..323f7b6 100644 (file)
@@ -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' );
index 5c8eee3..60b80c7 100644 (file)
@@ -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 );