From ed60c2faea9a2ffa03f9e43dd56befb596c5fe13 Mon Sep 17 00:00:00 2001
From: Oliver Hader <oliver@typo3.org>
Date: Fri, 21 May 2021 12:03:13 +0200
Subject: [PATCH] [BUGFIX] Correctly resolve nested arguments in SimpleEnhancer

Having `additionalParams = &simple[id]=okay&simple[other]=other`
with the following route enhancer configuration lead to some flaws:

    routeEnhancers:
      TestSimple:
        type: Simple
        routePath: '/simple/{simple_id}'
        _arguments:
          simple_id: 'simple/id'

* generated URI contained `simple__other`, not resolving nesting
  (`/simple/okay?simple__other=other&cHash=...`)
* `PageArguments::$staticArguments` contained parameter `simple_id`
  (unresolved to actual query parameter, incorrectly marked static)
* other `PageArguments` properties contained `simple__other`
  (unresolved to actual query parameter, ignored nesting)

Resolves: #91447
Releases: master, 10.4, 9.5
Change-Id: If96a6245f44a12d6e666d0ead8b1cc9cbbb43170
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/69227
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
---
 .../Routing/Enhancer/SimpleEnhancer.php       | 10 +++---
 .../Fixtures/LinkHandlingController.php       | 13 ++++++--
 .../Framework/Builder/Builder.php             |  5 +++
 .../SiteHandling/TestSetDataProviderTrait.php | 31 ++++++++++++-------
 4 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/typo3/sysext/core/Classes/Routing/Enhancer/SimpleEnhancer.php b/typo3/sysext/core/Classes/Routing/Enhancer/SimpleEnhancer.php
index 745b8b718987..7af253021371 100644
--- a/typo3/sysext/core/Classes/Routing/Enhancer/SimpleEnhancer.php
+++ b/typo3/sysext/core/Classes/Routing/Enhancer/SimpleEnhancer.php
@@ -51,23 +51,22 @@ class SimpleEnhancer extends AbstractEnhancer implements RoutingEnhancerInterfac
      */
     public function buildResult(Route $route, array $results, array $remainingQueryParameters = []): PageArguments
     {
-        $variableProcessor = $this->getVariableProcessor();
         // determine those parameters that have been processed
         $parameters = array_intersect_key(
             $results,
             array_flip($route->compile()->getPathVariables())
         );
         // strip of those that where not processed (internals like _route, etc.)
+        $internals = array_diff_key($results, $parameters);
         $matchedVariableNames = array_keys($parameters);
 
         $staticMappers = $route->filterAspects([StaticMappableAspectInterface::class], $matchedVariableNames);
         $dynamicCandidates = array_diff_key($parameters, $staticMappers);
 
         // all route arguments
-        $routeArguments = $variableProcessor->inflateParameters($parameters, $route->getArguments());
+        $routeArguments = $this->inflateParameters($parameters, $internals);
         // dynamic arguments, that don't have a static mapper
-        $dynamicArguments = $variableProcessor
-            ->inflateNamespaceParameters($dynamicCandidates, '');
+        $dynamicArguments = $this->inflateParameters($dynamicCandidates);
         // route arguments, that don't appear in dynamic arguments
         $staticArguments = ArrayUtility::arrayDiffKeyRecursive($routeArguments, $dynamicArguments);
 
@@ -142,7 +141,6 @@ class SimpleEnhancer extends AbstractEnhancer implements RoutingEnhancerInterfac
 
     public function inflateParameters(array $parameters, array $internals = []): array
     {
-        return $this->getVariableProcessor()
-            ->inflateNamespaceParameters($parameters, '');
+        return $this->getVariableProcessor()->inflateParameters($parameters, $internals);
     }
 }
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/LinkHandlingController.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/LinkHandlingController.php
index ba3033c213f3..0c3a11534cd4 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/LinkHandlingController.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Fixtures/LinkHandlingController.php
@@ -58,9 +58,11 @@ class LinkHandlingController
     }
 
     /**
+     * @param string|null $content
+     * @param array|null $configuration
      * @return string
      */
-    public function dumpPageArgumentsAction(): string
+    public function dumpPageArgumentsAction(?string $content, array $configuration = null): string
     {
         /** @var ServerRequestInterface $request */
         $request = $GLOBALS['TYPO3_REQUEST'];
@@ -68,15 +70,20 @@ class LinkHandlingController
         $pageArguments = $request->getAttribute('routing');
         /** @var SiteLanguage $language */
         $language = $request->getAttribute('language');
+        $flags = 0;
+        if ($configuration['userFunc.']['prettyPrint'] ?? true) {
+            $flags += JSON_PRETTY_PRINT;
+        }
         return json_encode([
             'pageId' => $pageArguments->getPageId(),
             'pageType' => $pageArguments->getPageType(),
             'languageId' => $language->getLanguageId(),
-            'dynamicArguments' => $pageArguments->getDynamicArguments(),
             'staticArguments' => $pageArguments->getStaticArguments(),
+            'routeArguments' => $pageArguments->getRouteArguments(),
+            'dynamicArguments' => $pageArguments->getDynamicArguments(),
             'queryArguments' => $pageArguments->getQueryArguments(),
             'requestQueryParams' => $request->getQueryParams(),
             '_GET' => $_GET,
-        ]);
+        ], $flags);
     }
 }
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/Builder.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/Builder.php
index 05daa95b18d3..a5584682fc15 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/Builder.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/Framework/Builder/Builder.php
@@ -230,6 +230,11 @@ class Builder
         $resolveArguments['staticArguments'] = $resolveArguments['staticArguments'] ?? [];
         $resolveArguments['dynamicArguments'] = $resolveArguments['dynamicArguments'] ?? [];
         $resolveArguments['queryArguments'] = $resolveArguments['queryArguments'] ?? [];
+        // generate ("assume") `routeArguments` from expected static and dynamic arguments if not declared
+        $resolveArguments['routeArguments'] = $resolveArguments['routeArguments'] ?? array_replace_recursive(
+            $resolveArguments['staticArguments'],
+            $resolveArguments['dynamicArguments']
+        );
         if (preg_match('#\?cHash=([a-z0-9]+)#i', $this->compileUrl($testSet), $matches)) {
             $resolveArguments['dynamicArguments']['cHash'] = $matches[1];
             $resolveArguments['queryArguments']['cHash'] = $matches[1];
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/TestSetDataProviderTrait.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/TestSetDataProviderTrait.php
index bd6ad6b97663..db96146801bc 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/TestSetDataProviderTrait.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/TestSetDataProviderTrait.php
@@ -61,18 +61,16 @@ trait TestSetDataProviderTrait
                         ->withRequiredDefinedVariableNames('value'),
                 ])
                 ->withResolveArguments([
+                    'routeArguments' => [
+                        'known' => ['value' => $resolveValueVar],
+                    ],
                     'dynamicArguments' => [
-                        // @todo Wrong
-                        'any__other' => 'other',
+                        'known' => ['value' => $resolveValueVar],
+                        'any' => ['other' => 'other'],
                         'cHash' => $cHashVar,
                     ],
-                    'staticArguments' => [
-                        // @todo Wrong
-                        'known_value' => 'known',
-                    ],
                     'queryArguments' => [
-                        // @todo Wrong
-                        'any__other' => 'other',
+                        'any' => ['other' => 'other'],
                         'cHash' => $cHashVar,
                     ],
                 ]),
@@ -90,6 +88,11 @@ trait TestSetDataProviderTrait
                         ->withRequiredDefinedVariableNames('value'),
                 ])
                 ->withResolveArguments([
+                    'routeArguments' => [
+                        'testing' => [
+                            'known' => ['value' => $resolveValueVar],
+                        ],
+                    ],
                     'dynamicArguments' => [
                         'testing' => [
                             'known' => ['value' => $resolveValueVar],
@@ -123,6 +126,13 @@ trait TestSetDataProviderTrait
                         ->withRequiredDefinedVariableNames('value'),
                 ])
                 ->withResolveArguments([
+                    'routeArguments' => [
+                        'tx_testing_link' => [
+                            'known' => ['value' => $resolveValueVar],
+                            'controller' => 'Link',
+                            'action' => 'index',
+                        ],
+                    ],
                     'dynamicArguments' => [
                         'tx_testing_link' => [
                             'known' => ['value' => $resolveValueVar],
@@ -166,9 +176,8 @@ trait TestSetDataProviderTrait
             ->withApplicableItems($enhancers)
             ->withApplicableSet(
                 VariablesContext::create(Variables::create([
-                    // @todo Should be '?any%5Bother%5D=other&cHash='
-                    'pathSuffix' => '?any__other=other&cHash=[[cHash]]',
-                    'cHash' => '02ee401c0a183d11d30b7a61deeb3361',
+                    'pathSuffix' => '?any%5Bother%5D=other&cHash=[[cHash]]',
+                    'cHash' => 'a655d1f1d346f7d3fa7aef5459a6547f',
                 ]))->withRequiredApplicables($enhancers['Simple']),
                 VariablesContext::create(Variables::create([
                     'pathSuffix' => '?testing%5Bany%5D%5Bother%5D=other&cHash=[[cHash]]',
-- 
GitLab