[BUGFIX] Fix nested arrays in conditions 35/58935/9
authorWolfgang Klinger <wolfgang@wazum.com>
Fri, 23 Nov 2018 12:23:25 +0000 (13:23 +0100)
committerMarkus Klein <markus.klein@typo3.org>
Fri, 7 Dec 2018 20:19:25 +0000 (21:19 +0100)
The RequestWrapper::getParsedBody method must return an array,
otherwise the a condition like [request.getParsedBody()['foo'] == 1]
results in a silent exception if the body is empty.

ConditionMatcher::normalizeExpression is now using an advanced regex
to handle multiple [] parts correctly.

Resolves: #86915
Releases: master
Change-Id: Ia3f951d1a3994d545025691e35521ca05b97a39a
Reviewed-on: https://review.typo3.org/58935
Tested-by: TYPO3com <no-reply@typo3.com>
Tested-by: Joerg Kummer <typo3@enobe.de>
Reviewed-by: Raphael Graf <r@undefined.ch>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcher.php
typo3/sysext/core/Classes/ExpressionLanguage/RequestWrapper.php
typo3/sysext/core/Tests/Unit/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcherTest.php

index e7908e5..f01ccbd 100644 (file)
@@ -146,7 +146,7 @@ abstract class AbstractConditionMatcher implements LoggerAwareInterface
     }
 
     /**
-     * Normalizes an expression and removes the first and last square bracket.
+     * Normalizes an expression
      * + OR normalization: "...]OR[...", "...]||[...", "...][..." --> "...]||[..."
      * + AND normalization: "...]AND[...", "...]&&[..."                   --> "...]&&[..."
      *
@@ -156,14 +156,47 @@ abstract class AbstractConditionMatcher implements LoggerAwareInterface
      */
     protected function normalizeExpression($expression)
     {
-        $normalizedExpression = preg_replace([
-            '/\\]\\s*(OR|\\|\\|)?\\s*\\[/i',
-            '/\\]\\s*(AND|&&)\\s*\\[/i'
-        ], [
-            ']||[',
-            ']&&['
-        ], trim($expression));
-        return $normalizedExpression;
+        $removeSpaces = '/
+          \\s*
+          (                    # subroutine 1
+            \\[
+              (?:
+                [^\\[\\]]      # any character except []
+                | (?1)         # recursive subroutine 1 when brackets are around
+              )*
+            \\]
+          )
+          \\s*
+          /xi';
+
+        $adjacentBrackets = '/
+          (                    # subroutine 1
+            \\[
+              (?:
+                [^\\[\\]]      # any character except []
+                | (?1)         # recursive subroutine 1 when brackets are around
+              )*
+            \\]
+          )
+          (*SKIP)              # avoid backtracking into completed bracket expressions
+          \\[                  # match the following [
+          /xi';
+
+        return preg_replace(
+            [
+                $removeSpaces,
+                '/\\]AND\\[/i',
+                '/\\]OR\\[/i',
+                $adjacentBrackets
+            ],
+            [
+                '\\1',
+                ']&&[',
+                ']||[',
+                '\\1||['
+            ],
+            trim($expression)
+        );
     }
 
     /**
index af72b65..9067467 100644 (file)
@@ -48,9 +48,9 @@ class RequestWrapper
         return $this->request->getQueryParams();
     }
 
-    public function getParsedBody()
+    public function getParsedBody(): array
     {
-        return $this->request->getParsedBody();
+        return (array)($this->request->getParsedBody() ?? []);
     }
 
     public function getHeaders(): array
index 358acb0..35c2658 100644 (file)
@@ -41,7 +41,7 @@ class AbstractConditionMatcherTest extends UnitTestCase
     protected $backupApplicationContext;
 
     /**
-     * @var AbstractConditionMatcher|\PHPUnit_Framework_MockObject_MockObject
+     * @var AbstractConditionMatcher|\PHPUnit_Framework_MockObject_MockObject|\TYPO3\TestingFramework\Core\AccessibleObjectInterface
      */
     protected $conditionMatcher;
 
@@ -702,4 +702,58 @@ class AbstractConditionMatcherTest extends UnitTestCase
             )
         );
     }
+
+    public function expressionDataProvider(): array
+    {
+        return [
+            // Default variants
+            '[]' => ['[]', '[]'],
+            '[foo]' => ['[foo]', '[foo]'],
+            '[foo] && [bar]' => ['[foo] && [bar]', '[foo]&&[bar]'],
+            '[foo] AND [bar]' => ['[foo] AND [bar]', '[foo]&&[bar]'],
+            '[foo] and [bar]' => ['[foo] and [bar]', '[foo]&&[bar]'],
+            '[foo] [bar]' => ['[foo] [bar]', '[foo]||[bar]'],
+            '[foo] || [bar]' => ['[foo] || [bar]', '[foo]||[bar]'],
+            '[foo] OR [bar]' => ['[foo] OR [bar]', '[foo]||[bar]'],
+            '[foo] or [bar]' => ['[foo] or [bar]', '[foo]||[bar]'],
+            '[foo] && [bar]&&[baz]' => ['[foo] && [bar]&&[baz]', '[foo]&&[bar]&&[baz]'],
+            '[foo] AND [bar]AND[baz]' => ['[foo] AND [bar]AND[baz]', '[foo]&&[bar]&&[baz]'],
+            '[foo] and [bar]and[baz]' => ['[foo] and [bar]and[baz]', '[foo]&&[bar]&&[baz]'],
+            '[foo] || [bar]||[baz]' => ['[foo] || [bar]||[baz]', '[foo]||[bar]||[baz]'],
+            '[foo] OR [bar]OR[baz]' => ['[foo] OR [bar]OR[baz]', '[foo]||[bar]||[baz]'],
+            '[foo] or [bar]or[baz]' => ['[foo] or [bar]or[baz]', '[foo]||[bar]||[baz]'],
+            '[foo] && [bar]||[baz]' => ['[foo] && [bar]||[baz]', '[foo]&&[bar]||[baz]'],
+            '[foo] AND [bar]OR[baz]' => ['[foo] AND [bar]OR[baz]', '[foo]&&[bar]||[baz]'],
+            '[foo] and [bar]or[baz]' => ['[foo] and [bar]or[baz]', '[foo]&&[bar]||[baz]'],
+            '[foo] || [bar]OR[baz]' => ['[foo] || [bar]OR[baz]', '[foo]||[bar]||[baz]'],
+            '[foo] || [bar]or[baz]' => ['[foo] || [bar]or[baz]', '[foo]||[bar]||[baz]'],
+            '[foo] OR [bar]AND[baz]' => ['[foo] OR [bar]AND[baz]', '[foo]||[bar]&&[baz]'],
+            '[foo] or [bar]and[baz]' => ['[foo] or [bar]and[baz]', '[foo]||[bar]&&[baz]'],
+
+            // Special variants
+            '[foo && bar && baz]' => ['[foo && bar && baz]', '[foo && bar && baz]'],
+            '[foo and bar and baz]' => ['[foo and bar and baz]', '[foo and bar and baz]'],
+            '[foo AND bar AND baz]' => ['[foo AND bar AND baz]', '[foo AND bar AND baz]'],
+            '[foo || bar || baz]' => ['[foo || bar || baz]', '[foo || bar || baz]'],
+            '[foo or bar or baz]' => ['[foo or bar or baz]', '[foo or bar or baz]'],
+            '[foo OR bar OR baz]' => ['[foo OR bar OR baz]', '[foo OR bar OR baz]'],
+            '[request.getParsedBody()[\'type\'] > 0]' => ['[request.getParsedBody()[\'type\'] > 0]', '[request.getParsedBody()[\'type\'] > 0]'],
+            '[request.getParsedBody()[\'type\'] > 0 || request.getQueryParams()[\'type\'] > 0]' => ['[request.getParsedBody()[\'type\'] > 0 || request.getQueryParams()[\'type\'] > 0]', '[request.getParsedBody()[\'type\'] > 0 || request.getQueryParams()[\'type\'] > 0]'],
+            '[request.getParsedBody()[\'type\'] > 0 or request.getQueryParams()[\'type\'] == 1]' => ['[request.getParsedBody()[\'type\'] > 0 or request.getQueryParams()[\'type\'] == 1]', '[request.getParsedBody()[\'type\'] > 0 or request.getQueryParams()[\'type\'] == 1]'],
+            '[ (request.getParsedBody()[\'type\'] > 0) || (request.getQueryParams()[\'type\'] > 0) ]' => ['[ (request.getParsedBody()[\'type\'] > 0) || (request.getQueryParams()[\'type\'] > 0) ]', '[ (request.getParsedBody()[\'type\'] > 0) || (request.getQueryParams()[\'type\'] > 0) ]'],
+            '[request.getParsedBody()[\'tx_news_pi1\'][\'news\'] > 0 || request.getQueryParams()[\'tx_news_pi1\'][\'news\'] > 0]' => ['[request.getParsedBody()[\'tx_news_pi1\'][\'news\'] > 0 || request.getQueryParams()[\'tx_news_pi1\'][\'news\'] > 0]', '[request.getParsedBody()[\'tx_news_pi1\'][\'news\'] > 0 || request.getQueryParams()[\'tx_news_pi1\'][\'news\'] > 0]'],
+            '[request.getQueryParams()[\'tx_news_pi1\'][\'news\'] > 0]' => ['[request.getQueryParams()[\'tx_news_pi1\'][\'news\'] > 0]', '[request.getQueryParams()[\'tx_news_pi1\'][\'news\'] > 0]'],
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider expressionDataProvider
+     * @param string $expression
+     * @param string $expectedResult
+     */
+    public function normalizeExpressionWorksAsExpected(string $expression, string $expectedResult): void
+    {
+        $this->assertSame($expectedResult, $this->conditionMatcher->_call('normalizeExpression', $expression));
+    }
 }