From 063417cb7d0a10c1caf281d6aa867e8414663860 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Fri, 28 Sep 2018 00:46:52 +0200
Subject: [PATCH] [TASK] Deprecate TSFE->makeCacheHash()

The functionality is moved into a new PSR-15 middleware
to base the logic on the request object directly, and
to make the validation more flexible when validating
page parameters for site-based routing.

The previous deprecation to add the request object
to the method has been reverted.

Resolves: #86411
Releases: master
Change-Id: I294fae7e7c0f9eb1e128a88238dabdd8ed27619f
Reviewed-on: https://review.typo3.org/58420
Reviewed-by: Jan Helke <typo3@helke.de>
Tested-by: Jan Helke <typo3@helke.de>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Reviewed-by: Frank Naegler <frank.naegler@typo3.org>
Tested-by: Frank Naegler <frank.naegler@typo3.org>
---
 ...ralTypoScriptFrontendControllerMethods.rst |   1 -
 .../Deprecation-86411-TSFE-makeCacheHash.rst  |  35 +++++
 .../TypoScriptFrontendController.php          |  20 +--
 .../Middleware/PageParameterValidator.php     | 124 ++++++++++++++++++
 .../Classes/Middleware/PageResolver.php       |   3 -
 .../Classes/Page/CacheHashCalculator.php      |   2 +-
 .../Configuration/RequestMiddlewares.php      |  11 +-
 .../Unit/Middleware/PageResolverTest.php      |   2 +-
 .../Php/MethodArgumentRequiredMatcher.php     |   7 -
 .../Php/MethodCallMatcher.php                 |   7 +
 10 files changed, 185 insertions(+), 27 deletions(-)
 create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Deprecation-86411-TSFE-makeCacheHash.rst
 create mode 100644 typo3/sysext/frontend/Classes/Middleware/PageParameterValidator.php

diff --git a/typo3/sysext/core/Documentation/Changelog/9.4/Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst b/typo3/sysext/core/Documentation/Changelog/9.4/Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst
index cf147d08fd6e..60ed1b28f335 100644
--- a/typo3/sysext/core/Documentation/Changelog/9.4/Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst
+++ b/typo3/sysext/core/Documentation/Changelog/9.4/Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst
@@ -11,7 +11,6 @@ Description
 
 The following public methods within :php:`TypoScriptFrontendController` now expect an argument:
 
-* :php:`makeCacheHash(ServerRequestInterface $request)`
 * :php:`calculateLinkVars(array $queryParams)`
 * :php:`preparePageContentGeneration(ServerRequestInterface $request)`
 
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-86411-TSFE-makeCacheHash.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-86411-TSFE-makeCacheHash.rst
new file mode 100644
index 000000000000..7b63fb155427
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-86411-TSFE-makeCacheHash.rst
@@ -0,0 +1,35 @@
+.. include:: ../../Includes.txt
+
+===========================================
+Deprecation: #86411 - TSFE->makeCacheHash()
+===========================================
+
+See :issue:`86411`
+
+Description
+===========
+
+The method :php:`TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->makeCacheHash()`
+which acts for validating the `&cHash` GET parameter against other given GET parameters
+has been marked as deprecated, as this functionality has been moved into a PSR-15 middleware.
+
+
+Impact
+======
+
+Calling the method directly will trigger a deprecation message.
+
+
+Affected Installations
+======================
+
+TYPO3 installations with extensions calling the PHP method directly.
+
+
+Migration
+=========
+
+Ensure to use the PSR-15 middleware stack with the PageParameterValidator in use to verify a
+given cHash signature against given query parameters.
+
+.. index:: Frontend, FullyScanned
\ No newline at end of file
diff --git a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
index fe3f4a7fff5e..bf72ff23e13a 100644
--- a/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
+++ b/typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
@@ -428,7 +428,7 @@ class TypoScriptFrontendController implements LoggerAwareInterface
     public $forceTemplateParsing = false;
 
     /**
-     * The array which cHash_calc is based on, see ->makeCacheHash().
+     * The array which cHash_calc is based on, see PageParameterValidator class.
      * @var array
      */
     public $cHash_array = [];
@@ -2229,22 +2229,16 @@ class TypoScriptFrontendController implements LoggerAwareInterface
      * This is used to cache pages with more parameters than just id and type.
      *
      * @see reqCHash()
-     * @param ServerRequestInterface $request
+     * @deprecated since TYPO3 v9.5, will be removed in TYPO3 v10.0. This validation is done in the PageParameterValidator PSR-15 middleware.
      */
-    public function makeCacheHash(ServerRequestInterface $request = null)
+    public function makeCacheHash()
     {
-        if ($request === null) {
-            trigger_error('TSFE->makeCacheHash() requires a ServerRequestInterface as first argument, add this argument in order to avoid this deprecation error.', E_USER_DEPRECATED);
-        }
+        trigger_error('TSFE->makeCacheHash() will be removed in TYPO3 v10.0, as this is now handled in the PSR-15 middleware.', E_USER_DEPRECATED);
         // No need to test anything if caching was already disabled.
         if ($this->no_cache && !$GLOBALS['TYPO3_CONF_VARS']['FE']['pageNotFoundOnCHashError']) {
             return;
         }
-        if ($request === null) {
-            $GET = GeneralUtility::_GET();
-        } else {
-            $GET = $request->getQueryParams();
-        }
+        $GET = GeneralUtility::_GET();
         if ($this->cHash && is_array($GET)) {
             // Make sure we use the page uid and not the page alias
             $GET['id'] = $this->id;
@@ -2272,9 +2266,9 @@ class TypoScriptFrontendController implements LoggerAwareInterface
 
     /**
      * Will disable caching if the cHash value was not set.
-     * This function should be called to check the _existence_ of "&cHash" whenever a plugin generating cacheable output is using extra GET variables. If there _is_ a cHash value the validation of it automatically takes place in makeCacheHash() (see above)
+     * This function should be called to check the _existence_ of "&cHash" whenever a plugin generating cacheable output is using extra GET variables. If there _is_ a cHash value the validation of it automatically takes place in \TYPO3\CMS\Frontend\Middleware\PageParameterValidator
      *
-     * @see makeCacheHash(), \TYPO3\CMS\Frontend\Plugin\AbstractPlugin::pi_cHashCheck()
+     * @see \TYPO3\CMS\Frontend\Plugin\AbstractPlugin::pi_cHashCheck()
      */
     public function reqCHash()
     {
diff --git a/typo3/sysext/frontend/Classes/Middleware/PageParameterValidator.php b/typo3/sysext/frontend/Classes/Middleware/PageParameterValidator.php
new file mode 100644
index 000000000000..d4b7d7387051
--- /dev/null
+++ b/typo3/sysext/frontend/Classes/Middleware/PageParameterValidator.php
@@ -0,0 +1,124 @@
+<?php
+declare(strict_types = 1);
+
+namespace TYPO3\CMS\Frontend\Middleware;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use TYPO3\CMS\Core\TimeTracker\TimeTracker;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+use TYPO3\CMS\Frontend\Controller\ErrorController;
+use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController;
+use TYPO3\CMS\Frontend\Page\CacheHashCalculator;
+use TYPO3\CMS\Frontend\Page\PageAccessFailureReasons;
+
+/**
+ * This middleware validates given request parameters.
+ */
+class PageParameterValidator implements MiddlewareInterface
+{
+
+    /**
+     * The cHash Service class used for cHash related functionality
+     *
+     * @var CacheHashCalculator
+     */
+    protected $cacheHashCalculator;
+
+    /**
+     * @var TypoScriptFrontendController
+     */
+    protected $controller;
+
+    /**
+     * @param TypoScriptFrontendController|null $controller
+     */
+    public function __construct(TypoScriptFrontendController $controller = null)
+    {
+        $this->controller = $controller ?? $GLOBALS['TSFE'];
+        $this->cacheHashCalculator = GeneralUtility::makeInstance(CacheHashCalculator::class);
+    }
+
+    /**
+     * Validates the &cHash parameter against the other $queryParameters / GET parameters
+     *
+     * @param ServerRequestInterface $request
+     * @param RequestHandlerInterface $handler
+     * @return ResponseInterface
+     */
+    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
+    {
+        $pageNotFoundOnValidationError = (bool)($GLOBALS['TYPO3_CONF_VARS']['FE']['pageNotFoundOnCHashError'] ?? true);
+        if ($this->controller->no_cache && !$pageNotFoundOnValidationError) {
+            // No need to test anything if caching was already disabled.
+        } else {
+            // Evaluate the cache hash parameter
+            $queryParams = $request->getQueryParams();
+            if (!empty($queryParams) && !$this->evaluateCacheHashParameter($queryParams, $pageNotFoundOnValidationError)) {
+                return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
+                    $request,
+                    'Request parameters could not be validated (&cHash comparison failed)',
+                    ['code' => PageAccessFailureReasons::CACHEHASH_COMPARISON_FAILED]
+                );
+            }
+        }
+        return $handler->handle($request);
+    }
+
+    /**
+     * Calculates a hash string based on additional parameters in the url.
+     *
+     * Calculated hash is stored in $this->controller->cHash_array.
+     * This is used to cache pages with more parameters than just id and type.
+     *
+     * @see TypoScriptFrontendController::reqCHash()
+     * @param array $queryParams GET parameters
+     * @param bool $pageNotFoundOnCacheHashError see $GLOBALS['TYPO3_CONF_VARS']['FE']['pageNotFoundOnCHashError']
+     * @return bool if false, then a PageNotFound response is triggered
+     */
+    protected function evaluateCacheHashParameter(array $queryParams, bool $pageNotFoundOnCacheHashError): bool
+    {
+        if ($this->controller->cHash) {
+            // Make sure we use the page uid and not the page alias
+            $queryParams['id'] = $this->controller->id;
+            $this->controller->cHash_array = $this->cacheHashCalculator->getRelevantParameters(GeneralUtility::implodeArrayForUrl('', $queryParams));
+            $cHash_calc = $this->cacheHashCalculator->calculateCacheHash($this->controller->cHash_array);
+            if (!hash_equals($cHash_calc, $this->controller->cHash)) {
+                // Early return to trigger the error controller
+                if ($pageNotFoundOnCacheHashError) {
+                    return false;
+                }
+                $this->controller->no_cache = true;
+                $this->getTimeTracker()->setTSlogMessage('The incoming cHash "' . $this->controller->cHash . '" and calculated cHash "' . $cHash_calc . '" did not match, so caching was disabled. The fieldlist used was "' . implode(',', array_keys($this->controller->cHash_array)) . '"', 2);
+            }
+            // No cHash is set, check if that is correct
+        } elseif ($this->cacheHashCalculator->doParametersRequireCacheHash(GeneralUtility::implodeArrayForUrl('', $queryParams))) {
+            // Will disable caching
+            $this->controller->reqCHash();
+        }
+        return true;
+    }
+
+    /**
+     * @return TimeTracker
+     */
+    protected function getTimeTracker(): TimeTracker
+    {
+        return GeneralUtility::makeInstance(TimeTracker::class);
+    }
+}
diff --git a/typo3/sysext/frontend/Classes/Middleware/PageResolver.php b/typo3/sysext/frontend/Classes/Middleware/PageResolver.php
index f2ec18484ba1..fa99c7bddf2d 100644
--- a/typo3/sysext/frontend/Classes/Middleware/PageResolver.php
+++ b/typo3/sysext/frontend/Classes/Middleware/PageResolver.php
@@ -140,9 +140,6 @@ class PageResolver implements MiddlewareInterface
             $this->controller->determineId();
         }
 
-        // Evaluate the cache hash parameter
-        $this->controller->makeCacheHash($request);
-
         return $handler->handle($request);
     }
 
diff --git a/typo3/sysext/frontend/Classes/Page/CacheHashCalculator.php b/typo3/sysext/frontend/Classes/Page/CacheHashCalculator.php
index 7646d4cb0d45..d3ad34cf46e5 100644
--- a/typo3/sysext/frontend/Classes/Page/CacheHashCalculator.php
+++ b/typo3/sysext/frontend/Classes/Page/CacheHashCalculator.php
@@ -108,7 +108,7 @@ class CacheHashCalculator implements SingletonInterface
      * @param string $queryString Query-parameters: "&xxx=yyy&zzz=uuu
      * @return array Array with key/value pairs of query-parameters WITHOUT a certain list of
      * @throws \RuntimeException
-     * @see \TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController::makeCacheHash(), \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::typoLink()
+     * @see \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer::typoLink()
      */
     public function getRelevantParameters($queryString)
     {
diff --git a/typo3/sysext/frontend/Configuration/RequestMiddlewares.php b/typo3/sysext/frontend/Configuration/RequestMiddlewares.php
index c331fbe49e48..a3de66f10193 100644
--- a/typo3/sysext/frontend/Configuration/RequestMiddlewares.php
+++ b/typo3/sysext/frontend/Configuration/RequestMiddlewares.php
@@ -106,11 +106,20 @@ return [
                 'typo3/cms-frontend/site',
             ]
         ],
+        'typo3/cms-frontend/page-parameter-validator' => [
+            'target' => \TYPO3\CMS\Frontend\Middleware\PageParameterValidator::class,
+            'after' => [
+                'typo3/cms-frontend/page-resolver',
+            ],
+            'before' => [
+                'typo3/cms-frontend/prepare-tsfe-rendering',
+            ]
+        ],
         /** internal: do not use or reference this middleware in your own code, as this will be possibly be removed */
         'typo3/cms-frontend/prepare-tsfe-rendering' => [
             'target' => \TYPO3\CMS\Frontend\Middleware\PrepareTypoScriptFrontendRendering::class,
             'after' => [
-                'typo3/cms-frontend/page-resolver',
+                'typo3/cms-frontend/page-parameter-validator',
             ]
         ],
         /** internal: do not use or reference this middleware in your own code, as this will be possibly be removed */
diff --git a/typo3/sysext/frontend/Tests/Unit/Middleware/PageResolverTest.php b/typo3/sysext/frontend/Tests/Unit/Middleware/PageResolverTest.php
index 665c18b0bbce..bf410f74ad6e 100644
--- a/typo3/sysext/frontend/Tests/Unit/Middleware/PageResolverTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/Middleware/PageResolverTest.php
@@ -55,7 +55,7 @@ class PageResolverTest extends UnitTestCase
 
     protected function setUp(): void
     {
-        $this->controller = $this->getAccessibleMock(TypoScriptFrontendController::class, ['getSiteScript', 'makeCacheHash', 'determineId', 'isBackendUserLoggedIn'], [], '', false);
+        $this->controller = $this->getAccessibleMock(TypoScriptFrontendController::class, ['getSiteScript', 'determineId', 'isBackendUserLoggedIn'], [], '', false);
 
         // A request handler which expects a site with some more details are found.
         $this->responseOutputHandler = new class implements RequestHandlerInterface {
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentRequiredMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentRequiredMatcher.php
index 4bfe58e2742a..581afcf0ce06 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentRequiredMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentRequiredMatcher.php
@@ -21,13 +21,6 @@ return [
             'Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst'
         ],
     ],
-    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->makeCacheHash' => [
-        'numberOfMandatoryArguments' => 1,
-        'maximumNumberOfArguments' => 1,
-        'restFiles' => [
-            'Deprecation-86046-AdditionalArgumentsInSeveralTypoScriptFrontendControllerMethods.rst'
-        ],
-    ],
     'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->preparePageContentGeneration' => [
         'numberOfMandatoryArguments' => 1,
         'maximumNumberOfArguments' => 1,
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
index b3a715d02237..232597970349 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodCallMatcher.php
@@ -3572,4 +3572,11 @@ return [
             'Deprecation-86406-TCATypeGroupInternal_typeFileAndFile_reference.rst',
         ],
     ],
+    'TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController->makeCacheHash' => [
+        'numberOfMandatoryArguments' => 0,
+        'maximumNumberOfArguments' => 0,
+        'restFiles' => [
+            'Deprecation-86411-TSFE-makeCacheHash.rst'
+        ],
+    ],
 ];
-- 
GitLab