From db22f222fe87d7732b7554940e540a7c7110ceca Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Thu, 16 Nov 2023 14:57:49 +0100
Subject: [PATCH] [BUGFIX] Consider URL encoded values for
 addQueryString.exclude

TypoScript property `addQueryString.exclude` concerns the internal,
URL-decoded values. However, the parameters are purged from a query
array that is URL-encoded.

Resolves: #102386
Releases: main, 12.4
Change-Id: Ibcdca016221035f5f2974007b1fc52c04f5d643f
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/82261
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
---
 .../Classes/Typolink/PageLinkBuilder.php      |  4 +-
 .../Unit/Typolink/PageLinkBuilderTest.php     | 50 +++++++++++--------
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php b/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
index f08601d80d9d..01bfa597e6e7 100644
--- a/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
+++ b/typo3/sysext/frontend/Classes/Typolink/PageLinkBuilder.php
@@ -775,8 +775,8 @@ class PageLinkBuilder extends AbstractTypolinkBuilder
             $currentQueryArray = array_replace_recursive($pageArguments->getQueryArguments(), $currentQueryArray);
         }
         if ($configuration['exclude'] ?? false) {
-            $excludeString = str_replace(',', '&', $configuration['exclude']);
-            $excludedQueryParts = [];
+            $excludeItems = array_map(urlencode(...), GeneralUtility::trimExplode(',', $configuration['exclude']));
+            $excludeString = implode('&', $excludeItems);
             parse_str($excludeString, $excludedQueryParts);
             $newQueryArray = ArrayUtility::arrayDiffKeyRecursive($currentQueryArray, $excludedQueryParts);
         } else {
diff --git a/typo3/sysext/frontend/Tests/Unit/Typolink/PageLinkBuilderTest.php b/typo3/sysext/frontend/Tests/Unit/Typolink/PageLinkBuilderTest.php
index fe479d3409bf..0804e821e8c1 100644
--- a/typo3/sysext/frontend/Tests/Unit/Typolink/PageLinkBuilderTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/Typolink/PageLinkBuilderTest.php
@@ -25,45 +25,51 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
 final class PageLinkBuilderTest extends UnitTestCase
 {
+    public static function getQueryArgumentsExcludesParametersDataProvider(): \Generator
+    {
+        $enc = self::rawUrlEncodeSquareBracketsInUrl(...);
+        yield 'nested exclude from untrusted args' => [
+            $enc('&key1=value1&key2=value2&key3[key31]=value31&key3[key32][key321]=value321&key3[key32][key322]=value322'),
+            'untrusted',
+            [
+                'exclude' => implode(',', ['key1', 'key3[key31]', 'key3[key32][key321]']),
+            ],
+            $enc('&key2=value2&key3[key32][key322]=value322'),
+        ];
+        yield 'URL encoded value' => [
+            '&param=1&param%25=2&param%2525=3',
+            'untrusted',
+            [
+                // internally: URL-decoded representation
+                'exclude' => 'param,param%,param%25',
+            ],
+            '',
+        ];
+    }
+
     /**
      * @test
+     * @dataProvider getQueryArgumentsExcludesParametersDataProvider
      */
-    public function getQueryArgumentsExcludesParameters(): void
+    public function getQueryArgumentsExcludesParameters(string $queryString, string $queryInformation, array $configuration, string $expectedResult): void
     {
-        $queryParameters = [
-            'key1' => 'value1',
-            'key2' => 'value2',
-            'key3' => [
-                'key31' => 'value31',
-                'key32' => [
-                    'key321' => 'value321',
-                    'key322' => 'value322',
-                ],
-            ],
-        ];
+        parse_str($queryString, $queryParameters);
         $request = new ServerRequest('https://example.com');
         $request = $request->withQueryParams($queryParameters);
         $request = $request->withAttribute('routing', new PageArguments(1, '', $queryParameters, [], []));
-        $configuration = [];
-        $configuration['exclude'] = [];
-        $configuration['exclude'][] = 'key1';
-        $configuration['exclude'][] = 'key3[key31]';
-        $configuration['exclude'][] = 'key3[key32][key321]';
-        $configuration['exclude'] = implode(',', $configuration['exclude']);
-        $expectedResult = $this->rawUrlEncodeSquareBracketsInUrl('&key2=value2&key3[key32][key322]=value322');
         $GLOBALS['TSFE'] = new \stdClass();
         $cObj = new ContentObjectRenderer();
         $cObj->setRequest($request);
         $subject = $this->getAccessibleMock(PageLinkBuilder::class, null, [], '', false);
         $subject->_set('contentObjectRenderer', $cObj);
-        $actualResult = $subject->_call('getQueryArguments', 'untrusted', $configuration);
+        $actualResult = $subject->_call('getQueryArguments', $queryInformation, $configuration);
         self::assertEquals($expectedResult, $actualResult);
     }
 
     /**
-     * Encodes square brackets in URL.
+     * Encodes square brackets in URL for a better readability in these tests.
      */
-    private function rawUrlEncodeSquareBracketsInUrl(string $string): string
+    private static function rawUrlEncodeSquareBracketsInUrl(string $string): string
     {
         return str_replace(['[', ']'], ['%5B', '%5D'], $string);
     }
-- 
GitLab