From 5f195c5854402fe94abd10ca188693a8bfcf9b53 Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Fri, 26 Nov 2021 15:58:45 +0100
Subject: [PATCH] [BUGFIX] Avoid Uri->__toString() swallows multi-slash paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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 <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
---
typo3/sysext/core/Classes/Http/Uri.php | 5 +++--
typo3/sysext/core/Tests/Unit/Http/UriTest.php | 15 +++++++++++----
.../SiteHandling/SlugSiteRequestTest.php | 6 ++++--
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/typo3/sysext/core/Classes/Http/Uri.php b/typo3/sysext/core/Classes/Http/Uri.php
index 9b3486933c5b..8b9f7a639141 100644
--- a/typo3/sysext/core/Classes/Http/Uri.php
+++ b/typo3/sysext/core/Classes/Http/Uri.php
@@ -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;
diff --git a/typo3/sysext/core/Tests/Unit/Http/UriTest.php b/typo3/sysext/core/Tests/Unit/Http/UriTest.php
index 99d0a8fc1bab..2c5afab0d195 100644
--- a/typo3/sysext/core/Tests/Unit/Http/UriTest.php
+++ b/typo3/sysext/core/Tests/Unit/Http/UriTest.php
@@ -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)));
}
/**
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
index 171d6c1bfab7..4d249b1c6568 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
@@ -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(
--
GitLab