Merge pull request #11321 from creative-commoners/pulls/5.2/normalise…

…-external

FIX Do not suffix trailing slash to external links

src/Control/Controller.php

@@ -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;
 
@@ -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, '/');
         }
 

src/Control/Director.php

@@ -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;
         }