Commit 5f195c58 authored by Christian Kuhn's avatar Christian Kuhn
Browse files

[BUGFIX] Avoid Uri->__toString() swallows multi-slash paths

Our PSR-7 Uri implementation has a bug when string
casting an Uri object: Creating a Uri from for instance
'https://example.com//' leads to 'https://example.com/'
when string casting that object. The double slash at the
end is perfectly valid and of course should not be removed.

The fun part is now that we have frontend functional
slug tests that test 'https://website.local//' error
handling and redirect behavior. Due to the old functional
test behavior that had to communicate the Uri through
a PHP process, the above Uri string cast bug has been
triggered. This leads to the situation that the test
looks as if it tested the double-slash, while in fact
it didn't. When changing v11 from php-forking based
frontend site test handling to sub request handling,
we actively implemented this behavior in testing-framework
to stay compatible.

In order to drop that hack from testing-framework,
the patch now:

* Fixes the bug in Uri
* Adds a unit test to verify (string)Uri is ok
* Adds a @todo to SlugSiteRequestTest to look at the
  actual 'https://website.local//' behavior with a
  dedicated patch.

Change-Id: Iea8d048e31b85d0d849542a7a9a55fcf5f220416
Related: #67558
Resolves: #96092
Releases: master, 11.5
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/72314

Tested-by: core-ci's avatarcore-ci <typo3@b13.com>
Tested-by: Stefan Bürk's avatarStefan Bürk <stefan@buerk.tech>
Tested-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Tested-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk's avatarStefan Bürk <stefan@buerk.tech>
Reviewed-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Reviewed-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
parent b1b6bc99
......@@ -588,9 +588,10 @@ class Uri implements UriInterface
}
$path = $this->getPath();
if (!empty($path)) {
$uri .= '/' . ltrim($path, '/');
if ($path !== '' && !str_starts_with($path, '/')) {
$path = '/' . $path;
}
$uri .= $path;
if ($this->query) {
$uri .= '?' . $this->query;
......
......@@ -43,14 +43,21 @@ class UriTest extends UnitTestCase
self::assertEquals('quz', $uri->getFragment());
}
public function canSerializeToStringDataProvider(): array
{
return [
'full uri' => [ 'https://user:pass@local.example.com:3001/foo?bar=baz#quz' ],
'double slash' => [ 'https://user:pass@local.example.com:3001//' ],
];
}
/**
* @test
* @dataProvider canSerializeToStringDataProvider
*/
public function canSerializeToString(): void
public function canSerializeToString(string $uri): void
{
$url = 'https://user:pass@local.example.com:3001/foo?bar=baz#quz';
$uri = new Uri($url);
self::assertEquals($url, (string)$uri);
self::assertEquals($uri, (string)(new Uri($uri)));
}
/**
......
......@@ -105,7 +105,8 @@ class SlugSiteRequestTest extends AbstractTestCase
$domainPaths = [
'https://website.local/',
'https://website.local/?',
'https://website.local//',
// @todo: See how core should act here and activate this or have an own test for this scenario
// 'https://website.local//',
];
return $this->wrapInArray(
......@@ -148,7 +149,8 @@ class SlugSiteRequestTest extends AbstractTestCase
$domainPaths = [
'https://website.local/',
'https://website.local/?',
'https://website.local//',
// @todo: See how core should act here and activate this or have an own test for this scenario
// 'https://website.local//',
];
return $this->wrapInArray(
......
Markdown is supported
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