http://www.mediawiki.org/wiki/Special:Code/MediaWiki/88750
Revision: 88750
Author: brion
Date: 2011-05-24 21:04:50 +0000 (Tue, 24 May 2011)
Log Message:
-----------
* (bug 29116) Fix regression breaking CheckUser extension
Fixes regression from r84475 and friends which made Block->load() and its new
front-end Block::newFromTarget() fail when an empty string was passed in as the
IP / $vagueTarget parameter to indicate skipping IP-based lookups.
Added phpunit test cases to confirm that both Block->load() and
Block::newFromTarget() work when given null (already ok), '' (as done from
CheckUser), or false (not seen, but perfectly legit sounding).
Adjusted comparisons to work as expected.
Modified Paths:
--------------
trunk/phase3/includes/Block.php
trunk/phase3/tests/phpunit/includes/BlockTest.php
Modified: trunk/phase3/includes/Block.php
===================================================================
--- trunk/phase3/includes/Block.php 2011-05-24 20:59:44 UTC (rev 88749)
+++ trunk/phase3/includes/Block.php 2011-05-24 21:04:50 UTC (rev 88750)
@@ -184,7 +184,7 @@
* 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
+ * make any sense to use TYPE_AUTO / TYPE_ID here. Leave blank to
skip IP lookups.
* @return Bool whether a relevant block was found
*/
protected function newLoad( $vagueTarget = null ) {
@@ -198,7 +198,8 @@
$conds = array( 'ipb_address' => array() );
}
- if( $vagueTarget !== null ){
+ # Be aware that the != '' check is explicit, since empty values
will be passed by some callers.
+ if( $vagueTarget != ''){
list( $target, $type ) = self::parseTarget(
$vagueTarget );
switch( $type ) {
case self::TYPE_USER:
@@ -962,7 +963,7 @@
* 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)
+ * get any relevant autoblocks). Leave empty or blank to skip
IP-based lookups.
* @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
@@ -979,8 +980,9 @@
if( $type == Block::TYPE_ID || $type == Block::TYPE_AUTO ){
return Block::newFromID( $target );
- } elseif( $target === null && $vagueTarget === null ){
+ } elseif( $target === null && $vagueTarget == '' ){
# We're not going to find anything useful here
+ # Be aware that the == '' check is explicit, since
empty values will be passed by some callers.
return null;
} elseif( in_array( $type, array( Block::TYPE_USER,
Block::TYPE_IP, Block::TYPE_RANGE, null ) ) ) {
Modified: trunk/phase3/tests/phpunit/includes/BlockTest.php
===================================================================
--- trunk/phase3/tests/phpunit/includes/BlockTest.php 2011-05-24 20:59:44 UTC
(rev 88749)
+++ trunk/phase3/tests/phpunit/includes/BlockTest.php 2011-05-24 21:04:50 UTC
(rev 88750)
@@ -48,5 +48,38 @@
}
+ /**
+ * This is the method previously used to load block info in CheckUser
etc
+ * passing an empty value (empty string, null, etc) as the ip parameter
bypasses IP lookup checks.
+ *
+ * This stopped working with r84475 and friends: regression being fixed
for bug 29116.
+ *
+ * @dataProvider dataBug29116
+ */
+ function testBug29116LoadWithEmptyIp( $vagueTarget ) {
+ $block = new Block();
+ $block->load( $vagueTarget, 'UTBlockee' );
+ $this->assertTrue( $this->block->equals(
Block::newFromTarget('UTBlockee', $vagueTarget) ), "Block->load() returns the
same block as the one that was made when given empty ip param " . var_export(
$vagueTarget, true ) );
+ }
+
+ /**
+ * CheckUser since being changed to use Block::newFromTarget started
failing
+ * because the new function didn't accept empty strings like
Block::load()
+ * had. Regression bug 29116.
+ *
+ * @dataProvider dataBug29116
+ */
+ function testBug29116NewFromTargetWithEmptyIp( $vagueTarget ) {
+ $block = Block::newFromTarget('UTBlockee', $vagueTarget);
+ $this->assertTrue( $this->block->equals( $block ),
"newFromTarget() returns the same block as the one that was made when given
empty vagueTarget param " . var_export( $vagueTarget, true ) );
+ }
+
+ function dataBug29116() {
+ return array(
+ array( null ),
+ array( '' ),
+ array( false )
+ );
+ }
}
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs
