Skip to content

Commit

Permalink
FIX Do not suffix trailing slash to external links
Browse files Browse the repository at this point in the history
  • Loading branch information
emteknetnz committed Aug 6, 2024
1 parent 1ac3428 commit 8c80a4f
Show file tree
Hide file tree
Showing 5 changed files with 302 additions and 68 deletions.
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

0 comments on commit 8c80a4f

Please sign in to comment.