Skip to content

Commit

Permalink
[BUGFIX] Avoid redirect loop for empty redirect url
Browse files Browse the repository at this point in the history
Sending a redirect response with a empty `Location` is
invalid per RFC. Browser vendor are dealing differntly
with it.

* Firefox executes a redirect to the current url, leading
  to an `endless` redirect chain - stopping it after some
  recursions with a coresponding notice in the network tab.
* Chrome determines this and is doing nothing at all with
  it - leading to a white page.

From the [1] RFC regarding invalid URI spec for `Location`:

> Note: Some recipients attempt to recover from Location
>       fields that are not valid URI references. This
>       specification does not mandate or define such
>       processing, but does allow it for the sake of
>       robustness.

A matching redirect record with a manually entered `/` as
redirect target leads in TYPO3 v11 to this behaviour. This
can be mitigated by selecting the corresponding site root.

For TYPO3 v12 and upwards a change in the LinkHandling has
been introduced which properly handles the `/` in the link
generation and correctly returning a `/` as redirect url.
That change has quite some impact and is not reasonable to
be backported to TYPO3 v11 within #100958.

This change adds an additionally guard to the `RedirectHandler`
to handle empty redirect urls as endless loop, just logging
it and not responding with an redirect. This helps in v11 and
keeps a safety guard for the future in this place.

[1] https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.2

Resolves: #100791
Related: #100958
Releases: main, 12.4, 11.5
Change-Id: I2af2d5bf759a277ade45bd0f7740ffe0099003b3
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/81279
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: core-ci <typo3@b13.com>
  • Loading branch information
sbuerk committed Sep 28, 2023
1 parent de14e40 commit 210a915
Showing 1 changed file with 17 additions and 1 deletion.
Expand Up @@ -68,7 +68,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
$url = $this->redirectService->getTargetUrl($matchedRedirect, $request);
if ($url instanceof UriInterface) {
if ($this->redirectUriWillRedirectToCurrentUri($request, $url)) {
if ($url->getFragment()) {
if ($this->isEmptyRedirectUri($url)) {
// Empty uri leads to a redirect loop in Firefox, whereas Chrome would stop it but not displaying anything.
// @see https://forge.typo3.org/issues/100791
$this->logger->error('Empty redirect points to itself! Aborting.', ['record' => $matchedRedirect, 'uri' => (string)$url]);
} elseif ($url->getFragment()) {
// Enrich error message for unsharp check with target url fragment.
$this->logger->error('Redirect ' . $url->getPath() . ' eventually points to itself! Target with fragment can not be checked and we take the safe check to avoid redirect loops. Aborting.', ['record' => $matchedRedirect, 'uri' => (string)$url]);
} else {
Expand Down Expand Up @@ -101,6 +105,9 @@ protected function buildRedirectResponse(UriInterface $uri, array $redirectRecor
*/
protected function redirectUriWillRedirectToCurrentUri(ServerRequestInterface $request, UriInterface $redirectUri): bool
{
if ($this->isEmptyRedirectUri($redirectUri)) {
return true;
}
$requestUri = $request->getUri();
$redirectIsAbsolute = $redirectUri->getHost() && $redirectUri->getScheme();
$requestUri = $this->sanitizeUriForComparison($requestUri, !$redirectIsAbsolute);
Expand Down Expand Up @@ -161,4 +168,13 @@ protected function sanitizeUriForComparison(UriInterface $uri, bool $relativeChe

return $uri;
}

/**
* Empty uri leads to a redirect loop in Firefox, whereas Chrome would stop it but not displaying anything.
* @see https://forge.typo3.org/issues/100791
*/
private function isEmptyRedirectUri(UriInterface $uri): bool
{
return (string)$uri === '';
}
}

0 comments on commit 210a915

Please sign in to comment.