From f5d7b6890898c2e4c672142de70a91ba3ec0f099 Mon Sep 17 00:00:00 2001 From: STran Date: Mon, 10 Jun 2024 10:56:10 -0700 Subject: [PATCH] Force full evaluation of filter in FilterEvaluator->getUsedVars() In some cases, evaluation short-circuits when getting a list of used variables resulting in an incomplete array of variables. This subsequently causes issues when using those arrays for validation checks (eg. if protected variables are used). - Force full evaluation by setting `mAllowShort` to false Bug: T364485 Change-Id: Idf2112d9ebf63846cde3ce9b8a8ade0ed909505d --- includes/Parser/FilterEvaluator.php | 8 +++++++- tests/phpunit/unit/Parser/ParserTest.php | 10 ++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/includes/Parser/FilterEvaluator.php b/includes/Parser/FilterEvaluator.php index a7724caba..7efb0ec09 100644 --- a/includes/Parser/FilterEvaluator.php +++ b/includes/Parser/FilterEvaluator.php @@ -454,7 +454,13 @@ class FilterEvaluator { */ public function getUsedVars( $filter ) { $this->resetState(); - $this->evaluateExpression( $filter ); + $cachedAllowShort = $this->mAllowShort; + try { + $this->mAllowShort = false; + $this->evaluateExpression( $filter ); + } finally { + $this->mAllowShort = $cachedAllowShort; + } return array_unique( $this->usedVars ); } diff --git a/tests/phpunit/unit/Parser/ParserTest.php b/tests/phpunit/unit/Parser/ParserTest.php index 1adaf5cbb..f7ed30e99 100644 --- a/tests/phpunit/unit/Parser/ParserTest.php +++ b/tests/phpunit/unit/Parser/ParserTest.php @@ -1170,7 +1170,13 @@ class ParserTest extends ParserTestCase { $parser = $this->getParser(); /** @var FilterEvaluator $wrapper */ $wrapper = TestingAccessWrapper::newFromObject( $parser ); - $this->assertSame( [ 'user_name' ], $wrapper->getUsedVars( 'ip_in_range( user_name, "1.2.3.4" )' ) ); - $this->assertSame( [ 'user_name' ], $wrapper->getUsedVars( 'ip_in_range( USER_NAME, "1.2.3.4" )' ) ); + $this->assertSame( + [ 'action', 'user_name' ], + $wrapper->getUsedVars( 'action = "edit" & ip_in_range( user_name, "1.2.3.4" )' ) + ); + $this->assertSame( + [ 'action', 'user_name' ], + $wrapper->getUsedVars( 'action = "edit" & ip_in_range( USER_NAME, "1.2.3.4" )' ) + ); } }