Partly handle set and set_var in shortcircuit

This is more complicated than the := operator, because the var name
could be a complicated expression, and we have to handle a function
call. This patch only covers the case where the variable name is a
literal, which is enough for WMF production.

Bug: T214674
Change-Id: I6c0f8e95663919a0235b5ccf0c88ad0a539315a7
This commit is contained in:
Daimona Eaytoy 2019-08-06 14:14:55 +02:00
parent 333bbaaeb3
commit 2ed6272bb2
3 changed files with 41 additions and 10 deletions

View file

@ -356,6 +356,16 @@ class AbuseFilterCachingParser extends AbuseFilterParser {
throw new AFPUserVisibleException( 'unrecognisedvar', $node->position, [ $varName ] );
}
$this->setUserVariable( $varName, new AFPData( AFPData::DUNDEFINED ) );
} elseif (
$node->type === AFPTreeNode::FUNCTION_CALL &&
in_array( $node->children[0], [ 'set', 'set_var' ] ) &&
isset( $node->children[1] )
) {
$varnameNode = $node->children[1];
if ( $varnameNode->type === AFPTreeNode::ATOM ) {
// @todo Handle expressions here
$this->setUserVariable( $varnameNode->children->value, new AFPData( AFPData::DUNDEFINED ) );
}
} elseif ( $node->type === AFPTreeNode::ATOM ) {
return;
}

View file

@ -243,18 +243,34 @@ class AbuseFilterParser {
}
} elseif ( $this->mCur->type === AFPToken::TID ) {
// T214674, define variables
$varname = $this->mCur->value;
$next = $this->getNextToken();
if ( $next->type === AFPToken::TOP && $next->value === ':=' ) {
$this->setUserVariable( $varname, new AFPData( AFPData::DUNDEFINED ) );
} elseif ( $next->type === AFPToken::TSQUAREBRACKET && $next->value === '[' ) {
if ( !$this->mVariables->varIsSet( $varname ) ) {
throw new AFPUserVisibleException( 'unrecognisedvar',
$next->pos,
[ $varname ]
);
if (
in_array( $this->mCur->value, [ 'set', 'set_var' ] ) &&
$next->type === AFPToken::TBRACE && $next->value === '('
) {
// This is for setter functions.
$this->move();
$braces++;
$next = $this->getNextToken();
if ( $next->type === AFPToken::TSTRING ) {
// @todo Handle expressions here
$this->setUserVariable( $next->value, new AFPData( AFPData::DUNDEFINED ) );
}
} else {
// Simple assignment with :=
$varname = $this->mCur->value;
$next = $this->getNextToken();
if ( $next->type === AFPToken::TOP && $next->value === ':=' ) {
$this->setUserVariable( $varname, new AFPData( AFPData::DUNDEFINED ) );
} elseif ( $next->type === AFPToken::TSQUAREBRACKET && $next->value === '[' ) {
if ( !$this->mVariables->varIsSet( $varname ) ) {
throw new AFPUserVisibleException( 'unrecognisedvar',
$next->pos,
[ $varname ]
);
}
$this->setUserVariable( $varname, new AFPData( AFPData::DUNDEFINED ) );
}
$this->setUserVariable( $varname, new AFPData( AFPData::DUNDEFINED ) );
}
}
}

View file

@ -226,6 +226,8 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
[ 'a := [1,2', 'doLevelAtom' ],
[ '1 = 1 | (', 'skipOverBraces' ],
[ 'a := [1,2,3]; 3 = a[5', 'doLevelArrayElements' ],
// `set` is the name of a function. It's unclear whether the following should be allowed.
[ 'set:= 1; set contains 1', 'doLevelFunction' ],
];
}
@ -739,12 +741,15 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
[ "x := [5]; false & (1 == 1; y := 'b'; x[1] := 'x'; 3 < 4); y != 'b' & x[1] != 'x'" ],
[ "(var := [1]); false & ( var[] := 'baz' ); var contains 'baz'" ],
[ "(var := [1]); false & ( var[1] := 'baz' ); var[1] === 'baz'" ],
[ "false & (set('myvar', 1)); myvar contains 1" ],
// The following tests are to ensure that we don't get a match
[ "false & ( var := 'foo'; x := get_matches( var, added_lines )[1] ); x != false" ],
[ "false & ( var := 'foo'); var !== null" ],
[ "false & ( var := 'foo'); var === null" ],
[ "false & (set('myvar', 'foo')); myvar === 'foo' | myvar !== 'foo'" ],
[ "false & ( var := 'foo'); var[0] !== 123456" ],
[ "false & ( var := 'foo'); var[0][123] !== 123456" ],
[ "false & (set('myvar', 'foo')); myvar[1][2] === 'foo' | myvar[1][2] !== 'foo'" ],
// Identifier before closing skipped brace, T214674#5374757
[ "false & ( var := 'foo'; 'x' in var )" ],
[ "false & ( var := 'foo'; added_lines irlike var )" ],