From 6c0d21343f46b9943196a643843e73e4184c8a03 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Sat, 8 Jun 2019 20:20:40 +0200
Subject: [PATCH] [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>
---
.../Middleware/PageArgumentValidator.php | 22 ++++++++++++++-----
.../Middleware/PageArgumentValidatorTest.php | 14 +++++++-----
2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php b/typo3/sysext/frontend/Classes/Middleware/PageArgumentValidator.php
index 65d64a58bdc8..6d95e54f5c21 100644
--- 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 af4fa0ff46d6..b8ac1bea1ae1 100644
--- 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';
--
GitLab