UserAuthority: Fix wikitext escaping for block errors

Status and Message behave differently when given a list of message
parameters. Since the errors generated by PermissionManager is intended
to be used by Message, we need to update UserAuthority to do that.

Fixed StatusValue::replaceMessage to support MessageSpecifier objects
like StatusValue::hasMessage does. This is so that the block message
formatting in ApiBase does not break.

Bug: T306494
Change-Id: I26405c680839b4679709e750d6d54e699c1efa66
This commit is contained in:
Taavi Väänänen 2022-05-04 09:36:34 +03:00
parent 3abca03ec0
commit 6128d841c3
No known key found for this signature in database
GPG key ID: EF242F709F912FBE
4 changed files with 64 additions and 2 deletions

View file

@ -264,7 +264,7 @@ class UserAuthority implements Authority {
$blockError = false;
foreach ( $errors as $err ) {
$status->fatal( ...$err );
$status->fatal( wfMessage( ...$err ) );
// HACK: Detect whether the permission was denied because the user is blocked.
// A similar hack exists in ApiBase::$blockMsgMap.

View file

@ -351,6 +351,10 @@ class StatusValue {
if ( $error['message'] === $source ) {
$this->errors[$index]['message'] = $dest;
$replaced = true;
} elseif ( $error['message'] instanceof MessageSpecifier
&& $error['message']->getKey() === $source ) {
$this->errors[$index]['message'] = $dest;
$replaced = true;
}
}

View file

@ -537,6 +537,20 @@ class StatusTest extends MediaWikiLangTestCase {
$this->assertEquals( $newMessage, $status->errors[0]['message'] );
}
/**
* @covers Status::replaceMessage
*/
public function testReplaceMessageByKey() {
$status = new Status();
$status->error( new Message( 'key1', [ 'foo1', 'bar1' ] ) );
$newMessage = new Message( 'key2', [ 'foo2', 'bar2' ] );
$status->replaceMessage( 'key1', $newMessage );
$this->assertEquals( $newMessage, $status->errors[0]['message'] );
}
/**
* @covers Status::getErrorMessage
*/

View file

@ -30,6 +30,7 @@ use MediaWiki\Permissions\PermissionStatus;
use MediaWiki\Permissions\UserAuthority;
use MediaWikiUnitTestCase;
use PHPUnit\Framework\MockObject\MockObject;
use Status;
use User;
/**
@ -37,6 +38,14 @@ use User;
*/
class UserAuthorityTest extends MediaWikiUnitTestCase {
/** @var string[] Some dummy message parameters to test error message formatting. */
private const FAKE_BLOCK_MESSAGE_PARAMS = [
'[[User:Blocker|Blocker]]',
'Block reason that can contain {{templates}}',
'192.168.0.1',
'Blocker',
];
/**
* @param string[] $permissions
* @return PermissionManager
@ -68,7 +77,10 @@ class UserAuthorityTest extends MediaWikiUnitTestCase {
}
if ( $user->getBlock() && $permission !== 'read' ) {
$errors[] = [ 'blockedtext-partial' ];
$errors[] = array_merge(
[ 'blockedtext-partial' ],
self::FAKE_BLOCK_MESSAGE_PARAMS
);
}
return $errors;
@ -268,4 +280,36 @@ class UserAuthorityTest extends MediaWikiUnitTestCase {
$this->assertSame( $block, $authority->getBlock() );
}
/**
* Regression test for T306494: check that when creating a PermissionStatus,
* the message contains all parameters and when converted to a Status, the parameters
* are not wikitext escaped.
*/
public function testInternalCanWithPermissionStatusMessageFormatting() {
$block = $this->createNoOpMock( Block::class );
$user = $this->getBlockedUser( $block );
$authority = $this->newAuthority( [ 'read', 'edit' ], $user );
$permissionStatus = PermissionStatus::newEmpty();
$target = new PageIdentityValue( 321, NS_MAIN, __METHOD__, PageIdentity::LOCAL );
$authority->authorizeWrite(
'edit',
$target,
$permissionStatus
);
$this->assertTrue( $permissionStatus->hasMessage( 'blockedtext-partial' ) );
$status = Status::wrap( $permissionStatus );
$message = $status->getMessage();
$this->assertEquals( 'blockedtext-partial', $message->getKey() );
$this->assertArrayEquals(
self::FAKE_BLOCK_MESSAGE_PARAMS,
$message->getParams()
);
}
}