Commit 4e880df6 authored by Benni Mack's avatar Benni Mack
Browse files

[TASK] Add clarification and tests to SiteBasedRedirectResolver

The original issue for endless redirect loops is already fixed via
https://forge.typo3.org/issues/88032
whereas the query does not result in 307's anymore.

Additional information on redirects is added as comment.

Resolves: #87814
Releases: master, 9.5
Change-Id: I8c97b85cc30d217fe793608da1f26be52e8f68dc
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/59815


Tested-by: default avatarTYPO3com <noreply@typo3.com>
Tested-by: Susanne Moog's avatarSusanne Moog <look@susi.dev>
Tested-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
Reviewed-by: Susanne Moog's avatarSusanne Moog <look@susi.dev>
Reviewed-by: Benni Mack's avatarBenni Mack <benni@typo3.org>
parent 9477f2e3
...@@ -31,6 +31,10 @@ use TYPO3\CMS\Frontend\Page\PageAccessFailureReasons; ...@@ -31,6 +31,10 @@ use TYPO3\CMS\Frontend\Page\PageAccessFailureReasons;
/** /**
* Resolves redirects of site if base is not / * Resolves redirects of site if base is not /
* Can be replaced or extended by extensions if GeoIP-based or user-agent based language redirects need to happen. * Can be replaced or extended by extensions if GeoIP-based or user-agent based language redirects need to happen.
*
* Please note that the redirect usually does not contain the Query Parameters, as special query parameters
* like "id", "L" and "cHash" could then result in an error loop.
* One special case (adding a "/") is keeping the query parameters though.
*/ */
class SiteBaseRedirectResolver implements MiddlewareInterface class SiteBaseRedirectResolver implements MiddlewareInterface
{ {
......
...@@ -110,6 +110,13 @@ class SiteBaseRedirectResolverTest extends UnitTestCase ...@@ -110,6 +110,13 @@ class SiteBaseRedirectResolverTest extends UnitTestCase
null, null,
'' ''
], ],
'redirect to first language adding the slash' => [
'https://twenty.one/en',
'https://twenty.one/en/',
$site1,
null,
''
],
'redirect to second language removing a slash' => [ 'redirect to second language removing a slash' => [
'https://twenty.one/fr/', 'https://twenty.one/fr/',
'https://twenty.one/fr', 'https://twenty.one/fr',
...@@ -124,6 +131,20 @@ class SiteBaseRedirectResolverTest extends UnitTestCase ...@@ -124,6 +131,20 @@ class SiteBaseRedirectResolverTest extends UnitTestCase
null, null,
'' ''
], ],
'redirect to first language and remove nested arguments' => [
'https://twenty.one/?foo[bar]=foobar&bar=foo',
'https://twenty.one/en/',
$site1,
null,
''
],
'redirect to second language removing a slash but keeping the nested arguments' => [
'https://twenty.one/fr/?foo[bar]=foobar&bar=foo',
'https://twenty.one/fr?foo%5Bbar%5D=foobar&bar=foo',
$site1,
$site1->getLanguageById(1),
'/'
],
]; ];
} }
...@@ -212,4 +233,71 @@ class SiteBaseRedirectResolverTest extends UnitTestCase ...@@ -212,4 +233,71 @@ class SiteBaseRedirectResolverTest extends UnitTestCase
$response = $subject->process($request, $this->siteFoundRequestHandler); $response = $subject->process($request, $this->siteFoundRequestHandler);
self::assertEquals($expectedStatusCode, $response->getStatusCode()); self::assertEquals($expectedStatusCode, $response->getStatusCode());
} }
/**
* @return array
*/
public function doNotRedirectOnBaseWithoutQueryDataProvider(): array
{
$site1 = new Site('outside-site', 13, [
'base' => 'https://twenty.one/',
'languages' => [
0 => [
'languageId' => 0,
'locale' => 'en_US.UTF-8',
'base' => '/en/'
],
1 => [
'languageId' => 1,
'locale' => 'fr_CA.UTF-8',
'base' => '/fr'
]
]
]);
return [
'no redirect for base' => [
'https://twenty.one/en/',
$site1,
$site1->getLanguageById(0),
''
],
'no redirect for base when ID is given' => [
'https://twenty.one/index.php?id=2',
$site1,
$site1->getLanguageById(0),
''
],
'no redirect for base and nested arguments' => [
'https://twenty.one/en/?foo[bar]=foobar&bar=foo',
$site1,
$site1->getLanguageById(0),
''
],
];
}
/**
* @param string $incomingUrl
* @param Site $site
* @param SiteLanguage|null $language
* @param string $tail
* @dataProvider doNotRedirectOnBaseWithoutQueryDataProvider
* @test
*/
public function doNotRedirectOnBaseWithoutQuery(
string $incomingUrl,
Site $site,
?SiteLanguage $language,
string $tail
): void {
$routeResult = new SiteRouteResult(new Uri($incomingUrl), $site, $language, $tail);
$request = new ServerRequest($incomingUrl, 'GET');
$request = $request->withAttribute('site', $site);
$request = $request->withAttribute('language', $language);
$request = $request->withAttribute('routing', $routeResult);
$subject = new SiteBaseRedirectResolver();
$response = $subject->process($request, $this->siteFoundRequestHandler);
self::assertEquals(200, $response->getStatusCode());
}
} }
Supports Markdown
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