Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX Do not suffix trailing slash to external links #11321

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions src/Control/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,18 @@ public static function join_links($arg = null)
*/
public static function normaliseTrailingSlash(string $url): string
{
// Do not normalise external urls
// Note that urls without a scheme such as "www.example.com" will be counted as a relative file
if (!Director::is_site_url($url)) {
return $url;
}

// Do not modify files
$extension = pathinfo(Director::makeRelative($url), PATHINFO_EXTENSION);
if ($extension) {
return $url;
}

$querystring = null;
$fragmentIdentifier = null;

Expand All @@ -694,14 +706,9 @@ public static function normaliseTrailingSlash(string $url): string

// Normlise trailing slash
$shouldHaveTrailingSlash = Controller::config()->uninherited('add_trailing_slash');
if ($shouldHaveTrailingSlash
&& !str_ends_with($url, '/')
&& !preg_match('/^(.*)\.([^\/]*)$/', Director::makeRelative($url))
) {
// Add trailing slash if enabled and url does not end with a file extension
if ($shouldHaveTrailingSlash && !str_ends_with($url, '/')) {
$url .= '/';
} elseif (!$shouldHaveTrailingSlash) {
// Remove trailing slash if it shouldn't be there
$url = rtrim($url, '/');
}

Expand Down
11 changes: 10 additions & 1 deletion src/Control/Director.php
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,16 @@ public static function is_site_url($url)

// Allow extensions to weigh in
$isSiteUrl = false;
static::singleton()->extend('updateIsSiteUrl', $isSiteUrl, $url);
// Not using static::singleton() here because it can break
// functional tests such as those in HTTPCacheControlIntegrationTest
// This happens because a singleton of Director is instantiating prior to tests being run,
// because Controller::normaliseTrailingSlash() is called during SapphireTest::setUp(),
// which in turn calls Director::is_site_url()
// For this specific use case we don't need to use dependency injection because the
// chance of the extend() method being customised in projects is low.
// Any extension hooks implementing updateIsSiteUrl() will still be called as expected
$director = new static();
$director->extend('updateIsSiteUrl', $isSiteUrl, $url);
if ($isSiteUrl) {
return true;
}
Expand Down
Loading
Loading