Better handling of keywords and functions

Always run the keyword/function handler, even if there are DUNDEFINED
arguments, so that the handler can perform further validation on the
input and report any error to the user. However, replace DUNDEFINED with
DNULL before running the handler, to avoid special-casing DUNDEFINED in
every handler. If any argument was a DUNDEFINED, we will return
DUNDEFINED anyway.

Also centralize the keyword handling logic to a new method, like it
happens for functions.

Bug: T234339
Change-Id: I875cb77418a39790e91fe5867c49917bfe406ed4
This commit is contained in:
Daimona Eaytoy 2019-10-01 18:27:56 +02:00
parent e98799a00a
commit 1abaff1aac
2 changed files with 52 additions and 5 deletions

View file

@ -1250,14 +1250,20 @@ class AbuseFilterParser {
} else {
$this->checkArgCount( $args, $fname );
$this->raiseCondCount();
// Any undefined argument should be special-cased by the function, but that would be too
// much overhead. We also cannot skip calling the handler in case it's making further
// validation (T234339). So temporarily replace the DUNDEFINED with a DNULL.
// @todo This is subpar.
$hasUndefinedArg = false;
foreach ( $args as $arg ) {
foreach ( $args as $i => $arg ) {
if ( $arg->type === AFPData::DUNDEFINED ) {
$args[$i] = new AFPData( AFPData::DNULL );
$hasUndefinedArg = true;
break;
}
}
if ( $hasUndefinedArg ) {
$this->$funcHandler( $args );
$result = new AFPData( AFPData::DUNDEFINED );
} else {
$result = $this->$funcHandler( $args );
@ -1284,11 +1290,24 @@ class AbuseFilterParser {
*/
protected function callKeyword( $kname, AFPData $lhs, AFPData $rhs ) : AFPData {
$func = self::$mKeywords[$kname];
$this->raiseCondCount();
if ( $lhs->getType() === AFPData::DUNDEFINED || $rhs->getType() === AFPData::DUNDEFINED ) {
return new AFPData( AFPData::DUNDEFINED );
$hasUndefinedOperand = false;
if ( $lhs->getType() === AFPData::DUNDEFINED ) {
$lhs = new AFPData( AFPData::DNULL );
$hasUndefinedOperand = true;
}
if ( $rhs->getType() === AFPData::DUNDEFINED ) {
$rhs = new AFPData( AFPData::DNULL );
$hasUndefinedOperand = true;
}
if ( $hasUndefinedOperand ) {
// We need to run the handler with bogus args, see the comment in self::callFunc (T234339)
// @todo Likewise, this is subpar.
// @phan-suppress-next-line PhanParamTooMany Not every function needs the position
$this->$func( $lhs, $rhs, $this->mCur->pos );
$result = new AFPData( AFPData::DUNDEFINED );
} else {
$this->raiseCondCount();
// @phan-suppress-next-line PhanParamTooMany Not every function needs the position
$result = $this->$func( $lhs, $rhs, $this->mCur->pos );
}

View file

@ -1142,4 +1142,32 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
[ "contains_any(1,2,,3,)" ],
];
}
/**
* Ensure that any error in the arguments to a keyword or function is reported when
* checking syntax (T234339)
* @param string $code
* @param string $expID Expected exception ID
* @dataProvider provideArgsErrorsInSyntaxCheck
*/
public function testArgsErrorsInSyntaxCheck( $code, $expID ) {
$caller = '[unavailable]';
$this->exceptionTest( $expID, $code, $caller );
$this->exceptionTestInSkippedBlock( $expID, $code, $caller );
}
/**
* @return array
*/
public function provideArgsErrorsInSyntaxCheck() {
return [
[ 'accountname rlike "("', 'regexfailure' ],
[ 'contains_any( new_wikitext, "foo", 3/0 )', 'dividebyzero' ],
[ 'rcount( "(", added_lines )', 'regexfailure' ],
[ 'get_matches( "(", new_wikitext )', 'regexfailure' ],
[ 'added_lines contains string(3/0)', 'dividebyzero' ],
[ 'norm(new_text) irlike ")"', 'regexfailure' ],
[ 'ip_in_range( user_name, "foobar" )', 'invalidiprange' ]
];
}
}