Remove is_numeric check from Title::checkUserBlock

This should allow the usernames of administrators such as "7"
to show correctly on permissions error pages.

I extracted the working code from UserBlockedError::__construct
into a separate method Block::getPermissionsError, called from
both places with context provided as an argument.

Additional changes to get the test suite to pass are included.

Bug: 46768
Change-Id: I49d973992a99e03b4e8de112b47b737037a85338
This commit is contained in:
Kevin Israel 2013-04-03 17:44:00 -04:00 committed by Gerrit Code Review
parent 12f5ab9a3f
commit 47d1060398
7 changed files with 53 additions and 69 deletions

View file

@ -55,6 +55,7 @@ production.
* Pager's properly validate which fields are allowed to be sorted on.
* mw.util.tooltipAccessKeyRegexp: The regex now matches "option-" as well.
Support for Mac "option" was added in 1.16, but the regex was never updated.
* (bug 46768) Usernames of blocking users now display correctly, even if numeric.
=== API changes in 1.22 ===
* (bug 46626) xmldoublequote parameter was removed. Because of a bug, the

View file

@ -1393,4 +1393,43 @@ class Block {
public function setBlocker( $user ) {
$this->blocker = $user;
}
/**
* Get the key and parameters for the corresponding error message.
*
* @since 1.22
* @param IContextSource $context
* @return array
*/
public function getPermissionsError( IContextSource $context ) {
$blocker = $this->getBlocker();
if ( $blocker instanceof User ) { // local user
$blockerUserpage = $blocker->getUserPage();
$link = "[[{$blockerUserpage->getPrefixedText()}|{$blockerUserpage->getText()}]]";
} else { // foreign user
$link = $blocker;
}
$reason = $this->mReason;
if ( $reason == '' ) {
$reason = $context->msg( 'blockednoreason' )->text();
}
/* $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 = $this->getTarget();
$lang = $context->getLanguage();
return array(
$this->mAuto ? 'autoblockedtext' : 'blockedtext',
$link,
$reason,
$context->getRequest()->getIP(),
$this->getByName(),
$this->getId(),
$lang->formatExpiry( $this->mExpiry ),
(string)$intended,
$lang->timeanddate( wfTimestamp( TS_MW, $this->mTimestamp ), true ),
);
}
}

View file

@ -467,39 +467,9 @@ class ThrottledError extends ErrorPageError {
*/
class UserBlockedError extends ErrorPageError {
public function __construct( Block $block ) {
global $wgLang, $wgRequest;
$blocker = $block->getBlocker();
if ( $blocker instanceof User ) { // local user
$blockerUserpage = $block->getBlocker()->getUserPage();
$link = "[[{$blockerUserpage->getPrefixedText()}|{$blockerUserpage->getText()}]]";
} else { // foreign user
$link = $blocker;
}
$reason = $block->mReason;
if ( $reason == '' ) {
$reason = wfMessage( 'blockednoreason' )->text();
}
/* $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 = $block->getTarget();
parent::__construct(
'blockedtitle',
$block->mAuto ? 'autoblockedtext' : 'blockedtext',
array(
$link,
$reason,
$wgRequest->getIP(),
$block->getByName(),
$block->getId(),
$wgLang->formatExpiry( $block->mExpiry ),
$intended,
$wgLang->timeanddate( wfTimestamp( TS_MW, $block->mTimestamp ), true )
)
);
// @todo FIXME: Implement a more proper way to get context here.
$params = $block->getPermissionsError( RequestContext::getMain() );
parent::__construct( 'blockedtitle', array_shift( $params ), $params );
}
}

View file

@ -2056,38 +2056,8 @@ class Title {
// Don't block the user from editing their own talk page unless they've been
// explicitly blocked from that too.
} elseif ( $user->isBlocked() && $user->mBlock->prevents( $action ) !== false ) {
$block = $user->getBlock();
// This is from OutputPage::blockedPage
// Copied at r23888 by werdna
$id = $user->blockedBy();
$reason = $user->blockedFor();
if ( $reason == '' ) {
$reason = wfMessage( 'blockednoreason' )->text();
}
$ip = $user->getRequest()->getIP();
if ( is_numeric( $id ) ) {
$name = User::whoIs( $id );
} else {
$name = $id;
}
$link = '[[' . $wgContLang->getNsText( NS_USER ) . ":{$name}|{$name}]]";
$blockid = $block->getId();
$blockExpiry = $block->getExpiry();
$blockTimestamp = $wgLang->timeanddate( wfTimestamp( TS_MW, $block->mTimestamp ), true );
if ( $blockExpiry == 'infinity' ) {
$blockExpiry = wfMessage( 'infiniteblock' )->text();
} else {
$blockExpiry = $wgLang->timeanddate( wfTimestamp( TS_MW, $blockExpiry ), true );
}
$intended = strval( $block->getTarget() );
$errors[] = array( ( $block->mAuto ? 'autoblockedtext' : 'blockedtext' ), $link, $reason, $ip, $name,
$blockid, $blockExpiry, $intended, $blockTimestamp );
// @todo FIXME: Pass the relevant context into this function.
$errors[] = $user->getBlock()->getPermissionsError( RequestContext::getMain() );
}
return $errors;

View file

@ -4139,15 +4139,14 @@ class Language {
* @since 1.18
*/
public function formatExpiry( $expiry, $format = true ) {
static $infinity, $infinityMsg;
static $infinity;
if ( $infinity === null ) {
$infinityMsg = wfMessage( 'infiniteblock' );
$infinity = wfGetDB( DB_SLAVE )->getInfinity();
}
if ( $expiry == '' || $expiry == $infinity ) {
return $format === true
? $infinityMsg
? $this->getMessageFromDB( 'infiniteblock' )
: $infinity;
} else {
return $format === true

View file

@ -15,6 +15,10 @@ abstract class MediaWikiLangTestCase extends MediaWikiTestCase {
"\$wgContLang->getCode() (" . $wgContLang->getCode() . ")" );
}
// HACK: Call getLanguage() so the real $wgContLang is cached as the user language
// rather than our fake one. This is to avoid breaking other, unrelated tests.
RequestContext::getMain()->getLanguage();
$langCode = 'en'; # For mainpage to be 'Main Page'
$langObj = Language::factory( $langCode );

View file

@ -648,7 +648,8 @@ class TitlePermissionTest extends MediaWikiLangTestCase {
global $wgLocalTZoffset;
$wgLocalTZoffset = -60;
$this->user->mBlockedby = $this->user->getName();
$this->user->mBlock = new Block( '127.0.8.1', 0, 1, 'no reason given', $now, 0, 10 );
$this->user->mBlock = new Block( '127.0.8.1', 0, $this->user->getId(),
'no reason given', $now, 0, 10 );
$this->assertEquals( array( array( 'blockedtext',
'[[User:Useruser|Useruser]]', 'no reason given', '127.0.0.1',
'Useruser', null, '23:00, 31 December 1969', '127.0.8.1',