[TASK] Streamline Page Argument merge strategies 07/58507/2
authorBenni Mack <benni@typo3.org>
Sun, 30 Sep 2018 17:36:18 +0000 (19:36 +0200)
committerFrank Naegler <frank.naegler@typo3.org>
Sun, 30 Sep 2018 18:58:30 +0000 (20:58 +0200)
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>
typo3/sysext/core/Classes/Routing/PageArguments.php
typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
typo3/sysext/frontend/Classes/Middleware/PageResolver.php
typo3/sysext/frontend/Classes/Middleware/PrepareTypoScriptFrontendRendering.php

index 58e12f5..996f4f6 100644 (file)
@@ -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
     {
index f88dd4b..7f0a18c 100644 (file)
@@ -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 {
index bd48da6..34018af 100644 (file)
@@ -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;
index 7987b93..b419c73 100644 (file)
@@ -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');