From e54c06e08bdd04c8048aacc16eb0d8154b77e7ee Mon Sep 17 00:00:00 2001
From: Helmut Hummel <typo3@helhum.io>
Date: Wed, 2 Mar 2022 10:10:53 +0100
Subject: [PATCH] [TASK] Move TS condition check to TS condition matcher
The ExpressionLanguageResolver should be generic and not
implement some TypoScript related logic.
Therefore the check for "ELSE" expression is moved to
AbstractConditionMatcher
Dead code is removed on the go.
Resolves: #97077
Releases: 10.4, 11.5, main
Change-Id: Ic5037b46c65ffb80f2f5b242e671e2454922171c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/73838
Tested-by: core-ci <typo3@b13.com>
Tested-by: Helmut Hummel <typo3@helhum.io>
Reviewed-by: Helmut Hummel <typo3@helhum.io>
---
.../AbstractConditionMatcher.php | 11 ++++++----
.../Classes/ExpressionLanguage/Resolver.php | 6 ------
.../AbstractConditionMatcherTest.php | 15 ++++++++++++++
.../Unit/ExpressionLanguage/ResolverTest.php | 20 +++----------------
4 files changed, 25 insertions(+), 27 deletions(-)
diff --git a/typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcher.php b/typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcher.php
index fd6a70435641..e9d2a7747960 100644
--- a/typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcher.php
+++ b/typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcher.php
@@ -189,11 +189,14 @@ abstract class AbstractConditionMatcher implements LoggerAwareInterface, Conditi
*/
protected function evaluateExpression(string $expression): bool
{
+ // The TypoScript [ELSE] condition is not known by the Symfony Expression Language
+ // and must not be evaluated. If/else logic is handled in TypoScriptParser.
+ if (strtoupper($expression) === 'ELSE') {
+ return false;
+ }
+
try {
- $result = $this->expressionLanguageResolver->evaluate($expression);
- if ($result !== null) {
- return $result;
- }
+ return $this->expressionLanguageResolver->evaluate($expression);
} catch (MissingTsfeException $e) {
// TSFE is not available in the current context (e.g. TSFE in BE context),
// we set all conditions false for this case.
diff --git a/typo3/sysext/core/Classes/ExpressionLanguage/Resolver.php b/typo3/sysext/core/Classes/ExpressionLanguage/Resolver.php
index b7a4a881f9fe..638600cad420 100644
--- a/typo3/sysext/core/Classes/ExpressionLanguage/Resolver.php
+++ b/typo3/sysext/core/Classes/ExpressionLanguage/Resolver.php
@@ -76,12 +76,6 @@ class Resolver
*/
public function evaluate(string $condition): bool
{
- // The TypoScript [ELSE] condition is not known by the Symfony Expression Language
- // and must not be evaluated. If/else logic is handled in TypoScriptParser.
- if (strtoupper($condition) === 'ELSE') {
- return false;
- }
-
return (bool)$this->expressionLanguage->evaluate($condition, $this->expressionLanguageVariables);
}
diff --git a/typo3/sysext/core/Tests/Unit/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcherTest.php b/typo3/sysext/core/Tests/Unit/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcherTest.php
index f1cad165d33d..61c771a900af 100644
--- a/typo3/sysext/core/Tests/Unit/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcherTest.php
+++ b/typo3/sysext/core/Tests/Unit/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcherTest.php
@@ -28,6 +28,7 @@ use TYPO3\CMS\Core\Context\Context;
use TYPO3\CMS\Core\Context\DateTimeAspect;
use TYPO3\CMS\Core\Core\ApplicationContext;
use TYPO3\CMS\Core\Core\Environment;
+use TYPO3\CMS\Core\ExpressionLanguage\Resolver;
use TYPO3\CMS\Core\Http\ServerRequest;
use TYPO3\CMS\Core\Package\PackageInterface;
use TYPO3\CMS\Core\Package\PackageManager;
@@ -373,4 +374,18 @@ class AbstractConditionMatcherTest extends UnitTestCase
$result = $this->evaluateExpressionMethod->invokeArgs($this->conditionMatcher, ['ip("devIP")']);
self::assertSame($expectedResult, $result);
}
+
+ /**
+ * @test
+ */
+ public function typoScriptElseConditionIsNotEvaluatedAndAlwaysReturnsFalse(): void
+ {
+ $this->initConditionMatcher();
+ $expressionProperty = new \ReflectionProperty(AbstractConditionMatcher::class, 'expressionLanguageResolver');
+ $expressionProperty->setAccessible(true);
+ $resolverProphecy = $this->prophesize(Resolver::class);
+ $resolverProphecy->evaluate(Argument::cetera())->shouldNotBeCalled();
+ $expressionProperty->setValue($this->conditionMatcher, $resolverProphecy->reveal());
+ self::assertFalse($this->evaluateExpressionMethod->invokeArgs($this->conditionMatcher, ['ELSE']));
+ }
}
diff --git a/typo3/sysext/core/Tests/Unit/ExpressionLanguage/ResolverTest.php b/typo3/sysext/core/Tests/Unit/ExpressionLanguage/ResolverTest.php
index 8b4032bc6bfa..397eccfc49c9 100644
--- a/typo3/sysext/core/Tests/Unit/ExpressionLanguage/ResolverTest.php
+++ b/typo3/sysext/core/Tests/Unit/ExpressionLanguage/ResolverTest.php
@@ -24,7 +24,6 @@ use TYPO3\CMS\Core\Cache\Frontend\PhpFrontend;
use TYPO3\CMS\Core\ExpressionLanguage\DefaultProvider;
use TYPO3\CMS\Core\ExpressionLanguage\FunctionsProvider\DefaultFunctionsProvider;
use TYPO3\CMS\Core\ExpressionLanguage\Resolver;
-use TYPO3\CMS\Core\Http\ServerRequest;
use TYPO3\CMS\Core\Package\PackageInterface;
use TYPO3\CMS\Core\Package\PackageManager;
use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -80,21 +79,10 @@ class ResolverTest extends UnitTestCase
*/
public function basicExpressionHandlingResultsWorksAsExpected(string $expression, $expectedResult)
{
- $request = new ServerRequest();
- $expressionLanguageResolver = new Resolver('default', [], $request);
+ $expressionLanguageResolver = new Resolver('default', []);
self::assertSame($expectedResult, $expressionLanguageResolver->evaluate($expression));
}
- /**
- * @test
- */
- public function typoScriptElseConditionIsNotEvaluatedAndAlwaysReturnsFalse()
- {
- $request = new ServerRequest();
- $expressionLanguageResolver = new Resolver('default', [], $request);
- self::assertFalse($expressionLanguageResolver->evaluate('ELSE'));
- }
-
/**
* @return array
*/
@@ -127,9 +115,8 @@ class ResolverTest extends UnitTestCase
'varTrue' => true,
'varFalse' => false,
]);
- $request = new ServerRequest();
GeneralUtility::addInstance(DefaultProvider::class, $contextProphecy->reveal());
- $expressionLanguageResolver = new Resolver('default', [], $request);
+ $expressionLanguageResolver = new Resolver('default', []);
self::assertSame($expectedResult, $expressionLanguageResolver->evaluate($expression));
}
@@ -168,10 +155,9 @@ class ResolverTest extends UnitTestCase
'var1' => 'FOO',
'var2' => 'foo'
]);
- $request = new ServerRequest();
GeneralUtility::addInstance(DefaultProvider::class, $contextProphecy->reveal());
GeneralUtility::addInstance(DefaultFunctionsProvider::class, $expressionProvider->reveal());
- $expressionLanguageResolver = new Resolver('default', [], $request);
+ $expressionLanguageResolver = new Resolver('default', []);
self::assertSame($expectedResult, $expressionLanguageResolver->evaluate($expression));
}
}
--
GitLab