diff --git a/includes/AbuseFilter.php b/includes/AbuseFilter.php index 86d4f7c9d..63c8ca656 100644 --- a/includes/AbuseFilter.php +++ b/includes/AbuseFilter.php @@ -338,74 +338,6 @@ class AbuseFilter { return $generator->addGenericVars()->getVariableHolder(); } - /** - * @param string $filter - * @return true|array True when successful, otherwise a two-element array with exception message - * and character position of the syntax error - */ - public static function checkSyntax( $filter ) { - try { - $res = self::getDefaultParser()->checkSyntax( $filter ); - } catch ( AFPUserVisibleException $excep ) { - $res = [ $excep->getMessageObj()->text(), $excep->mPosition ]; - } - return $res; - } - - /** - * @param string $expr - * @return Status - */ - public static function evaluateExpression( $expr ) { - if ( self::checkSyntax( $expr ) !== true ) { - return Status::newFatal( 'abusefilter-tools-syntax-error' ); - } - - $vars = new AbuseFilterVariableHolder(); - // Generic vars are the only ones available - $generator = new VariableGenerator( $vars ); - $vars = $generator->addGenericVars()->getVariableHolder(); - $vars->setVar( 'timestamp', wfTimestamp( TS_UNIX ) ); - $parser = self::getDefaultParser( $vars ); - - return Status::newGood( $parser->evaluateExpression( $expr ) ); - } - - /** - * @param string $conds - * @param AbuseFilterParser $parser The parser instance to use. - * @param bool $ignoreError - * @param string|null $filter The ID of the filter being parsed - * @return bool - * @throws Exception - */ - public static function checkConditions( - $conds, AbuseFilterParser $parser, $ignoreError = true, $filter = null - ) { - try { - $result = $parser->parse( $conds ); - } catch ( Exception $excep ) { - $result = false; - - if ( $excep instanceof AFPUserVisibleException ) { - $msg = $excep->getMessageForLogs(); - $excep->setLocalizedMessage(); - } else { - $msg = $excep->getMessage(); - } - - $logger = LoggerFactory::getInstance( 'AbuseFilter' ); - $extraInfo = $filter !== null ? " for filter $filter" : ''; - $logger->warning( "AbuseFilter parser error$extraInfo: $msg" ); - - if ( !$ignoreError ) { - throw $excep; - } - } - - return $result; - } - /** * Returns an associative array of filters which were tripped * @@ -1197,7 +1129,7 @@ class AbuseFilter { $user = $context->getUser(); // Check the syntax - $syntaxerr = self::checkSyntax( $request->getVal( 'wpFilterRules' ) ); + $syntaxerr = self::getDefaultParser()->checkSyntax( $request->getVal( 'wpFilterRules' ) ); if ( $syntaxerr !== true ) { $validationStatus->error( 'abusefilter-edit-badsyntax', $syntaxerr[0] ); return $validationStatus; diff --git a/includes/AbuseFilterRunner.php b/includes/AbuseFilterRunner.php index 374865105..eca20baea 100644 --- a/includes/AbuseFilterRunner.php +++ b/includes/AbuseFilterRunner.php @@ -433,7 +433,7 @@ class AbuseFilterRunner { $pattern = trim( $row->af_pattern ); $this->parser->setFilter( $filterName ); - $result = AbuseFilter::checkConditions( $pattern, $this->parser, true, $filterName ); + $result = $this->parser->checkConditions( $pattern, true, $filterName ); $actualExtra = AFComputedVariable::$profilingExtraTime - $origExtraTime; $timeTaken = 1000 * ( microtime( true ) - $startTime - $actualExtra ); diff --git a/includes/Views/AbuseFilterViewTestBatch.php b/includes/Views/AbuseFilterViewTestBatch.php index ac8643e52..89ae088a8 100644 --- a/includes/Views/AbuseFilterViewTestBatch.php +++ b/includes/Views/AbuseFilterViewTestBatch.php @@ -157,8 +157,10 @@ class AbuseFilterViewTestBatch extends AbuseFilterView { public function doTest() { // Quick syntax check. $out = $this->getOutput(); - $result = AbuseFilter::checkSyntax( $this->testPattern ); - if ( $result !== true ) { + $parser = AbuseFilter::getDefaultParser(); + + $validSyntax = $parser->checkSyntax( $this->testPattern ); + if ( $validSyntax !== true ) { $out->addWikiMsg( 'abusefilter-test-syntaxerr' ); return; } @@ -215,6 +217,7 @@ class AbuseFilterViewTestBatch extends AbuseFilterView { $counter = 1; $contextUser = $this->getUser(); + $parser->toggleConditionLimit( false ); foreach ( $res as $row ) { $vars = new AbuseFilterVariableHolder(); $rc = RecentChange::newFromRow( $row ); @@ -225,9 +228,8 @@ class AbuseFilterViewTestBatch extends AbuseFilterView { continue; } - $parser = AbuseFilter::getDefaultParser( $vars ); - $parser->toggleConditionLimit( false ); - $result = AbuseFilter::checkConditions( $this->testPattern, $parser ); + $parser->setVariables( $vars ); + $result = $parser->checkConditions( $this->testPattern ); if ( $result || $this->mShowNegative ) { // Stash result in RC item diff --git a/includes/api/ApiAbuseFilterCheckMatch.php b/includes/api/ApiAbuseFilterCheckMatch.php index 3d39fd390..517ea8043 100644 --- a/includes/api/ApiAbuseFilterCheckMatch.php +++ b/includes/api/ApiAbuseFilterCheckMatch.php @@ -45,15 +45,19 @@ class ApiAbuseFilterCheckMatch extends ApiBase { $vars = AbuseFilter::loadVarDump( $row->afl_var_dump ); } + if ( $vars === null ) { + throw new LogicException( 'Impossible.' ); + } - if ( AbuseFilter::checkSyntax( $params[ 'filter' ] ) !== true ) { + $parser = AbuseFilter::getDefaultParser(); + if ( $parser->checkSyntax( $params[ 'filter' ] ) !== true ) { $this->dieWithError( 'apierror-abusefilter-badsyntax', 'badsyntax' ); } - $parser = AbuseFilter::getDefaultParser( $vars ); + $parser->setVariables( $vars ); $result = [ ApiResult::META_BC_BOOLS => [ 'result' ], - 'result' => AbuseFilter::checkConditions( $params['filter'], $parser ), + 'result' => $parser->checkConditions( $params['filter'] ), ]; $this->getResult()->addValue( diff --git a/includes/api/ApiAbuseFilterCheckSyntax.php b/includes/api/ApiAbuseFilterCheckSyntax.php index ef4699125..819906ac9 100644 --- a/includes/api/ApiAbuseFilterCheckSyntax.php +++ b/includes/api/ApiAbuseFilterCheckSyntax.php @@ -12,7 +12,7 @@ class ApiAbuseFilterCheckSyntax extends ApiBase { } $params = $this->extractRequestParams(); - $result = AbuseFilter::checkSyntax( $params[ 'filter' ] ); + $result = AbuseFilter::getDefaultParser()->checkSyntax( $params[ 'filter' ] ); $r = []; if ( $result === true ) { diff --git a/includes/api/ApiAbuseFilterEvalExpression.php b/includes/api/ApiAbuseFilterEvalExpression.php index c3eeecefd..5b7070123 100644 --- a/includes/api/ApiAbuseFilterEvalExpression.php +++ b/includes/api/ApiAbuseFilterEvalExpression.php @@ -1,5 +1,7 @@ extractRequestParams(); - $status = AbuseFilter::evaluateExpression( $params['expression'] ); + $status = $this->evaluateExpression( $params['expression'] ); if ( !$status->isGood() ) { $this->dieWithError( $status->getErrors()[0] ); } else { @@ -26,6 +28,26 @@ class ApiAbuseFilterEvalExpression extends ApiBase { } } + /** + * @param string $expr + * @return Status + */ + private function evaluateExpression( string $expr ) : Status { + $parser = AbuseFilter::getDefaultParser(); + if ( $parser->checkSyntax( $expr ) !== true ) { + return Status::newFatal( 'abusefilter-tools-syntax-error' ); + } + + $vars = new AbuseFilterVariableHolder(); + // Generic vars are the only ones available + $generator = new VariableGenerator( $vars ); + $vars = $generator->addGenericVars()->getVariableHolder(); + $vars->setVar( 'timestamp', wfTimestamp( TS_UNIX ) ); + $parser->setVariables( $vars ); + + return Status::newGood( $parser->evaluateExpression( $expr ) ); + } + /** * @see ApiBase::getAllowedParams() * @return array diff --git a/includes/parser/AbuseFilterParser.php b/includes/parser/AbuseFilterParser.php index 2af6aae71..1ca05cd66 100644 --- a/includes/parser/AbuseFilterParser.php +++ b/includes/parser/AbuseFilterParser.php @@ -206,11 +206,12 @@ class AbuseFilterParser extends AFPTransitionBase { } /** + * Check the syntax of $filter, throwing an exception if invalid * @param string $filter * @return true When successful * @throws AFPUserVisibleException */ - public function checkSyntax( $filter ) { + public function checkSyntaxThrow( string $filter ) { $this->allowMissingVariables = true; $origAS = $this->mAllowShort; try { @@ -224,6 +225,57 @@ class AbuseFilterParser extends AFPTransitionBase { return true; } + /** + * Check the syntax of $filter, without throwing + * + * @param string $filter + * @return true|array True when successful, otherwise a two-element array with exception message + * and character position of the syntax error + */ + public function checkSyntax( string $filter ) { + try { + $res = $this->checkSyntaxThrow( $filter ); + } catch ( AFPUserVisibleException $excep ) { + $res = [ $excep->getMessageObj()->text(), $excep->mPosition ]; + } + return $res; + } + + /** + * This is the main entry point. It checks the given conditions and returns whether + * they match. In case of bad syntax, this is always logged, and $ignoreError can + * be used to determine whether this method should throw. + * + * @param string $conds + * @param bool $ignoreError + * @param string|null $filter The ID of the filter being parsed + * @return bool + * @throws Exception + */ + public function checkConditions( string $conds, $ignoreError = true, $filter = null ) : bool { + try { + $result = $this->parse( $conds ); + } catch ( Exception $excep ) { + $result = false; + + if ( $excep instanceof AFPUserVisibleException ) { + $msg = $excep->getMessageForLogs(); + $excep->setLocalizedMessage(); + } else { + $msg = $excep->getMessage(); + } + + $extraInfo = $filter !== null ? " for filter $filter" : ''; + $this->logger->warning( "AbuseFilter parser error$extraInfo: $msg" ); + + if ( !$ignoreError ) { + throw $excep; + } + } + + return $result; + } + /** * Move to the next token */ diff --git a/tests/phpunit/unit/AbuseFilterParserTestCase.php b/tests/phpunit/unit/AbuseFilterParserTestCase.php index d09ed453b..abdb37269 100644 --- a/tests/phpunit/unit/AbuseFilterParserTestCase.php +++ b/tests/phpunit/unit/AbuseFilterParserTestCase.php @@ -60,7 +60,7 @@ abstract class AbuseFilterParserTestCase extends MediaWikiUnitTestCase { try { if ( $skippedBlock ) { // Skipped blocks are, well, skipped when actually parsing. - $parser->checkSyntax( $expr ); + $parser->checkSyntaxThrow( $expr ); } else { $parser->parse( $expr ); }