From b8efb924f3f12a6b683d519bed3655ac6a7ddc8e Mon Sep 17 00:00:00 2001 From: Daimona Eaytoy Date: Thu, 7 Jan 2021 17:17:43 +0100 Subject: [PATCH] Fix a bunch of fatal errors seen in production Mostly uncaught exceptions, that appeared in places where the previous code was silently using DWIM-style booleans. Also a TypeError due to ViewDiff not using filter objects. Copy the fix from Ic8032592799756521a59ee23c0e76cb03a510b94 to another place as well. Bug: T271430 Bug: T271431 Bug: T271432 Bug: T271433 Change-Id: Ica4b82024c57482656cf6bca95bf37641c09cb9a --- includes/Special/SpecialAbuseLog.php | 47 ++++++++++++++++-------- includes/View/AbuseFilterViewDiff.php | 4 +- includes/View/AbuseFilterViewHistory.php | 22 +++++++---- 3 files changed, 49 insertions(+), 24 deletions(-) diff --git a/includes/Special/SpecialAbuseLog.php b/includes/Special/SpecialAbuseLog.php index 586252dee..e49a66840 100644 --- a/includes/Special/SpecialAbuseLog.php +++ b/includes/Special/SpecialAbuseLog.php @@ -13,7 +13,9 @@ use MediaWiki\Cache\LinkBatchFactory; use MediaWiki\Extension\AbuseFilter\AbuseFilter; use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager; use MediaWiki\Extension\AbuseFilter\AbuseFilterServices; +use MediaWiki\Extension\AbuseFilter\CentralDBNotAvailableException; use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry; +use MediaWiki\Extension\AbuseFilter\Filter\FilterNotFoundException; use MediaWiki\Extension\AbuseFilter\GlobalNameUtils; use MediaWiki\Extension\AbuseFilter\Pager\AbuseLogPager; use MediaWiki\Extension\AbuseFilter\SpecsFormatter; @@ -459,6 +461,28 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage { } } + // if a filter is hidden, users who can't view private filters should + // not be able to find log entries generated by it. + if ( !$this->afPermissionManager->canViewPrivateFiltersLogs( $user ) ) { + $searchedForPrivate = false; + foreach ( $filtersList as $index => $filterData ) { + try { + $filter = AbuseFilterServices::getFilterLookup()->getFilter( ...$filterData ); + } catch ( FilterNotFoundException $_ ) { + unset( $filtersList[$index] ); + $foundInvalid = true; + continue; + } + if ( $filter->isHidden() ) { + unset( $filtersList[$index] ); + $searchedForPrivate = true; + } + } + if ( $searchedForPrivate ) { + $out->addWikiMsg( 'abusefilter-log-private-not-included' ); + } + } + // @phan-suppress-next-line PhanImpossibleCondition if ( $foundInvalid ) { // @todo Tell what the invalid IDs are @@ -471,21 +495,6 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage { ); } - // if a filter is hidden, users who can't view private filters should - // not be able to find log entries generated by it. - if ( !$this->afPermissionManager->canViewPrivateFiltersLogs( $user ) ) { - $searchedForPrivate = false; - foreach ( $filtersList as $index => $filterData ) { - if ( AbuseFilterServices::getFilterLookup()->getFilter( ...$filterData )->isHidden() ) { - unset( $filtersList[$index] ); - $searchedForPrivate = true; - } - } - if ( $searchedForPrivate ) { - $out->addWikiMsg( 'abusefilter-log-private-not-included' ); - } - } - foreach ( $filtersList as $filterData ) { $searchFilters[] = GlobalNameUtils::buildGlobalName( ...$filterData ); } @@ -679,7 +688,13 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage { } if ( $global ) { - $filter_hidden = AbuseFilterServices::getFilterLookup()->getFilter( $filterID, $global )->isHidden(); + try { + $filter_hidden = AbuseFilterServices::getFilterLookup()->getFilter( $filterID, $global ) + ->isHidden(); + } catch ( CentralDBNotAvailableException $_ ) { + // Conservatively assume that it's hidden, like in formatRow + $filter_hidden = true; + } } else { $filter_hidden = $row->af_hidden; } diff --git a/includes/View/AbuseFilterViewDiff.php b/includes/View/AbuseFilterViewDiff.php index c85652050..b9c7b9cf7 100644 --- a/includes/View/AbuseFilterViewDiff.php +++ b/includes/View/AbuseFilterViewDiff.php @@ -286,6 +286,7 @@ class AbuseFilterViewDiff extends AbuseFilterView { /** * @param stdClass $row + * @fixme Should use Filter objects * @return (string|array)[] */ public function loadFromHistoryRow( $row ) { @@ -300,7 +301,8 @@ class AbuseFilterViewDiff extends AbuseFilterView { 'description' => $row->afh_public_comments, 'flags' => $row->afh_flags, 'notes' => $row->afh_comments, - 'group' => $row->afh_group, + // FIXME T263324 + 'group' => $row->afh_group ?? 'default', ], 'pattern' => $row->afh_pattern, 'actions' => unserialize( $row->afh_actions ), diff --git a/includes/View/AbuseFilterViewHistory.php b/includes/View/AbuseFilterViewHistory.php index d368ad00a..0ec1cc105 100644 --- a/includes/View/AbuseFilterViewHistory.php +++ b/includes/View/AbuseFilterViewHistory.php @@ -6,6 +6,7 @@ use HTMLForm; use IContextSource; use Linker; use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager; +use MediaWiki\Extension\AbuseFilter\Filter\FilterNotFoundException; use MediaWiki\Extension\AbuseFilter\FilterLookup; use MediaWiki\Extension\AbuseFilter\Pager\AbuseFilterHistoryPager; use MediaWiki\Linker\LinkRenderer; @@ -49,19 +50,26 @@ class AbuseFilterViewHistory extends AbuseFilterView { $out->enableOOUI(); $filter = $this->getRequest()->getIntOrNull( 'filter' ) ?: $this->filter; + if ( $filter ) { + try { + $filterObj = $this->filterLookup->getFilter( $filter, false ); + } catch ( FilterNotFoundException $_ ) { + $filter = null; + } + if ( isset( $filterObj ) && $filterObj->isHidden() + && !$this->afPermManager->canViewPrivateFilters( $this->getUser() ) + ) { + $out->addWikiMsg( 'abusefilter-history-error-hidden' ); + return; + } + } + if ( $filter ) { $out->setPageTitle( $this->msg( 'abusefilter-history' )->numParams( $filter ) ); } else { $out->setPageTitle( $this->msg( 'abusefilter-filter-log' ) ); } - if ( $filter && $this->filterLookup->getFilter( $filter, false )->isHidden() - && !$this->afPermManager->canViewPrivateFilters( $this->getUser() ) - ) { - $out->addWikiMsg( 'abusefilter-history-error-hidden' ); - return; - } - // Useful links $links = []; if ( $filter ) {