wikimedia/mediawiki-extensions-AbuseFilter
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git synced 2024-11-27 15:30:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

Hard-deprecate empty operands

Browse source 
This bumps the level to WARN, and makes it very clear that people should
fix the affected filters. It also removes the calling method, which was
mostly meant for debugging purposes, and changes the type to 'op_type'
to avoid conflicting with type:mediawiki in logstash.

Bug: T156096
Change-Id: Ie73f1604e8ed82bc2e1be9fc90fa065be37889a3
This commit is contained in:
Daimona Eaytoy 2019-09-16 13:13:44 +02:00 committed by Mobrovac
parent c0a99d876b
commit a77a59b962
2 changed files with 30 additions and 28 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
includes/parser/AbuseFilterParser.php
View file

@ -475,7 +475,7 @@ class AbuseFilterParser {
$checkEmpty = $result->getType() === AFPData::DEMPTY;
$this->doLevelSet( $result );
if ( $checkEmpty && $result->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'var assignment', __METHOD__ );
$this->logEmptyOperand( 'var assignment' );
}
$this->setUserVariable( $varname, $result );
@ -518,7 +518,7 @@ class AbuseFilterParser {
}
$this->move();
if ( $this->mCur->type === AFPToken::TNONE ) {
$this->logEmptyOperand( 'array assignment', __METHOD__ );
$this->logEmptyOperand( 'array assignment' );
}
$this->doLevelSet( $result );
if ( $array->getType() === AFPData::DARRAY ) {
@ -646,7 +646,7 @@ class AbuseFilterParser {
}
$this->doLevelConditions( $r2 );
if ( $r2->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'ternary else', __METHOD__ );
$this->logEmptyOperand( 'ternary else' );
}
if ( $isTrue ) {
$this->mShortCircuit = $scOrig;
@ -680,7 +680,7 @@ class AbuseFilterParser {
$scOrig = wfSetVar( $this->mShortCircuit, $this->mAllowShort, true );
$this->doLevelCompares( $r2 );
if ( $r2->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'bool operand', __METHOD__ );
$this->logEmptyOperand( 'bool operand' );
}
$this->mShortCircuit = $scOrig;
$result = new AFPData( AFPData::DBOOL, $curVal );
@ -689,7 +689,7 @@ class AbuseFilterParser {
$this->doLevelCompares( $r2 );
if ( $r2->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'bool operand', __METHOD__ );
$this->logEmptyOperand( 'bool operand' );
}
$result = $result->boolOp( $r2, $op );
}
@ -716,7 +716,7 @@ class AbuseFilterParser {
$r2 = new AFPData( AFPData::DEMPTY );
$this->doLevelSumRels( $r2 );
if ( $r2->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'compare operand', __METHOD__ );
$this->logEmptyOperand( 'compare operand' );
}
if ( $this->mShortCircuit ) {
// The result doesn't matter.
@ -741,7 +741,7 @@ class AbuseFilterParser {
$r2 = new AFPData( AFPData::DEMPTY );
$this->doLevelMulRels( $r2 );
if ( $r2->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'sum operand', __METHOD__ );
$this->logEmptyOperand( 'sum operand' );
}
if ( $this->mShortCircuit ) {
// The result doesn't matter.
@ -770,7 +770,7 @@ class AbuseFilterParser {
$r2 = new AFPData( AFPData::DEMPTY );
$this->doLevelPow( $r2 );
if ( $r2->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'multiplication operand', __METHOD__ );
$this->logEmptyOperand( 'multiplication operand' );
}
if ( $this->mShortCircuit ) {
// The result doesn't matter.
@ -792,7 +792,7 @@ class AbuseFilterParser {
$expanent = new AFPData( AFPData::DEMPTY );
$this->doLevelBoolInvert( $expanent );
if ( $expanent->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'power operand', __METHOD__ );
$this->logEmptyOperand( 'power operand' );
}
if ( $this->mShortCircuit ) {
// The result doesn't matter.
@ -813,7 +813,7 @@ class AbuseFilterParser {
$checkEmpty = $result->getType() === AFPData::DEMPTY;
$this->doLevelSpecialWords( $result );
if ( $checkEmpty && $result->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'bool inversion', __METHOD__ );
$this->logEmptyOperand( 'bool inversion' );
}
if ( $this->mShortCircuit ) {
// The result doesn't matter.
@ -841,7 +841,7 @@ class AbuseFilterParser {
$this->doLevelUnarys( $r2 );
if ( $r2->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'keyword operand', __METHOD__ );
$this->logEmptyOperand( 'keyword operand' );
}
if ( $this->mShortCircuit ) {
@ -865,7 +865,7 @@ class AbuseFilterParser {
$checkEmpty = $result->getType() === AFPData::DEMPTY;
$this->doLevelArrayElements( $result );
if ( $checkEmpty && $result->getType() === AFPData::DEMPTY ) {
$this->logEmptyOperand( 'unary operand', __METHOD__ );
$this->logEmptyOperand( 'unary operand' );
}
if ( $this->mShortCircuit ) {
// The result doesn't matter.
@ -923,7 +923,7 @@ class AbuseFilterParser {
if ( $this->mCur->type === AFPToken::TBRACE && $this->mCur->value === '(' ) {
$next = $this->getNextToken();
if ( $next->type === AFPToken::TBRACE && $next->value === ')' ) {
$this->logEmptyOperand( 'parenthesized expression', __METHOD__ );
$this->logEmptyOperand( 'parenthesized expression' );
// We don't need DUNDEFINED here
$this->move();
$this->move();
@ -1002,7 +1002,7 @@ class AbuseFilterParser {
$r = new AFPData( AFPData::DEMPTY );
$this->doLevelSemicolon( $r );
if ( $r->getType() === AFPData::DEMPTY && !$this->functionIsVariadic( $fname ) ) {
$this->logEmptyOperand( 'function argument', __METHOD__ );
$this->logEmptyOperand( 'non-variadic function argument' );
}
$args[] = $r;
} while ( $this->mCur->type === AFPToken::TCOMMA );
@ -1950,17 +1950,19 @@ class AbuseFilterParser {
* Log empty operands for T156096
*
* @param string $type Type of the empty operand
* @param string $fname Method where the empty operand is found
*/
protected function logEmptyOperand( $type, $fname ) {
$this->logger->info(
"Empty operand of type {type} at method {fname}. Filter: {filter}",
[
'type' => $type,
'fname' => $fname,
'filter' => $this->mFilter ?? 'unavailable'
]
);
protected function logEmptyOperand( $type ) {
if ( $this->mFilter !== null ) {
$this->logger->warning(
'DEPRECATED! Found empty operand of type `{op_type}` when parsing filter: {filter}. ' .
'This is deprecated since 1.34 and support will be discontinued soon. Please fix ' .
'the affected filter!',
[
'op_type' => $type,
'filter' => $this->mFilter
]
);
}
}
/**

8
tests/phpunit/unit/AbuseFilterParserTest.php
View file

@ -1041,9 +1041,9 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
[ '1**', 'power operand' ],
[ '"string" contains', 'keyword operand' ],
[ '1 in', 'keyword operand' ],
[ "str_replace('a','b','c',)", 'function argument' ],
[ "count('a','b',)", 'function argument' ],
[ "ccnorm('a',)", 'function argument' ],
[ "str_replace('a','b','c',)", 'non-variadic function argument' ],
[ "count('a','b',)", 'non-variadic function argument' ],
[ "ccnorm('a',)", 'non-variadic function argument' ],
[ "(!)", 'bool inversion' ],
// `(false &!)` and `(true &!)`, originally reported in T156096,
// should be used in the future to test that they throw. However,
@ -1083,7 +1083,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
$mock->expects( $this->never() )
->method( 'logEmptyOperand' )
->with( 'function argument' );
->with( 'non-variadic function argument' );
$mock->toggleConditionLimit( false );
$mock->parse( $code );