From 8950e266fde7c1690496edf610b00bc6889c1b81 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Sun, 30 Sep 2018 19:36:18 +0200
Subject: [PATCH] [TASK] Streamline Page Argument merge strategies

PageArguments are fetched and added on top of PSR-7 request
queryParams right after they are validated from the PageRouter.

They are also re-populated after config.defaultGetVars has
modified global state.

But they do not need to be set to TSFE again within the
the PageArgumentValidator middleware.

Resolves: #86483
Releases: master
Change-Id: I03df4223832845038d4207417cfcab7cbcc687dc
Reviewed-on: https://review.typo3.org/58507
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
---
 .../core/Classes/Routing/PageArguments.php    |  1 +
 .../Middleware/PageArgumentValidator.php      |  3 ---
 .../Classes/Middleware/PageResolver.php       | 19 +++++++++++++------
 .../PrepareTypoScriptFrontendRendering.php    |  7 +------
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/typo3/sysext/core/Classes/Routing/PageArguments.php b/typo3/sysext/core/Classes/Routing/PageArguments.php
index 58e12f5ec2ef..996f4f696bd0 100644
--- a/typo3/sysext/core/Classes/Routing/PageArguments.php
+++ b/typo3/sysext/core/Classes/Routing/PageArguments.php
@@ -144,6 +144,7 @@ class PageArguments implements RouteResultInterface
     /**
      * @param array $queryArguments
      * @return static
+     * @internal this is internal due to the issue that a PageArgument should not be modified, but must be within TYPO3 Core currently.
      */
     public function withQueryArguments(array $queryArguments): self
     {
diff --git a/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php b/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
index f88dd4b5755e..7f0a18c7a6e9 100644
--- a/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
+++ b/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
@@ -66,9 +66,6 @@ class PageArgumentValidator implements MiddlewareInterface
     {
         $pageNotFoundOnValidationError = (bool)($GLOBALS['TYPO3_CONF_VARS']['FE']['pageNotFoundOnCHashError'] ?? true);
         $pageArguments = $request->getAttribute('routing', null);
-        if ($pageArguments instanceof PageArguments) {
-            $this->controller->setPageArguments($pageArguments);
-        }
         if ($this->controller->no_cache && !$pageNotFoundOnValidationError) {
             // No need to test anything if caching was already disabled.
         } else {
diff --git a/typo3/sysext/frontend/Classes/Middleware/PageResolver.php b/typo3/sysext/frontend/Classes/Middleware/PageResolver.php
index bd48da61dc81..34018af9b549 100644
--- a/typo3/sysext/frontend/Classes/Middleware/PageResolver.php
+++ b/typo3/sysext/frontend/Classes/Middleware/PageResolver.php
@@ -89,7 +89,7 @@ class PageResolver implements MiddlewareInterface
             $requestId = (string)($request->getQueryParams()['id'] ?? '');
             if (!empty($requestId) && !empty($page = $this->resolvePageId($requestId))) {
                 // Legacy URIs (?id=12345) takes precedence, not matter if a route is given
-                $routeResult = new PageArguments(
+                $pageArguments = new PageArguments(
                     (int)($page['l10n_parent'] ?: $page['uid']),
                     [],
                     [],
@@ -97,9 +97,9 @@ class PageResolver implements MiddlewareInterface
                 );
             } else {
                 // Check for the route
-                $routeResult = $site->getRouter()->matchRequest($request, $previousResult);
+                $pageArguments = $site->getRouter()->matchRequest($request, $previousResult);
             }
-            if ($routeResult === null || !$routeResult->getPageId()) {
+            if ($pageArguments === null || !$pageArguments->getPageId()) {
                 return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
                     $request,
                     'The requested page does not exist',
@@ -107,16 +107,23 @@ class PageResolver implements MiddlewareInterface
                 );
             }
 
-            $this->controller->id = $routeResult->getPageId();
-            $request = $request->withAttribute('routing', $routeResult);
+            $this->controller->id = $pageArguments->getPageId();
+            $this->controller->type = $pageArguments->getArguments()['type'] ?? $this->controller->type;
+            $request = $request->withAttribute('routing', $pageArguments);
             // stop in case arguments are dirty (=defined twice in route and GET query parameters)
-            if ($routeResult->areDirty()) {
+            if ($pageArguments->areDirty()) {
                 return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
                     $request,
                     'The requested URL is not distinct',
                     ['code' => PageAccessFailureReasons::PAGE_NOT_FOUND]
                 );
             }
+
+            // merge the PageArguments with the request query parameters
+            $queryParams = array_replace_recursive($request->getQueryParams(), $pageArguments->getArguments());
+            $request = $request->withQueryParams($queryParams);
+            $this->controller->setPageArguments($pageArguments);
+
             // At this point, we later get further route modifiers
             // for bw-compat we update $GLOBALS[TYPO3_REQUEST] to be used later in TSFE.
             $GLOBALS['TYPO3_REQUEST'] = $request;
diff --git a/typo3/sysext/frontend/Classes/Middleware/PrepareTypoScriptFrontendRendering.php b/typo3/sysext/frontend/Classes/Middleware/PrepareTypoScriptFrontendRendering.php
index 7987b935f0fd..b419c737d873 100644
--- a/typo3/sysext/frontend/Classes/Middleware/PrepareTypoScriptFrontendRendering.php
+++ b/typo3/sysext/frontend/Classes/Middleware/PrepareTypoScriptFrontendRendering.php
@@ -77,6 +77,7 @@ class PrepareTypoScriptFrontendRendering implements MiddlewareInterface
             $modifiedGetVars = GeneralUtility::removeDotsFromTS($this->controller->config['config']['defaultGetVars.']);
             if ($pageArguments instanceof PageArguments) {
                 $pageArguments = $pageArguments->withQueryArguments($modifiedGetVars);
+                $this->controller->setPageArguments($pageArguments);
                 $request = $request->withAttribute('routing', $pageArguments);
             }
             if (!empty($request->getQueryParams())) {
@@ -85,12 +86,6 @@ class PrepareTypoScriptFrontendRendering implements MiddlewareInterface
             $request = $request->withQueryParams($modifiedGetVars);
             $GLOBALS['TYPO3_REQUEST'] = $request;
         }
-        // Populate internal route query arguments to super global $_GET
-        if ($pageArguments instanceof PageArguments) {
-            $_GET = $pageArguments->getArguments();
-            $GLOBALS['HTTP_GET_VARS'] = $pageArguments->getArguments();
-            $this->controller->setPageArguments($pageArguments);
-        }
 
         // Setting language and locale
         $this->timeTracker->push('Setting language and locale');
-- 
GitLab