Fix error reporting in BlockedDomainStorage for real

This is a direct follow up for I6373fa6 where we apparently fixed
half of the cases while breaking the other half. There was actualy
a code path that can return null, and anther one that can return a
status object.

Since there is never anything done with the status object we can as
well get rid of it and always return null in case of an error.

Bug: T337431
Bug: T279275
Change-Id: I2ccb58756182897bcd6649c9f589e2f7a0321b20
This commit is contained in:
thiemowmde 2023-06-12 16:54:13 +02:00
parent afaf9d34f8
commit 518955f9c3
2 changed files with 17 additions and 15 deletions

View file

@ -202,12 +202,12 @@ class BlockedDomainStorage implements IDBAccessObject {
* @param string $domain domain to be blocked
* @param string $notes User provided notes
* @param \MediaWiki\Permissions\Authority|\MediaWiki\User\UserIdentity $user Performer
* @return RevisionRecord|StatusValue RevisionRecord on success, StatusValue on failure.
* @return RevisionRecord|null
*/
public function addDomain( string $domain, string $notes, $user ) {
public function addDomain( string $domain, string $notes, $user ): ?RevisionRecord {
$content = $this->loadConfigContent();
if ( $content instanceof StatusValue ) {
return $content;
if ( !$content ) {
return null;
}
$content[] = [ 'domain' => $domain, 'notes' => $notes ];
$comment = Message::newFromSpecifier( 'abusefilter-blocked-domains-domain-added-comment' )
@ -222,12 +222,12 @@ class BlockedDomainStorage implements IDBAccessObject {
* @param string $domain domain to be removed from the blocked list
* @param string $notes User provided notes
* @param \MediaWiki\Permissions\Authority|\MediaWiki\User\UserIdentity $user Performer
* @return RevisionRecord|StatusValue RevisionRecord on success, StatusValue on failure.
* @return RevisionRecord|null RevisionRecord on success, StatusValue on failure.
*/
public function removeDomain( string $domain, string $notes, $user ) {
public function removeDomain( string $domain, string $notes, $user ): ?RevisionRecord {
$content = $this->loadConfigContent();
if ( $content instanceof StatusValue ) {
return $content;
if ( !$content ) {
return null;
}
foreach ( $content as $key => $value ) {
if ( ( $value['domain'] ?? '' ) == $domain ) {
@ -240,7 +240,10 @@ class BlockedDomainStorage implements IDBAccessObject {
return $this->saveContent( $content, $user, $comment );
}
private function loadConfigContent() {
/**
* @return array|null
*/
private function loadConfigContent(): ?array {
$configPage = $this->getBlockedDomainPage();
$revision = $this->revisionLookup->getRevisionByTitle( $configPage, 0, self::READ_LATEST );
if ( !$revision ) {
@ -248,11 +251,11 @@ class BlockedDomainStorage implements IDBAccessObject {
} else {
$revContent = $revision->getContent( SlotRecord::MAIN );
if ( !$revContent instanceof JsonContent ) {
return StatusValue::newFatal( 'Invalid JSON' );
return null;
}
$status = FormatJson::parse( $revContent->getText(), FormatJson::FORCE_ASSOC );
if ( !$status->isOK() ) {
return StatusValue::newFatal( 'Invalid JSON' );
return null;
}
$content = $status->getValue();
}
@ -265,7 +268,7 @@ class BlockedDomainStorage implements IDBAccessObject {
* @param array $content To be turned into JSON
* @param \MediaWiki\Permissions\Authority|\MediaWiki\User\UserIdentity $user Performer
* @param string $comment Save comment
* @return RevisionRecord|StatusValue RevisionRecord on success, StatusValue on failure.
* @return RevisionRecord|null
*/
private function saveContent( $content, $user, $comment ) {
$configPage = $this->getBlockedDomainPage();

View file

@ -27,7 +27,6 @@ use MediaWiki\Extension\AbuseFilter\BlockedDomainStorage;
use MediaWiki\Utils\UrlUtils;
use PermissionsError;
use SpecialPage;
use StatusValue;
use WANObjectCache;
use Xml;
@ -243,7 +242,7 @@ class BlockedExternalDomains extends SpecialPage {
$this->getUser()
);
if ( $rev instanceof StatusValue ) {
if ( !$rev ) {
$out->wrapWikiTextAsInterface( 'error', 'Save failed' );
return false;
} else {
@ -340,7 +339,7 @@ class BlockedExternalDomains extends SpecialPage {
$this->getUser()
);
if ( $rev instanceof StatusValue ) {
if ( !$rev ) {
$out->wrapWikiTextAsInterface( 'error', 'Save failed' );
return false;
} else {