Skip to content
Snippets Groups Projects
Commit d74d5d4d authored by Oliver Hader's avatar Oliver Hader Committed by Susanne Moog
Browse files

[BUGFIX] Avoid applying parameter inflation during route resolving

Remaining parameters need to be deflated (prepared and normalized to
strings and fitting into Symfony route length and literal constraints)
during route generation.

However, when resolving a route remaining query parameters have to be
kept as is - explained in more detail in the following examples based
on using enhancer with namespace set to 'app':

* https://example.org/page/route-value/app__value=inject
  + previously assigned `app[value]=inject` -> dirty -> rejected
  + not inflated anymore, kept as `app_value`
* https://example.org/page/route-value/app__other=inject
  + previously assigned `app[other]=inject` -> okay, wrong namespace
  + not inflated anymore, kept as `app__other`
* https://example.org/page/route-value/[32+ characters]=inject
  + arbitrary characters lead to OutOfRangeException -> no valid hash
  + previously `md5('app__@any__value')` assigned `app[@any][value]`
    -> dirty -> rejected
  + not inflated anymore, kept as `md5('app__@any__value')`

Basically the mentioned OutOfRangeException revealed that misbehavior,
hacking `VariableProcessor::resolveHash` would not have solve issues
with (incorrectly) merged query parameters - but would have hidden it.

Resolves: #87688
Releases: master, 9.5
Change-Id: I3daf02d3b4ed540b9eb098a8b116f485cc79fa72
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/62385


Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: default avatarMichael Telgkamp <michael.telgkamp@mindscreen.de>
Tested-by: default avatarSusanne Moog <look@susi.dev>
Reviewed-by: default avatarMichael Telgkamp <michael.telgkamp@mindscreen.de>
Reviewed-by: default avatarBenni Mack <benni@typo3.org>
Reviewed-by: default avatarFrank Nägler <frank.naegler@typo3.org>
parent ceafda03
Branches
Tags
No related merge requests found
<?php
declare(strict_types = 1);
namespace TYPO3\CMS\Core\Routing\Enhancer;
/*
* 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!
*/
/**
* Interface asserting that enhancer is capable of inflating parameters.
*/
interface InflatableEnhancerInterface
{
public function inflateParameters(array $parameters, array $internals = []): array;
}
......@@ -38,7 +38,7 @@ use TYPO3\CMS\Core\Utility\ArrayUtility;
* user_id: '[a-z]+'
* hash: '[a-z]{0-6}'
*/
class PluginEnhancer extends AbstractEnhancer implements RoutingEnhancerInterface, ResultingInterface
class PluginEnhancer extends AbstractEnhancer implements RoutingEnhancerInterface, InflatableEnhancerInterface, ResultingInterface
{
/**
* @var array
......@@ -81,9 +81,6 @@ class PluginEnhancer extends AbstractEnhancer implements RoutingEnhancerInterfac
->inflateNamespaceParameters($dynamicCandidates, $this->namespace);
// static arguments, that don't appear in dynamic arguments
$staticArguments = ArrayUtility::arrayDiffAssocRecursive($routeArguments, $dynamicArguments);
// inflate remaining query arguments that could not be applied to the route
$remainingQueryParameters = $variableProcessor
->inflateNamespaceParameters($remainingQueryParameters, $this->namespace);
$page = $route->getOption('_page');
$pageId = (int)($page['l10n_parent'] > 0 ? $page['l10n_parent'] : $page['uid']);
......@@ -182,7 +179,7 @@ class PluginEnhancer extends AbstractEnhancer implements RoutingEnhancerInterfac
* @param array $internals Internal instructions (_route, _controller, ...)
* @return array
*/
protected function inflateParameters(array $parameters, array $internals = []): array
public function inflateParameters(array $parameters, array $internals = []): array
{
return $this->getVariableProcessor()
->inflateNamespaceParameters($parameters, $this->namespace);
......
......@@ -33,7 +33,7 @@ use TYPO3\CMS\Core\Utility\ArrayUtility;
* category_id: 'category/id'
* scope_id: 'scope/id'
*/
class SimpleEnhancer extends AbstractEnhancer implements RoutingEnhancerInterface, ResultingInterface
class SimpleEnhancer extends AbstractEnhancer implements RoutingEnhancerInterface, InflatableEnhancerInterface, ResultingInterface
{
/**
* @var array
......@@ -69,9 +69,6 @@ class SimpleEnhancer extends AbstractEnhancer implements RoutingEnhancerInterfac
->inflateNamespaceParameters($dynamicCandidates, '');
// static arguments, that don't appear in dynamic arguments
$staticArguments = ArrayUtility::arrayDiffAssocRecursive($routeArguments, $dynamicArguments);
// inflate remaining query arguments that could not be applied to the route
$remainingQueryParameters = $variableProcessor
->inflateNamespaceParameters($remainingQueryParameters, '');
$page = $route->getOption('_page');
$pageId = (int)($page['l10n_parent'] > 0 ? $page['l10n_parent'] : $page['uid']);
......@@ -132,4 +129,10 @@ class SimpleEnhancer extends AbstractEnhancer implements RoutingEnhancerInterfac
$variant->addOptions(['deflatedParameters' => $deflatedParameters]);
$collection->add('enhancer_' . spl_object_hash($variant), $variant);
}
public function inflateParameters(array $parameters, array $internals = []): array
{
return $this->getVariableProcessor()
->inflateNamespaceParameters($parameters, '');
}
}
......@@ -33,6 +33,7 @@ use TYPO3\CMS\Core\Routing\Aspect\StaticMappableAspectInterface;
use TYPO3\CMS\Core\Routing\Enhancer\DecoratingEnhancerInterface;
use TYPO3\CMS\Core\Routing\Enhancer\EnhancerFactory;
use TYPO3\CMS\Core\Routing\Enhancer\EnhancerInterface;
use TYPO3\CMS\Core\Routing\Enhancer\InflatableEnhancerInterface;
use TYPO3\CMS\Core\Routing\Enhancer\ResultingInterface;
use TYPO3\CMS\Core\Routing\Enhancer\RoutingEnhancerInterface;
use TYPO3\CMS\Core\Site\Entity\Site;
......@@ -289,6 +290,10 @@ class PageRouter implements RouterInterface
/** @var Route $matchedRoute */
$matchedRoute = $collection->get($routeName);
parse_str($uri->getQuery() ?? '', $remainingQueryParameters);
$enhancer = $route->getEnhancer();
if ($enhancer instanceof InflatableEnhancerInterface) {
$remainingQueryParameters = $enhancer->inflateParameters($remainingQueryParameters);
}
$pageRouteResult = $this->buildPageArguments($route, $parameters, $remainingQueryParameters);
break;
} catch (MissingMandatoryParametersException $e) {
......
......@@ -154,7 +154,7 @@ class ExtbasePluginEnhancer extends PluginEnhancer
* @param array $internals Internal instructions (_route, _controller, ...)
* @return array
*/
protected function inflateParameters(array $parameters, array $internals = []): array
public function inflateParameters(array $parameters, array $internals = []): array
{
$parameters = $this->getVariableProcessor()
->inflateNamespaceParameters($parameters, $this->namespace);
......
......@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Frontend\Tests\Functional\SiteHandling;
*/
use TYPO3\CMS\Core\Core\Bootstrap;
use TYPO3\CMS\Core\Utility\ArrayUtility;
use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerFactory;
use TYPO3\TestingFramework\Core\Functional\Framework\DataHandling\Scenario\DataHandlerWriter;
use TYPO3\TestingFramework\Core\Functional\Framework\Frontend\InternalRequest;
......@@ -624,6 +625,180 @@ class EnhancerSiteRequestTest extends AbstractTestCase
self::assertEquals($expectation, $pageArguments);
}
public function routeIdentifiersAreResolvedDataProvider(): array
{
return [
// namespace[value]
'namespace[value] ? test' => [
'namespace',
'value',
'test',
],
'namespace[value] ? x^31' => [
'namespace',
'value',
str_repeat('x', 31),
],
'namespace[value] ? x^32' => [
'namespace',
'value',
str_repeat('x', 32),
],
'namespace[value] ? x^33' => [
'namespace',
'value',
str_repeat('x', 33),
],
'namespace[value] ? 1^32 (type-cast)' => [
'namespace',
'value',
str_repeat('1', 32),
],
// md5('namespace__@otne3') is 60360798585102000952995164024754 (numeric)
// md5('ximaz') is 61529519452809720693702583126814 (numeric)
'namespace[@otne3] ? numeric-md5 (type-cast)' => [
'namespace',
'@otne3',
md5('ximaz'),
],
'namespace[value] ? namespace__value' => [
'namespace',
'value',
'namespace__value',
],
'namespace[value] ? namespace/value' => [
'namespace',
'value',
'namespace/value',
'The requested URL is not distinct',
],
'namespace[value] ? namespace__other' => [
'namespace',
'value',
'namespace__other',
],
'namespace[value] ? namespace/other' => [
'namespace',
'value',
'namespace/other',
],
// namespace[any/value]
'namespace[any/value] ? x^31' => [
'namespace',
'any/value',
str_repeat('x', 31),
],
'namespace[any/value] ? x^32' => [
'namespace',
'any/value',
str_repeat('x', 32),
],
'namespace[any/value] ? namespace__any__value' => [
'namespace',
'any/value',
'namespace__any__value',
],
'namespace[any/value] ? namespace/any/value' => [
'namespace',
'any/value',
'namespace/any/value',
'The requested URL is not distinct',
],
'namespace[any/value] ? namespace__any__other' => [
'namespace',
'any/value',
'namespace__any__other',
],
'namespace[any/value] ? namespace/any/other' => [
'namespace',
'any/value',
'namespace/any/other',
],
// namespace[@any/value]
'namespace[@any/value] ? x^31' => [
'namespace',
'@any/value',
str_repeat('x', 31),
],
'namespace[@any/value] ? x^32' => [
'namespace',
'@any/value',
str_repeat('x', 32),
],
'namespace[@any/value] ? md5(namespace__@any__value)' => [
'namespace',
'@any/value',
md5('namespace__@any__value'),
],
'namespace[@any/value] ? namespace/@any/value' => [
'namespace',
'@any/value',
'namespace/@any/value',
'The requested URL is not distinct',
],
'namespace[@any/value] ? md5(namespace__@any__other)' => [
'namespace',
'@any/value',
md5('namespace__@any__other'),
],
'namespace[@any/value] ? namespace/@any/other' => [
'namespace',
'@any/value',
'namespace/@any/other',
],
];
}
/**
* @param string $namespace
* @param string $argumentName
* @param string $queryPath
* @param string|null $failureReason
*
* @test
* @dataProvider routeIdentifiersAreResolvedDataProvider
*/
public function routeIdentifiersAreResolved(string $namespace, string $argumentName, string $queryPath, string $failureReason = null)
{
$query = [];
$routeValue = 'route-value';
$queryValue = 'parameter-value';
$query = ArrayUtility::setValueByPath($query, $queryPath, $queryValue);
$queryParameters = http_build_query($query, '', '&', PHP_QUERY_RFC3986);
$targetUri = sprintf('https://acme.us/welcome/%s?%s', $routeValue, $queryParameters);
$this->mergeSiteConfiguration('acme-com', [
'routeEnhancers' => ['Enhancer' => [
'type' => 'Plugin',
'routePath' => '/{name}',
'_arguments' => [
'name' => $argumentName,
],
'namespace' => $namespace,
]]
]);
$response = $this->executeFrontendRequest(
new InternalRequest($targetUri),
$this->internalRequestContext,
true
);
$body = (string)$response->getBody();
if ($failureReason === null) {
$pageArguments = json_decode($body, true);
var_dump($pageArguments);
self::assertNotNull($pageArguments, 'PageArguments could not be resolved');
$expected = [];
$expected = ArrayUtility::setValueByPath($expected, $namespace . '/' . $argumentName, $routeValue);
$expected = ArrayUtility::setValueByPath($expected, $queryPath, $queryValue);
self::assertEquals($expected, $pageArguments['requestQueryParams']);
} else {
self::assertStringContainsString($failureReason, $body);
}
}
/**
* @param array $enhancer
* @param string $targetUri
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment