[BUGFIX] Correctly build query string without cHash

For building URIs using the Symfony routing, array URL arguments are deflated (foo[bar] -> foo__bar). In case the final URI contains a remaining query string, the arguments get inflated again. However the query string of the URI is currently only updated, when at least one of the remaining arguments require a cHash. In case remaining arguments exist, the query string needs to be always updated, whether the URI requires a cHash or not. Resolves: #92183 Releases: master, 10.4, 9.5 Change-Id: I1966b4b1e8e09716733b3357922cf857543c14e4 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65595 Tested-by: TYPO3com <noreply@typo3.com> Tested-by: Helmut Hummel <typo3@helhum.io> Reviewed-by: Helmut Hummel <typo3@helhum.io>
TYPO3 · Sep 7, 2020 · e5066bc · e5066bc
1 parent f1e4427
commit e5066bc
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 3 deletions.
diff --git a/typo3/sysext/core/Classes/Routing/PageRouter.php b/typo3/sysext/core/Classes/Routing/PageRouter.php
@@ -365,11 +365,11 @@ public function generateUri($route, array $parameters = [], string $fragment = '
         if ($matchedRoute && $pageRouteResult && !empty($pageRouteResult->getDynamicArguments())) {
             $cacheHash = $this->generateCacheHash($pageId, $pageRouteResult);
 
+            $queryArguments = $pageRouteResult->getQueryArguments();
             if (!empty($cacheHash)) {
-                $queryArguments = $pageRouteResult->getQueryArguments();
                 $queryArguments['cHash'] = $cacheHash;
-                $uri = $uri->withQuery(http_build_query($queryArguments, '', '&', PHP_QUERY_RFC3986));
             }
+            $uri = $uri->withQuery(http_build_query($queryArguments, '', '&', PHP_QUERY_RFC3986));
         }
         if ($fragment) {
             $uri = $uri->withFragment($fragment);

diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/AbstractTestCase.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/AbstractTestCase.php
@@ -37,7 +37,8 @@ abstract class AbstractTestCase extends FunctionalTestCase
         ],
         'FE' => [
             'cacheHash' => [
-                'requireCacheHashPresenceParameters' => ['value', 'testing[value]', 'tx_testing_link[value]']
+                'requireCacheHashPresenceParameters' => ['value', 'testing[value]', 'tx_testing_link[value]'],
+                'excludedParameters' => ['tx_testing_link[excludedValue]']
             ],
         ]
     ];

diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/EnhancerLinkGeneratorTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/EnhancerLinkGeneratorTest.php
@@ -1095,4 +1095,89 @@ public function defaultExtbaseControllerActionNamesAreApplied(string $additional
 
         self::assertSame($expectation, (string)$response->getBody());
     }
+
+    /**
+     * @return array
+     */
+    public function defaultExtbaseControllerActionNamesAreAppliedWithAdditionalNonMappedQueryArgumentsDataProvider(): array
+    {
+        return [
+            '*::*' => [
+                '&tx_testing_link[value]=1&tx_testing_link[excludedValue]=random',
+                'https://acme.us/welcome/link/index/one?tx_testing_link%5BexcludedValue%5D=random'
+            ],
+            '*::list' => [
+                '&tx_testing_link[action]=list&tx_testing_link[value]=1&tx_testing_link[excludedValue]=random',
+                'https://acme.us/welcome/link/list/one?tx_testing_link%5BexcludedValue%5D=random'
+            ],
+            'Link::*' => [
+                // correctly falling back to defaultController here
+                '&tx_testing_link[controller]=Link&tx_testing_link[value]=1&tx_testing_link[excludedValue]=random',
+                'https://acme.us/welcome/link/index/one?tx_testing_link%5BexcludedValue%5D=random'
+            ],
+            'Page::*' => [
+                // correctly falling back to defaultController here
+                '&tx_testing_link[controller]=Page&tx_testing_link[value]=1&tx_testing_link[excludedValue]=random',
+                'https://acme.us/welcome/link/index/one?tx_testing_link%5BexcludedValue%5D=random'
+            ],
+            'Page::show' => [
+                '&tx_testing_link[controller]=Page&tx_testing_link[action]=show&tx_testing_link[value]=1&tx_testing_link[excludedValue]=random',
+                'https://acme.us/welcome/page/show/one?tx_testing_link%5BexcludedValue%5D=random'
+            ],
+        ];
+    }
+
+    /**
+     * Tests whether ExtbasePluginEnhancer applies `defaultController` values correctly but keeps additional Query Parameters.
+     *
+     * @param string $additionalParameters
+     * @param string $expectation
+     *
+     * @test
+     *
+     * @dataProvider defaultExtbaseControllerActionNamesAreAppliedWithAdditionalNonMappedQueryArgumentsDataProvider
+     */
+    public function defaultExtbaseControllerActionNamesAreAppliedWithAdditionalNonMappedQueryArguments(string $additionalParameters, string $expectation)
+    {
+        $targetLanguageId = 0;
+        $this->mergeSiteConfiguration('acme-com', [
+            'routeEnhancers' => [
+                'Enhancer' => [
+                    'type' => 'Extbase',
+                    'routes' => [
+                        ['routePath' => '/link/index/{value}', '_controller' => 'Link::index'],
+                        ['routePath' => '/link/list/{value}',  '_controller' => 'Link::list'],
+                        ['routePath' => '/page/show/{value}', '_controller' => 'Page::show'],
+                    ],
+                    'defaultController' => 'Link::index',
+                    'extension' => 'testing',
+                    'plugin' => 'link',
+                    'aspects' => [
+                        'value' => [
+                            'type' => 'StaticValueMapper',
+                            'map' => [
+                                'one' => 1,
+                            ],
+                        ],
+                    ],
+                ]
+            ]
+        ]);
+
+        $response = $this->executeFrontendRequest(
+            (new InternalRequest('https://acme.us/'))
+                ->withPageId(1100)
+                ->withInstructions([
+                    $this->createTypoLinkUrlInstruction([
+                        'parameter' => 1100,
+                        'language' => $targetLanguageId,
+                        'additionalParams' => $additionalParameters,
+                        'forceAbsoluteUrl' => 1,
+                    ])
+                ]),
+            $this->internalRequestContext
+        );
+
+        self::assertSame($expectation, (string)$response->getBody());
+    }
 }