[TASK] Create Redirect instead of silently ignoring superfluous cHash

Introduced with https://review.typo3.org/c/Packages/TYPO3.CMS/+/60895
cHash's that are used within the URL but not needed
anymore, should rather create a redirect instead
of silently unsetting the cHash argument.

Related: #41033
Resolves: #88531
Releases: master, 9.5
Change-Id: Iaae3e72160c055f8848942d506f7cc3e25d31af4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/60905
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Nicole Cordes <typo3@cordes.co>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Nicole Cordes <typo3@cordes.co>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>

diff --git a/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php b/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
index 65d64a5..6d95e54 100644 (file)
--- a/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
+++ b/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
@@ -20,6 +20,9 @@ use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\MiddlewareInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerAwareTrait;
+use TYPO3\CMS\Core\Http\RedirectResponse;
 use TYPO3\CMS\Core\Routing\PageArguments;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -32,8 +35,9 @@ use TYPO3\CMS\Frontend\Page\PageAccessFailureReasons;
 /**
  * This middleware validates given request parameters against the common "cHash" functionality.
  */
-class PageArgumentValidator implements MiddlewareInterface
+class PageArgumentValidator implements MiddlewareInterface, LoggerAwareInterface
 {
+    use LoggerAwareTrait;
 
     /**
      * The cHash Service class used for cHash related functionality
@@ -77,6 +81,14 @@ class PageArgumentValidator implements MiddlewareInterface
                 $queryParams = $request->getQueryParams();
             }
             if (!empty($queryParams) && !$this->evaluateCacheHashParameter($queryParams, $pageNotFoundOnValidationError)) {
+                // cHash was given, but nothing to be calculated, so let's do a redirect to the current page
+                // but without the cHash
+                if ($this->controller->cHash && empty($this->controller->cHash_array)) {
+                    $uri = $request->getUri();
+                    unset($queryParams['cHash']);
+                    $uri = $uri->withQuery(HttpUtility::buildQueryString($queryParams));
+                    return new RedirectResponse($uri, 308);
+                }
                 return GeneralUtility::makeInstance(ErrorController::class)->pageNotFoundAction(
                     $request,
                     'Request parameters could not be validated (&cHash comparison failed)',
@@ -104,14 +116,12 @@ class PageArgumentValidator implements MiddlewareInterface
             $queryParams['id'] = $this->controller->id;
             $relevantParameters = $this->cacheHashCalculator->getRelevantParameters(HttpUtility::buildQueryString($queryParams));
             $this->controller->cHash_array = $relevantParameters;
-            $calculatedCacheHash = $this->cacheHashCalculator->calculateCacheHash($relevantParameters);
             // cHash was given, but nothing to be calculated, so cHash is unset and all is good.
             if (empty($relevantParameters)) {
-                $this->getTimeTracker()->setTSlogMessage('The incoming cHash "' . $this->controller->cHash . '" is given but not needed. cHash is unset', 2);
-                // We do not need to update the query params as everything is checked via $TSFE->cHash, see $TSFE->reqCHash()
-                $this->controller->cHash = '';
-                return true;
+                $this->logger->notice('The incoming cHash "' . $this->controller->cHash . '" is given but not needed. cHash is unset');
+                return false;
             }
+            $calculatedCacheHash = $this->cacheHashCalculator->calculateCacheHash($relevantParameters);
             if (!hash_equals($calculatedCacheHash, $this->controller->cHash)) {
                 // Early return to trigger the error controller
                 if ($pageNotFoundOnCacheHashError) {

diff --git a/typo3/sysext/frontend/Tests/Unit/Middleware/PageArgumentValidatorTest.php b/typo3/sysext/frontend/Tests/Unit/Middleware/PageArgumentValidatorTest.php
index af4fa0f..b8ac1be 100644 (file)
--- a/typo3/sysext/frontend/Tests/Unit/Middleware/PageArgumentValidatorTest.php
+++ b/typo3/sysext/frontend/Tests/Unit/Middleware/PageArgumentValidatorTest.php
@@ -19,6 +19,7 @@ namespace TYPO3\CMS\Frontend\Tests\Unit\Middleware;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use Psr\Log\NullLogger;
 use TYPO3\CMS\Core\Http\Response;
 use TYPO3\CMS\Core\Http\ServerRequest;
 use TYPO3\CMS\Core\Routing\PageArguments;
@@ -71,9 +72,10 @@ class PageArgumentValidatorTest extends UnitTestCase
     /**
      * @test
      */
-    public function givenCacheHashWithoutRequiredParametersIgnoresCacheHashAndUnsetsCacheHash(): void
+    public function givenCacheHashWithoutRequiredParametersTriggersRedirect(): void
     {
-        $incomingUrl = 'https://example.com/lotus-flower/en/mr-magpie/bloom/';
+        $incomingUrl = 'https://example.com/lotus-flower/en/mr-magpie/bloom/?cHash=XYZ';
+        $expectedResult = 'https://example.com/lotus-flower/en/mr-magpie/bloom/';
         $this->controller->id = 13;
         $this->controller->cHash = 'XYZ';
 
@@ -83,9 +85,11 @@ class PageArgumentValidatorTest extends UnitTestCase
         $request = $request->withAttribute('routing', $pageArguments);
 
         $subject = new PageArgumentValidator($this->controller);
+        $subject->setLogger(new NullLogger());
+
         $response = $subject->process($request, $this->responseOutputHandler);
-        static::assertEquals(200, $response->getStatusCode());
-        static::assertEquals('', $this->controller->cHash);
+        static::assertEquals(308, $response->getStatusCode());
+        static::assertEquals($expectedResult, $response->getHeader('Location')[0]);
     }
 
     /**
@@ -93,7 +97,7 @@ class PageArgumentValidatorTest extends UnitTestCase
      */
     public function givenCacheHashNotMatchingCalculatedCacheHashTriggers404(): void
     {
-        $incomingUrl = 'https://example.com/lotus-flower/en/mr-magpie/bloom/';
+        $incomingUrl = 'https://example.com/lotus-flower/en/mr-magpie/bloom/?cHash=YAZ';
         $this->controller->id = 13;
         $this->controller->cHash = 'XYZ';