API Normalise trailing slashes for all paths

NOTE: There will be additional related PRs required for at least
silverstripe/cms and silverstripe/admin.

Co-authored-by: Guy Sartorelli <guy.sartorelli@silverstripe.com>

src/Control/Controller.php

@@ -66,6 +66,11 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
      */
     protected HTTPResponse $response;
 
+    /**
+     * If true, a trailing slash is added to the end of URLs, e.g. from {@link Controller::join_links()}
+     */
+    private static bool $add_trailing_slash = false;
+
     /**
      * Default URL handlers.
      *
@@ -648,6 +653,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
                 parse_str($suffix ?? '', $localargs);
                 $queryargs = array_merge($queryargs, $localargs);
             }
+            // Join paths together
             if ((is_string($arg) && $arg) || is_numeric($arg)) {
                 $arg = (string) $arg;
                 if ($result && substr($result ?? '', -1) != '/' && $arg[0] != '/') {
@@ -658,6 +664,8 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
             }
         }
 
+        $result = static::normaliseTrailingSlash($result);
+
         if ($queryargs) {
             $result .= '?' . http_build_query($queryargs ?? []);
         }
@@ -669,6 +677,52 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
         return $result;
     }
 
+    /**
+     * Normalises a URL according to the configuration for add_trailing_slash
+     */
+    public static function normaliseTrailingSlash(string $url): string
+    {
+        $querystring = null;
+        $fragmentIdentifier = null;
+
+        // Find fragment identifier
+        if (strpos($url, '#') !== false) {
+            list($url, $fragmentIdentifier) = explode('#', $url, 2);
+        }
+        // Find querystrings
+        if (strpos($url, '?') !== false) {
+            list($url, $querystring) = explode('?', $url, 2);
+        }
+
+        // Normlise trailing slash
+        $shouldHaveTrailingSlash = self::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
+            $url .= '/';
+        } elseif (!$shouldHaveTrailingSlash) {
+            // Remove trailing slash if it shouldn't be there
+            $url = rtrim($url, '/');
+        }
+
+        // Ensure relative root URLs are represented with a slash
+        if ($url === '') {
+            $url = '/';
+        }
+
+        // Add back fragment identifier and querystrings
+        if ($querystring) {
+            $url .= '?' . $querystring;
+        }
+        if ($fragmentIdentifier) {
+            $url .= "#$fragmentIdentifier";
+        }
+
+        return $url;
+    }
+
     /**
      * @return array
      */

src/Control/Director.php

@@ -400,13 +400,13 @@ class Director implements TemplateGlobalProvider
     {
         // Check if there is already a protocol given
         if (preg_match('/^http(s?):\/\//', $url ?? '')) {
-            return $url;
+            return Controller::normaliseTrailingSlash($url);
         }
 
         // Absolute urls without protocol are added
         // E.g. //google.com -> http://google.com
         if (strpos($url ?? '', '//') === 0) {
-            return self::protocol() . substr($url ?? '', 2);
+            return Controller::normaliseTrailingSlash(self::protocol() . substr($url ?? '', 2));
         }
 
         // Determine method for mapping the parent to this relative url
@@ -686,7 +686,7 @@ class Director implements TemplateGlobalProvider
         $url = preg_replace('#([^:])//#', '\\1/', trim($url ?? ''));
 
         // If using a real url, remove protocol / hostname / auth / port
-        if (preg_match('#^(?<protocol>https?:)?//(?<hostpart>[^/]*)(?<url>(/.*)?)$#i', $url ?? '', $matches)) {
+        if (preg_match('@^(?<protocol>https?:)?//(?<hostpart>[^/?#]*)(?<url>([/?#].*)?)$@i', $url ?? '', $matches)) {
             $url = $matches['url'];
         }
 
@@ -878,10 +878,11 @@ class Director implements TemplateGlobalProvider
      */
     public static function absoluteBaseURL()
     {
-        return self::absoluteURL(
+        $baseURL = self::absoluteURL(
             self::baseURL(),
             self::ROOT
         );
+        return Controller::normaliseTrailingSlash($baseURL);
     }
 
     /**

src/Control/HTTP.php

@@ -172,7 +172,7 @@ class HTTP
         $isRelative = false;
         // We need absolute URLs for parse_url()
         if (Director::is_relative_url($uri)) {
-            $uri = Director::absoluteBaseURL() . $uri;
+            $uri = Controller::join_links(Director::absoluteBaseURL(), $uri);
             $isRelative = true;
         }
 
@@ -212,7 +212,7 @@ class HTTP
         $newUri = $scheme . '://' . $user . $host . $port . $path . $params . $fragment;
 
         if ($isRelative) {
-            return Director::baseURL() . Director::makeRelative($newUri);
+            return Controller::join_links(Director::baseURL(), Director::makeRelative($newUri));
         }
 
         return $newUri;

src/Control/Middleware/CanonicalURLMiddleware.php

@@ -4,11 +4,12 @@ namespace SilverStripe\Control\Middleware;
 
 use SilverStripe\Control\Controller;
 use SilverStripe\Control\Director;
-use SilverStripe\Control\HTTP;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\HTTPResponse_Exception;
+use SilverStripe\Core\Config\Configurable;
 use SilverStripe\Core\CoreKernel;
+use SilverStripe\Core\Environment;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\Core\Injector\Injector;
 
@@ -17,10 +18,31 @@ use SilverStripe\Core\Injector\Injector;
  *  - redirect basic auth requests to HTTPS
  *  - force WWW, redirect to the subdomain "www."
  *  - force SSL, redirect to https
+ *  - force the correct path (with vs without trailing slash)
  */
 class CanonicalURLMiddleware implements HTTPMiddleware
 {
     use Injectable;
+    use Configurable;
+
+    /**
+     * If set, the trailing slash configuration set in {@link Controller::add_trailing_slash} is enforced
+     * with a redirect.
+     */
+    protected bool $enforceTrailingSlashConfig = true;
+
+    /**
+     * If enforceTrailingSlashConfig is enabled, this is the list of paths that are ignored
+     */
+    protected array $enforceTrailingSlashConfigIgnorePaths = [
+        'admin/',
+        'dev/',
+    ];
+
+    /**
+     * If enforceTrailingSlashConfig is enabled, this is the list of user agents that are ignored
+     */
+    protected array $enforceTrailingSlashConfigIgnoreUserAgents = [];
 
     /**
      * Set if we should redirect to WWW
@@ -76,6 +98,39 @@ class CanonicalURLMiddleware implements HTTPMiddleware
      */
     protected $forceSSLDomain = null;
 
+    public function setEnforceTrailingSlashConfig(bool $value): static
+    {
+        $this->enforceTrailingSlashConfig = $value;
+        return $this;
+    }
+
+    public function getEnforceTrailingSlashConfig(): bool
+    {
+        return $this->enforceTrailingSlashConfig;
+    }
+
+    public function setEnforceTrailingSlashConfigIgnorePaths(array $value): static
+    {
+        $this->enforceTrailingSlashConfigIgnorePaths = $value;
+        return $this;
+    }
+
+    public function getEnforceTrailingSlashConfigIgnorePaths(): array
+    {
+        return $this->enforceTrailingSlashConfigIgnorePaths;
+    }
+
+    public function setEnforceTrailingSlashConfigIgnoreUserAgents(array $value): static
+    {
+        $this->enforceTrailingSlashConfigIgnoreUserAgents = $value;
+        return $this;
+    }
+
+    public function getEnforceTrailingSlashConfigIgnoreUserAgents(): array
+    {
+        return $this->enforceTrailingSlashConfigIgnoreUserAgents;
+    }
+
     /**
      * @return array
      */
@@ -214,6 +269,7 @@ class CanonicalURLMiddleware implements HTTPMiddleware
         // Get properties of current request
         $host = $request->getHost();
         $scheme = $request->getScheme();
+        $url = strtok(Environment::getEnv('REQUEST_URI'), '?');
 
         // Check https
         if ($this->requiresSSL($request)) {
@@ -228,8 +284,16 @@ class CanonicalURLMiddleware implements HTTPMiddleware
             $host = "www.{$host}";
         }
 
+        // Check trailing Slash
+        if ($this->requiresTrailingSlashRedirect($request, $url)) {
+            $url = Controller::normaliseTrailingSlash($url);
+        }
+
         // No-op if no changes
-        if ($request->getScheme() === $scheme && $request->getHost() === $host) {
+        if ($request->getScheme() === $scheme
+            && $request->getHost() === $host
+            && strtok(Environment::getEnv('REQUEST_URI'), '?') === $url
+        ) {
             return null;
         }
 
@@ -309,6 +373,72 @@ class CanonicalURLMiddleware implements HTTPMiddleware
         return false;
     }
 
+    /**
+     * Check if a redirect for trailing slash is necessary
+     */
+    protected function requiresTrailingSlashRedirect(HTTPRequest $request, string $url)
+    {
+        // Get the URL without querystrings or fragment identifiers
+        if (strpos($url, '#') !== false) {
+            $url = explode('#', $url, 2)[0];
+        }
+        if (strpos($url, '?') !== false) {
+            $url = explode('?', $url, 2)[0];
+        }
+
+        // Check if force Trailing Slash is enabled
+        if ($this->getEnforceTrailingSlashConfig() !== true) {
+            return false;
+        }
+
+        $requestPath = $request->getURL();
+
+        // skip if requesting root
+        if ($requestPath === '/' || $requestPath === '') {
+            return false;
+        }
+
+        // Skip if is AJAX request
+        if (Director::is_ajax()) {
+            return false;
+        }
+
+        // Skip if request has a file extension
+        if (!empty($request->getExtension())) {
+            return false;
+        }
+
+        // Skip if any configured ignore paths match
+        $paths = (array) $this->getEnforceTrailingSlashConfigIgnorePaths();
+        if (!empty($paths)) {
+            foreach ($paths as $path) {
+                if (str_starts_with(trim($path, '/'), trim($requestPath, '/'))) {
+                    return false;
+                }
+            }
+        }
+
+        // Skip if any configured ignore user agents match
+        $agents = (array) $this->getEnforceTrailingSlashConfigIgnoreUserAgents();
+        if (!empty($agents)) {
+            if (in_array(
+                $request->getHeader('User-Agent'),
+                $agents
+            )) {
+                return false;
+            }
+        }
+
+        // Already using trailing slash correctly
+        $addTrailingSlash = Controller::config()->uninherited('add_trailing_slash');
+        $hasTrailingSlash = str_ends_with($url, '/');
+        if (($addTrailingSlash && $hasTrailingSlash) || (!$addTrailingSlash && !$hasTrailingSlash)) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * @return int
      */
@@ -357,7 +487,7 @@ class CanonicalURLMiddleware implements HTTPMiddleware
     protected function isEnabled()
     {
         // At least one redirect must be enabled
-        if (!$this->getForceWWW() && !$this->getForceSSL()) {
+        if (!$this->getForceWWW() && !$this->getForceSSL() && $this->getEnforceTrailingSlashConfig() !== true) {
             return false;
         }
 

src/Control/RequestHandler.php

@@ -561,7 +561,7 @@ class RequestHandler extends ViewableData
         // Check configured url_segment
         $url = $this->config()->get('url_segment');
         if ($url) {
-            $link = Controller::join_links($url, $action, '/');
+            $link = Controller::join_links($url, $action);
 
             // Give extensions the chance to modify by reference
             $this->extend('updateLink', $link, $action);

src/Dev/DebugView.php

@@ -135,8 +135,9 @@ class DebugView
         $pathLinks = [];
         foreach ($parts as $part) {
             if ($part != '') {
-                $pathPart .= "$part/";
+                $pathPart = Controller::join_links($pathPart, $part);
-                $pathLinks[] = "<a href=\"$base$pathPart\">$part</a>";
+                $href = Controller::join_links($base, $pathPart);
+                $pathLinks[] = "<a href=\"$href\">$part</a>";
             }
         }
         return implode('&nbsp;&rarr;&nbsp;', $pathLinks);

src/Dev/TaskRunner.php

@@ -76,7 +76,7 @@ class TaskRunner extends Controller
 
         foreach ($tasks as $task) {
             $list->push(ArrayData::create([
-                'TaskLink' => $baseUrl . 'dev/tasks/' . $task['segment'],
+                'TaskLink' => Controller::join_links($baseUrl, 'dev/tasks/', $task['segment']),
                 'Title' => $task['title'],
                 'Description' => $task['description'],
             ]));

src/Forms/HTMLEditor/TinyMCEConfig.php

@@ -651,7 +651,7 @@ class TinyMCEConfig extends HTMLEditorConfig implements i18nEntityProvider
         $settings = $this->getSettings();
 
         // https://www.tiny.cloud/docs/tinymce/6/url-handling/#document_base_url
-        $settings['document_base_url'] = Director::absoluteBaseURL();
+        $settings['document_base_url'] = rtrim(Director::absoluteBaseURL(), '/') . '/';
 
         // https://www.tiny.cloud/docs/tinymce/6/apis/tinymce.root/#properties
         $baseResource = $this->getTinyMCEResource();

src/View/SSViewer.php

@@ -825,7 +825,8 @@ PHP;
      */
     public static function get_base_tag($contentGeneratedSoFar)
     {
-        $base = Director::absoluteBaseURL();
+        // Base href should always have a trailing slash
+        $base = rtrim(Director::absoluteBaseURL(), '/') . '/';
 
         // Is the document XHTML?
         if (preg_match('/<!DOCTYPE[^>]+xhtml/i', $contentGeneratedSoFar ?? '')) {

tests/php/Control/ControllerTest.php

@@ -59,7 +59,7 @@ class ControllerTest extends FunctionalTest
     public function testDefaultAction()
     {
         /* For a controller with a template, the default action will simple run that template. */
-        $response = $this->get("TestController/");
+        $response = $this->get("TestController");
         $this->assertStringContainsString("This is the main template. Content is 'default content'", $response->getBody());
     }
 
@@ -94,7 +94,7 @@ class ControllerTest extends FunctionalTest
 
     public function testAllowedActions()
     {
-        $response = $this->get("UnsecuredController/");
+        $response = $this->get("UnsecuredController");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
@@ -115,7 +115,7 @@ class ControllerTest extends FunctionalTest
             'Access denied on action without $allowed_actions on defining controller, ' . 'when called without an action in the URL'
         );
 
-        $response = $this->get("AccessBaseController/");
+        $response = $this->get("AccessBaseController");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
@@ -143,7 +143,7 @@ class ControllerTest extends FunctionalTest
             'Access denied on action with empty $allowed_actions on defining controller, ' . 'even when action is allowed in subclasses (allowed_actions don\'t inherit)'
         );
 
-        $response = $this->get("AccessSecuredController/");
+        $response = $this->get("AccessSecuredController");
         $this->assertEquals(
             200,
             $response->getStatusCode(),
@@ -238,7 +238,7 @@ class ControllerTest extends FunctionalTest
             "Access denied to method not defined in allowed_actions on extension, " . "where method is also defined on extension"
         );
 
-        $response = $this->get('IndexSecuredController/');
+        $response = $this->get('IndexSecuredController');
         $this->assertEquals(
             403,
             $response->getStatusCode(),
@@ -253,7 +253,7 @@ class ControllerTest extends FunctionalTest
         );
 
         $this->logInAs('admin');
-        $response = $this->get('IndexSecuredController/');
+        $response = $this->get('IndexSecuredController');
         $this->assertEquals(
             200,
             $response->getStatusCode(),
@@ -276,18 +276,41 @@ class ControllerTest extends FunctionalTest
     {
         /* Controller::join_links() will reliably join two URL-segments together so that they will be
         * appropriately parsed by the URL parser */
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals("admin/crm/MyForm", Controller::join_links("admin/crm", "MyForm"));
         $this->assertEquals("admin/crm/MyForm", Controller::join_links("admin/crm/", "MyForm"));
+        $this->assertEquals("https://www.test.com/admin/crm/MyForm", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm"));
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals("admin/crm/MyForm/", Controller::join_links("admin/crm", "MyForm"));
+        $this->assertEquals("admin/crm/MyForm/", Controller::join_links("admin/crm/", "MyForm"));
+        $this->assertEquals("https://www.test.com/admin/crm/MyForm/", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm"));
 
         /* It will also handle appropriate combination of querystring variables */
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals("admin/crm/MyForm?flush=1", Controller::join_links("admin/crm/?flush=1", "MyForm"));
         $this->assertEquals("admin/crm/MyForm?flush=1", Controller::join_links("admin/crm/", "MyForm?flush=1"));
         $this->assertEquals(
             "admin/crm/MyForm?field=1&other=1",
             Controller::join_links("admin/crm/?field=1", "MyForm?other=1")
         );
+        $this->assertEquals("https://www.test.com/admin/crm/MyForm?flush=1", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm?flush=1"));
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals("admin/crm/MyForm/?flush=1", Controller::join_links("admin/crm/?flush=1", "MyForm"));
+        $this->assertEquals("admin/crm/MyForm/?flush=1", Controller::join_links("admin/crm/", "MyForm?flush=1"));
+        $this->assertEquals(
+            "admin/crm/MyForm/?field=1&other=1",
+            Controller::join_links("admin/crm/?field=1", "MyForm?other=1")
+        );
+        $this->assertEquals("https://www.test.com/admin/crm/MyForm/?flush=1", Controller::join_links("https://www.test.com", "admin/crm/", "MyForm?flush=1"));
 
         /* It can handle arbitrary numbers of components, and will ignore empty ones */
+        Controller::config()->set('add_trailing_slash', false);
+        $this->assertEquals("admin/crm/MyForm", Controller::join_links("admin/", "crm", "", "MyForm/"));
+        $this->assertEquals(
+            "admin/crm/MyForm?a=1&b=2",
+            Controller::join_links("admin/?a=1", "crm", "", "MyForm/?b=2")
+        );
+        Controller::config()->set('add_trailing_slash', true);
         $this->assertEquals("admin/crm/MyForm/", Controller::join_links("admin/", "crm", "", "MyForm/"));
         $this->assertEquals(
             "admin/crm/MyForm/?a=1&b=2",
@@ -295,49 +318,170 @@ class ControllerTest extends FunctionalTest
         );
 
         /* It can also be used to attach additional get variables to a link */
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals("admin/crm?flush=1", Controller::join_links("admin/crm", "?flush=1"));
         $this->assertEquals("admin/crm?existing=1&flush=1", Controller::join_links("admin/crm?existing=1", "?flush=1"));
         $this->assertEquals(
             "admin/crm/MyForm?a=1&b=2&c=3",
             Controller::join_links("?a=1", "admin/crm", "?b=2", "MyForm?c=3")
         );
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals("admin/crm/?flush=1", Controller::join_links("admin/crm", "?flush=1"));
+        $this->assertEquals("admin/crm/?existing=1&flush=1", Controller::join_links("admin/crm?existing=1", "?flush=1"));
+        $this->assertEquals(
+            "admin/crm/MyForm/?a=1&b=2&c=3",
+            Controller::join_links("?a=1", "admin/crm", "?b=2", "MyForm?c=3")
+        );
 
         // And duplicates are handled nicely
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals(
             "admin/crm?foo=2&bar=3&baz=1",
             Controller::join_links("admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3")
         );
+        $this->assertEquals(
+            "https://www.test.com/admin/crm?foo=2&bar=3&baz=1",
+            Controller::join_links("https://www.test.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3")
+        );
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals(
+            "admin/crm/?foo=2&bar=3&baz=1",
+            Controller::join_links("admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3")
+        );
+        $this->assertEquals(
+            "https://www.test.com/admin/crm/?foo=2&bar=3&baz=1",
+            Controller::join_links("https://www.test.com", "admin/crm?foo=1&bar=1&baz=1", "?foo=2&bar=3")
+        );
 
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals(
             'admin/action',
             Controller::join_links('admin/', '/', '/action'),
             'Test that multiple slashes are trimmed.'
         );
-
         $this->assertEquals('/admin/action', Controller::join_links('/admin', 'action'));
+        $this->assertEquals(
+            'https://www.test.com/admin/action',
+            Controller::join_links('https://www.test.com', '/', '/admin/', '/', '/action'),
+            'Test that multiple slashes are trimmed.'
+        );
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals(
+            'admin/action/',
+            Controller::join_links('admin/', '/', '/action'),
+            'Test that multiple slashes are trimmed.'
+        );
+        $this->assertEquals('/admin/action/', Controller::join_links('/admin', 'action'));
+        $this->assertEquals(
+            'https://www.test.com/admin/action/',
+            Controller::join_links('https://www.test.com', '/', '/admin/', '/', '/action'),
+            'Test that multiple slashes are trimmed.'
+        );
 
         /* One fragment identifier is handled as you would expect */
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals("my-page?arg=var#subsection", Controller::join_links("my-page#subsection", "?arg=var"));
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals("my-page/?arg=var#subsection", Controller::join_links("my-page#subsection", "?arg=var"));
 
         /* If there are multiple, it takes the last one */
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals(
             "my-page?arg=var#second-section",
             Controller::join_links("my-page#subsection", "?arg=var", "#second-section")
         );
+        $this->assertEquals(
+            "https://www.test.com/my-page?arg=var#second-section",
+            Controller::join_links("https://www.test.com", "my-page#subsection", "?arg=var", "#second-section")
+        );
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals(
+            "my-page/?arg=var#second-section",
+            Controller::join_links("my-page#subsection", "?arg=var", "#second-section")
+        );
+        $this->assertEquals(
+            "https://www.test.com/my-page/?arg=var#second-section",
+            Controller::join_links("https://www.test.com", "my-page#subsection", "?arg=var", "#second-section")
+        );
 
         /* Does type-safe checks for zero value */
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals("my-page/0", Controller::join_links("my-page", 0));
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals("my-page/0/", Controller::join_links("my-page", 0));
 
         // Test array args
+        Controller::config()->set('add_trailing_slash', false);
         $this->assertEquals(
-            "admin/crm/MyForm?a=1&b=2&c=3",
+            "https://www.test.com/admin/crm/MyForm?a=1&b=2&c=3",
-            Controller::join_links(["?a=1", "admin/crm", "?b=2", "MyForm?c=3"])
+            Controller::join_links(["https://www.test.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"])
         );
+        Controller::config()->set('add_trailing_slash', true);
+        $this->assertEquals(
+            "https://www.test.com/admin/crm/MyForm/?a=1&b=2&c=3",
+            Controller::join_links(["https://www.test.com", "?a=1", "admin/crm", "?b=2", "MyForm?c=3"])
+        );
+    }
+
+    public function testNormaliseTrailingSlash()
+    {
+        foreach ([true, false] as $withTrailingSlash) {
+            Controller::config()->set('add_trailing_slash', $withTrailingSlash);
+            $slash = $withTrailingSlash ? '/' : '';
+
+            // Correctly gives slash to a relative root path
+            $this->assertEquals('/', Controller::normaliseTrailingSlash(''));
+            $this->assertEquals('/', Controller::normaliseTrailingSlash('/'));
+
+            // Correctly adds or removes trailing slash
+            $this->assertEquals("some/path{$slash}", Controller::normaliseTrailingSlash('some/path/'));
+            $this->assertEquals("some/path{$slash}", Controller::normaliseTrailingSlash('some/path'));
+
+            // Retains leading slash, if there is one
+            $this->assertEquals("/some/path{$slash}", Controller::normaliseTrailingSlash('/some/path/'));
+            $this->assertEquals("/some/path{$slash}", Controller::normaliseTrailingSlash('/some/path'));
+
+            // Effectively treats absolute URL as relative
+            $this->assertEquals("https://www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('https://www.google.com/some/path/'));
+            $this->assertEquals("//www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('//www.google.com/some/path'));
+            $this->assertEquals("www.google.com/some/path{$slash}", Controller::normaliseTrailingSlash('www.google.com/some/path'));
+            $this->assertEquals("https://www.google.com{$slash}", Controller::normaliseTrailingSlash('https://www.google.com'));
+            $this->assertEquals("//www.google.com{$slash}", Controller::normaliseTrailingSlash('//www.google.com/'));
+
+            // Retains query string and anchor if present
+            $this->assertEquals("some/path{$slash}?key=value&key2=value2", Controller::normaliseTrailingSlash('some/path/?key=value&key2=value2'));
+            $this->assertEquals("some/path{$slash}#some-id", Controller::normaliseTrailingSlash('some/path/#some-id'));
+            $this->assertEquals("some/path{$slash}?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/path/?key=value&key2=value2#some-id'));
+            $this->assertEquals("some/path{$slash}?key=value&key2=value2", Controller::normaliseTrailingSlash('some/path?key=value&key2=value2'));
+            $this->assertEquals("some/path{$slash}#some-id", Controller::normaliseTrailingSlash('some/path#some-id'));
+            $this->assertEquals("some/path{$slash}?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/path?key=value&key2=value2#some-id'));
+
+            // Don't ever add a trailing slash to the end of a URL that looks like a file
+            $this->assertEquals("https://www.google.com/some/file.txt", Controller::normaliseTrailingSlash('https://www.google.com/some/file.txt'));
+            $this->assertEquals("//www.google.com/some/file.txt", Controller::normaliseTrailingSlash('//www.google.com/some/file.txt'));
+            $this->assertEquals("www.google.com/some/file.txt", Controller::normaliseTrailingSlash('www.google.com/some/file.txt'));
+            $this->assertEquals("/some/file.txt", Controller::normaliseTrailingSlash('/some/file.txt'));
+            $this->assertEquals("some/file.txt", Controller::normaliseTrailingSlash('some/file.txt'));
+            $this->assertEquals("file.txt", Controller::normaliseTrailingSlash('file.txt'));
+            $this->assertEquals("some/file.txt?key=value&key2=value2#some-id", Controller::normaliseTrailingSlash('some/file.txt?key=value&key2=value2#some-id'));
+            // NOTE: `www.google.com` is already treated as "relative" by Director::makeRelative(), which means we can't tell that it's a host (and not a file).
+            $this->assertEquals("www.google.com", Controller::normaliseTrailingSlash('www.google.com'));
+        }
     }
 
     public function testLink()
     {
         $controller = new HasAction();
+
+        Controller::config()->set('add_trailing_slash', false);
+
+        $this->assertEquals('HasAction', $controller->Link());
+        $this->assertEquals('HasAction', $controller->Link(null));
+        $this->assertEquals('HasAction', $controller->Link(false));
+        $this->assertEquals('HasAction/allowed-action', $controller->Link('allowed-action'));
+
+        Controller::config()->set('add_trailing_slash', true);
+
         $this->assertEquals('HasAction/', $controller->Link());
         $this->assertEquals('HasAction/', $controller->Link(null));
         $this->assertEquals('HasAction/', $controller->Link(false));
@@ -461,7 +605,7 @@ class ControllerTest extends FunctionalTest
         );
 
         // BackURL is internal link
-        $internalAbsoluteUrl = Director::absoluteBaseURL() . '/some-url';
+        $internalAbsoluteUrl = Controller::join_links(Director::absoluteBaseURL(), '/some-url');
         $link = 'TestController/redirectbacktest?BackURL=' . urlencode($internalAbsoluteUrl ?? '');
         $response = $this->get($link);
         $this->assertEquals($internalAbsoluteUrl, $response->getHeader('Location'));

tests/php/Control/DirectorTest.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\Control\Tests;
 
+use SilverStripe\Control\Controller;
 use SilverStripe\Control\Cookie_Backend;
 use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPRequest;
@@ -92,116 +93,126 @@ class DirectorTest extends SapphireTest
 
     public function testAbsoluteURL()
     {
-        Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/mysite/');
+        foreach ([true, false] as $withTrailingSlash) {
-        $_SERVER['REQUEST_URI'] = "http://www.mysite.com:9090/mysite/sub-page/";
+            Controller::config()->set('add_trailing_slash', $withTrailingSlash);
+            $slash = $withTrailingSlash ? '/' : '';
 
-        //test empty / local urls
+            Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/mysite/');
-        foreach (['', './', '.'] as $url) {
+            $_SERVER['REQUEST_URI'] = "http://www.mysite.com:9090/mysite/sub-page/";
-            $this->assertEquals("http://www.mysite.com:9090/mysite/", Director::absoluteURL($url, Director::BASE));
+
-            $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL($url, Director::ROOT));
+            //test empty / local urls
-            $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/", Director::absoluteURL($url, Director::REQUEST));
+            foreach (['', './', '.'] as $url) {
+                $this->assertEquals("http://www.mysite.com:9090/mysite{$slash}", Director::absoluteURL($url, Director::BASE));
+                $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL($url, Director::ROOT));
+                $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page{$slash}", Director::absoluteURL($url, Director::REQUEST));
+            }
+
+            // Test site root url
+            $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('/'));
+
+            // Test Director::BASE
+            $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::BASE));
+            $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::BASE));
+            $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::BASE));
+            $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::BASE));
+            $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::BASE));
+
+            // Test Director::ROOT
+            $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::ROOT));
+            $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::ROOT));
+            $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::ROOT));
+            $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::ROOT));
+            $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::ROOT));
+
+            // Test Director::REQUEST
+            $this->assertEquals("http://www.mysite.com:9090{$slash}", Director::absoluteURL('http://www.mysite.com:9090/', Director::REQUEST));
+            $this->assertEquals("http://www.mytest.com{$slash}", Director::absoluteURL('http://www.mytest.com', Director::REQUEST));
+            $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("http://www.mysite.com:9090/test", Director::REQUEST));
+            $this->assertEquals("http://www.mysite.com:9090/root{$slash}", Director::absoluteURL("/root", Director::REQUEST));
+            $this->assertEquals("http://www.mysite.com:9090/root/url{$slash}", Director::absoluteURL("/root/url", Director::REQUEST));
+
+            // Test evaluating relative urls relative to base (default)
+            $this->assertEquals("http://www.mysite.com:9090/mysite/test{$slash}", Director::absoluteURL("test"));
+            $this->assertEquals("http://www.mysite.com:9090/mysite/test/url{$slash}", Director::absoluteURL("test/url"));
+            $this->assertEquals("http://www.mysite.com:9090/mysite/test{$slash}", Director::absoluteURL("test", Director::BASE));
+            $this->assertEquals("http://www.mysite.com:9090/mysite/test/url{$slash}", Director::absoluteURL("test/url", Director::BASE));
+
+            // Test evaluating relative urls relative to root
+            $this->assertEquals("http://www.mysite.com:9090/test{$slash}", Director::absoluteURL("test", Director::ROOT));
+            $this->assertEquals("http://www.mysite.com:9090/test/url{$slash}", Director::absoluteURL("test/url", Director::ROOT));
+
+            // Test relative to requested page
+            $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/test{$slash}", Director::absoluteURL("test", Director::REQUEST));
+            $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/test/url{$slash}", Director::absoluteURL("test/url", Director::REQUEST));
+
+            // Test that javascript links are not left intact
+            $this->assertStringStartsNotWith('javascript', Director::absoluteURL('javascript:alert("attack")'));
+            $this->assertStringStartsNotWith('alert', Director::absoluteURL('javascript:alert("attack")'));
+            $this->assertStringStartsNotWith('javascript', Director::absoluteURL('alert("attack")'));
+            $this->assertStringStartsNotWith('alert', Director::absoluteURL('alert("attack")'));
         }
-
-        // Test site root url
-        $this->assertEquals("http://www.mysite.com:9090/", Director::absoluteURL('/'));
-
-        // Test Director::BASE
-        $this->assertEquals('http://www.mysite.com:9090/', Director::absoluteURL('http://www.mysite.com:9090/', Director::BASE));
-        $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::BASE));
-        $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("http://www.mysite.com:9090/test", Director::BASE));
-        $this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::BASE));
-        $this->assertEquals("http://www.mysite.com:9090/root/url", Director::absoluteURL("/root/url", Director::BASE));
-
-        // Test Director::ROOT
-        $this->assertEquals('http://www.mysite.com:9090/', Director::absoluteURL('http://www.mysite.com:9090/', Director::ROOT));
-        $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::ROOT));
-        $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("http://www.mysite.com:9090/test", Director::ROOT));
-        $this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::ROOT));
-        $this->assertEquals("http://www.mysite.com:9090/root/url", Director::absoluteURL("/root/url", Director::ROOT));
-
-        // Test Director::REQUEST
-        $this->assertEquals('http://www.mysite.com:9090/', Director::absoluteURL('http://www.mysite.com:9090/', Director::REQUEST));
-        $this->assertEquals('http://www.mytest.com', Director::absoluteURL('http://www.mytest.com', Director::REQUEST));
-        $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("http://www.mysite.com:9090/test", Director::REQUEST));
-        $this->assertEquals("http://www.mysite.com:9090/root", Director::absoluteURL("/root", Director::REQUEST));
-        $this->assertEquals("http://www.mysite.com:9090/root/url", Director::absoluteURL("/root/url", Director::REQUEST));
-
-        // Test evaluating relative urls relative to base (default)
-        $this->assertEquals("http://www.mysite.com:9090/mysite/test", Director::absoluteURL("test"));
-        $this->assertEquals("http://www.mysite.com:9090/mysite/test/url", Director::absoluteURL("test/url"));
-        $this->assertEquals("http://www.mysite.com:9090/mysite/test", Director::absoluteURL("test", Director::BASE));
-        $this->assertEquals("http://www.mysite.com:9090/mysite/test/url", Director::absoluteURL("test/url", Director::BASE));
-
-        // Test evaluating relative urls relative to root
-        $this->assertEquals("http://www.mysite.com:9090/test", Director::absoluteURL("test", Director::ROOT));
-        $this->assertEquals("http://www.mysite.com:9090/test/url", Director::absoluteURL("test/url", Director::ROOT));
-
-        // Test relative to requested page
-        $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/test", Director::absoluteURL("test", Director::REQUEST));
-        $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/test/url", Director::absoluteURL("test/url", Director::REQUEST));
-
-        // Test that javascript links are not left intact
-        $this->assertStringStartsNotWith('javascript', Director::absoluteURL('javascript:alert("attack")'));
-        $this->assertStringStartsNotWith('alert', Director::absoluteURL('javascript:alert("attack")'));
-        $this->assertStringStartsNotWith('javascript', Director::absoluteURL('alert("attack")'));
-        $this->assertStringStartsNotWith('alert', Director::absoluteURL('alert("attack")'));
     }
 
     public function testAlternativeBaseURL()
     {
-        // relative base URLs - you should end them in a /
+        foreach ([true, false] as $withTrailingSlash) {
-        Director::config()->set('alternate_base_url', '/relativebase/');
+            Controller::config()->set('add_trailing_slash', $withTrailingSlash);
-        $_SERVER['HTTP_HOST'] = 'www.somesite.com';
+            $slash = $withTrailingSlash ? '/' : '';
-        $_SERVER['REQUEST_URI'] = "/relativebase/sub-page/";
 
-        $this->assertEquals('/relativebase/', Director::baseURL());
+            // relative base URLs - you should end them in a /
-        $this->assertEquals('http://www.somesite.com/relativebase/', Director::absoluteBaseURL());
+            Director::config()->set('alternate_base_url', '/relativebase/');
-        $this->assertEquals(
+            $_SERVER['HTTP_HOST'] = 'www.somesite.com';
-            'http://www.somesite.com/relativebase/subfolder/test',
+            $_SERVER['REQUEST_URI'] = "/relativebase/sub-page/";
-            Director::absoluteURL('subfolder/test')
-        );
 
-        // absolute base URLS with subdirectory - You should end them in a /
+            $this->assertEquals('/relativebase/', Director::baseURL());
-        Director::config()->set('alternate_base_url', 'http://www.example.org/relativebase/');
+            $this->assertEquals("http://www.somesite.com/relativebase{$slash}", Director::absoluteBaseURL());
-        $_SERVER['REQUEST_URI'] = "http://www.example.org/relativebase/sub-page/";
+            $this->assertEquals(
-        $this->assertEquals('/relativebase/', Director::baseURL()); // Non-absolute url
+                "http://www.somesite.com/relativebase/subfolder/test{$slash}",
-        $this->assertEquals('http://www.example.org/relativebase/', Director::absoluteBaseURL());
+                Director::absoluteURL('subfolder/test')
-        $this->assertEquals('http://www.example.org/relativebase/sub-page/', Director::absoluteURL('', Director::REQUEST));
+            );
-        $this->assertEquals('http://www.example.org/relativebase/', Director::absoluteURL('', Director::BASE));
-        $this->assertEquals('http://www.example.org/', Director::absoluteURL('', Director::ROOT));
-        $this->assertEquals(
-            'http://www.example.org/relativebase/sub-page/subfolder/test',
-            Director::absoluteURL('subfolder/test', Director::REQUEST)
-        );
-        $this->assertEquals(
-            'http://www.example.org/subfolder/test',
-            Director::absoluteURL('subfolder/test', Director::ROOT)
-        );
-        $this->assertEquals(
-            'http://www.example.org/relativebase/subfolder/test',
-            Director::absoluteURL('subfolder/test', Director::BASE)
-        );
 
-        // absolute base URLs - you should end them in a /
+            // absolute base URLS with subdirectory - You should end them in a /
-        Director::config()->set('alternate_base_url', 'http://www.example.org/');
+            Director::config()->set('alternate_base_url', 'http://www.example.org/relativebase/');
-        $_SERVER['REQUEST_URI'] = "http://www.example.org/sub-page/";
+            $_SERVER['REQUEST_URI'] = 'http://www.example.org/relativebase/sub-page/';
-        $this->assertEquals('/', Director::baseURL()); // Non-absolute url
+            $this->assertEquals('/relativebase/', Director::baseURL()); // Non-absolute url
-        $this->assertEquals('http://www.example.org/', Director::absoluteBaseURL());
+            $this->assertEquals("http://www.example.org/relativebase{$slash}", Director::absoluteBaseURL());
-        $this->assertEquals('http://www.example.org/sub-page/', Director::absoluteURL('', Director::REQUEST));
+            $this->assertEquals("http://www.example.org/relativebase/sub-page{$slash}", Director::absoluteURL('', Director::REQUEST));
-        $this->assertEquals('http://www.example.org/', Director::absoluteURL('', Director::BASE));
+            $this->assertEquals("http://www.example.org/relativebase{$slash}", Director::absoluteURL('', Director::BASE));
-        $this->assertEquals('http://www.example.org/', Director::absoluteURL('', Director::ROOT));
+            $this->assertEquals("http://www.example.org{$slash}", Director::absoluteURL('', Director::ROOT));
-        $this->assertEquals(
+            $this->assertEquals(
-            'http://www.example.org/sub-page/subfolder/test',
+                "http://www.example.org/relativebase/sub-page/subfolder/test{$slash}",
-            Director::absoluteURL('subfolder/test', Director::REQUEST)
+                Director::absoluteURL('subfolder/test', Director::REQUEST)
-        );
+            );
-        $this->assertEquals(
+            $this->assertEquals(
-            'http://www.example.org/subfolder/test',
+                "http://www.example.org/subfolder/test{$slash}",
-            Director::absoluteURL('subfolder/test', Director::ROOT)
+                Director::absoluteURL('subfolder/test', Director::ROOT)
-        );
+            );
-        $this->assertEquals(
+            $this->assertEquals(
-            'http://www.example.org/subfolder/test',
+                "http://www.example.org/relativebase/subfolder/test{$slash}",
-            Director::absoluteURL('subfolder/test', Director::BASE)
+                Director::absoluteURL('subfolder/test', Director::BASE)
-        );
+            );
+
+            // absolute base URLs - you should end them in a /
+            Director::config()->set('alternate_base_url', 'http://www.example.org/');
+            $_SERVER['REQUEST_URI'] = "http://www.example.org/sub-page/";
+            $this->assertEquals('/', Director::baseURL()); // Non-absolute url
+            $this->assertEquals("http://www.example.org{$slash}", Director::absoluteBaseURL());
+            $this->assertEquals("http://www.example.org/sub-page{$slash}", Director::absoluteURL('', Director::REQUEST));
+            $this->assertEquals("http://www.example.org{$slash}", Director::absoluteURL('', Director::BASE));
+            $this->assertEquals("http://www.example.org{$slash}", Director::absoluteURL('', Director::ROOT));
+            $this->assertEquals(
+                "http://www.example.org/sub-page/subfolder/test{$slash}",
+                Director::absoluteURL('subfolder/test', Director::REQUEST)
+            );
+            $this->assertEquals(
+                "http://www.example.org/subfolder/test{$slash}",
+                Director::absoluteURL('subfolder/test', Director::ROOT)
+            );
+            $this->assertEquals(
+                "http://www.example.org/subfolder/test{$slash}",
+                Director::absoluteURL('subfolder/test', Director::BASE)
+            );
+        }
     }
 
     /**

tests/php/Control/HTTPTest.php

@@ -244,36 +244,41 @@ class HTTPTest extends FunctionalTest
      */
     public function testAbsoluteURLsCSS()
     {
-        $this->withBaseURL(
+        foreach ([true, false] as $withTrailingSlash) {
-            'http://www.silverstripe.org/',
+            // The results should be the same with both settings, since files should never have trailing slashes.
-            function () {
+            Controller::config()->set('add_trailing_slash', $withTrailingSlash);
 
-                // background-image
+            $this->withBaseURL(
-                // Note that using /./ in urls is absolutely acceptable
+                'http://www.silverstripe.org/',
-                $this->assertEquals(
+                function () {
-                    '<div style="background-image: url(\'http://www.silverstripe.org/./images/mybackground.gif\');">' . 'Content</div>',
-                    HTTP::absoluteURLs('<div style="background-image: url(\'./images/mybackground.gif\');">Content</div>')
-                );
 
-                // background
+                    // background-image
-                $this->assertEquals(
+                    // Note that using /./ in urls is absolutely acceptable
-                    '<div style="background: url(\'http://www.silverstripe.org/images/mybackground.gif\');">Content</div>',
+                    $this->assertEquals(
-                    HTTP::absoluteURLs('<div style="background: url(\'images/mybackground.gif\');">Content</div>')
+                        '<div style="background-image: url(\'http://www.silverstripe.org/./images/mybackground.gif\');">' . 'Content</div>',
-                );
+                        HTTP::absoluteURLs('<div style="background-image: url(\'./images/mybackground.gif\');">Content</div>')
+                    );
 
-                // list-style-image
+                    // background
-                $this->assertEquals(
+                    $this->assertEquals(
-                    '<div style=\'background: url(http://www.silverstripe.org/list.png);\'>Content</div>',
+                        '<div style="background: url(\'http://www.silverstripe.org/images/mybackground.gif\');">Content</div>',
-                    HTTP::absoluteURLs('<div style=\'background: url(list.png);\'>Content</div>')
+                        HTTP::absoluteURLs('<div style="background: url(\'images/mybackground.gif\');">Content</div>')
-                );
+                    );
 
-                // list-style
+                    // list-style-image
-                $this->assertEquals(
+                    $this->assertEquals(
-                    '<div style=\'background: url("http://www.silverstripe.org/./assets/list.png");\'>Content</div>',
+                        '<div style=\'background: url(http://www.silverstripe.org/list.png);\'>Content</div>',
-                    HTTP::absoluteURLs('<div style=\'background: url("./assets/list.png");\'>Content</div>')
+                        HTTP::absoluteURLs('<div style=\'background: url(list.png);\'>Content</div>')
-                );
+                    );
-            }
+
-        );
+                    // list-style
+                    $this->assertEquals(
+                        '<div style=\'background: url("http://www.silverstripe.org/./assets/list.png");\'>Content</div>',
+                        HTTP::absoluteURLs('<div style=\'background: url("./assets/list.png");\'>Content</div>')
+                    );
+                }
+            );
+        }
     }
 
     /**
@@ -281,70 +286,75 @@ class HTTPTest extends FunctionalTest
      */
     public function testAbsoluteURLsAttributes()
     {
-        $this->withBaseURL(
+        foreach ([true, false] as $withTrailingSlash) {
-            'http://www.silverstripe.org/',
+            Controller::config()->set('add_trailing_slash', $withTrailingSlash);
-            function () {
+            $slash = $withTrailingSlash ? '/' : '';
-                //empty links
-                $this->assertEquals(
-                    '<a href="http://www.silverstripe.org/">test</a>',
-                    HTTP::absoluteURLs('<a href="">test</a>')
-                );
 
-                $this->assertEquals(
+            $this->withBaseURL(
-                    '<a href="http://www.silverstripe.org/">test</a>',
+                'http://www.silverstripe.org/',
-                    HTTP::absoluteURLs('<a href="/">test</a>')
+                function () use ($slash) {
-                );
+                    //empty links
+                    $this->assertEquals(
+                        "<a href=\"http://www.silverstripe.org{$slash}\">test</a>",
+                        HTTP::absoluteURLs('<a href="">test</a>')
+                    );
 
-                //relative
+                    $this->assertEquals(
-                $this->assertEquals(
+                        "<a href=\"http://www.silverstripe.org{$slash}\">test</a>",
-                    '<a href="http://www.silverstripe.org/">test</a>',
+                        HTTP::absoluteURLs('<a href="/">test</a>')
-                    HTTP::absoluteURLs('<a href="./">test</a>')
+                    );
-                );
-                $this->assertEquals(
-                    '<a href="http://www.silverstripe.org/">test</a>',
-                    HTTP::absoluteURLs('<a href=".">test</a>')
-                );
 
-                // links
+                    //relative
-                $this->assertEquals(
+                    $this->assertEquals(
-                    '<a href=\'http://www.silverstripe.org/blog/\'>SS Blog</a>',
+                        "<a href=\"http://www.silverstripe.org{$slash}\">test</a>",
-                    HTTP::absoluteURLs('<a href=\'/blog/\'>SS Blog</a>')
+                        HTTP::absoluteURLs('<a href="./">test</a>')
-                );
+                    );
+                    $this->assertEquals(
+                        "<a href=\"http://www.silverstripe.org{$slash}\">test</a>",
+                        HTTP::absoluteURLs('<a href=".">test</a>')
+                    );
 
-                // background
+                    // links
-                // Note that using /./ in urls is absolutely acceptable
+                    $this->assertEquals(
-                $this->assertEquals(
+                        "<a href='http://www.silverstripe.org/blog{$slash}'>SS Blog</a>",
-                    '<div background="http://www.silverstripe.org/./themes/silverstripe/images/nav-bg-repeat-2.png">' . 'SS Blog</div>',
+                        HTTP::absoluteURLs('<a href=\'/blog/\'>SS Blog</a>')
-                    HTTP::absoluteURLs('<div background="./themes/silverstripe/images/nav-bg-repeat-2.png">SS Blog</div>')
+                    );
-                );
 
-                //check dot segments
+                    // background
-                // Assumption: dots are not removed
+                    // Note that using /./ in urls is absolutely acceptable
-                //if they were, the url should be: http://www.silverstripe.org/abc
+                    $this->assertEquals(
-                $this->assertEquals(
+                        '<div background="http://www.silverstripe.org/./themes/silverstripe/images/nav-bg-repeat-2.png">' . 'SS Blog</div>',
-                    '<a href="http://www.silverstripe.org/test/page/../../abc">Test</a>',
+                        HTTP::absoluteURLs('<div background="./themes/silverstripe/images/nav-bg-repeat-2.png">SS Blog</div>')
-                    HTTP::absoluteURLs('<a href="test/page/../../abc">Test</a>')
+                    );
-                );
 
-                // image
+                    //check dot segments
-                $this->assertEquals(
+                    // Assumption: dots are not removed
-                    '<img src=\'http://www.silverstripe.org/themes/silverstripe/images/logo-org.png\' />',
+                    //if they were, the url should be: http://www.silverstripe.org/abc
-                    HTTP::absoluteURLs('<img src=\'themes/silverstripe/images/logo-org.png\' />')
+                    $this->assertEquals(
-                );
+                        "<a href=\"http://www.silverstripe.org/test/page/../../abc{$slash}\">Test</a>",
+                        HTTP::absoluteURLs('<a href="test/page/../../abc">Test</a>')
+                    );
 
-                // link
+                    // image
-                $this->assertEquals(
+                    $this->assertEquals(
-                    '<link href=http://www.silverstripe.org/base.css />',
+                        '<img src=\'http://www.silverstripe.org/themes/silverstripe/images/logo-org.png\' />',
-                    HTTP::absoluteURLs('<link href=base.css />')
+                        HTTP::absoluteURLs('<img src=\'themes/silverstripe/images/logo-org.png\' />')
-                );
+                    );
 
-                // Test special characters are retained
+                    // link
-                $this->assertEquals(
+                    $this->assertEquals(
-                    '<a href="http://www.silverstripe.org/Security/changepassword?m=3&amp;t=7214fdfde">password reset link</a>',
+                        '<link href=http://www.silverstripe.org/base.css />',
-                    HTTP::absoluteURLs('<a href="/Security/changepassword?m=3&amp;t=7214fdfde">password reset link</a>')
+                        HTTP::absoluteURLs('<link href=base.css />')
-                );
+                    );
-            }
+
-        );
+                    // Test special characters are retained
+                    $this->assertEquals(
+                        "<a href=\"http://www.silverstripe.org/Security/changepassword{$slash}?m=3&amp;t=7214fdfde\">password reset link</a>",
+                        HTTP::absoluteURLs('<a href="/Security/changepassword?m=3&amp;t=7214fdfde">password reset link</a>')
+                    );
+                }
+            );
+        }
     }
 
     /**

tests/php/Control/Middleware/CanonicalURLMiddlewareTest.php

@@ -2,44 +2,28 @@
 
 namespace SilverStripe\Control\Tests\Middleware;
 
+use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Control\Middleware\CanonicalURLMiddleware;
+use SilverStripe\Core\Environment;
 use SilverStripe\Dev\SapphireTest;
 
 class CanonicalURLMiddlewareTest extends SapphireTest
 {
-    /**
-     * Stub middleware class, partially mocked
-     *
-     * @var CanonicalURLMiddleware
-     */
-    protected $middleware;
-
-    protected function setUp(): void
-    {
-        parent::setUp();
-
-        /** @var CanonicalURLMiddleware $middleware */
-        $this->middleware = $this->getMockBuilder(CanonicalURLMiddleware::class)
-            ->setMethods(['getRedirect'])
-            ->getMock();
-
-        $this->middleware->expects($this->any())->method('getRedirect')->willReturn(false);
-
-        $this->middleware->setForceBasicAuthToSSL(true);
-    }
 
     public function testHttpsIsForcedForBasicAuth()
     {
-        $this->middleware->expects($this->once())->method('getRedirect');
+        $middleware = $this->getMockedMiddleware();
+        $middleware->expects($this->once())->method('getRedirect');
 
         $request = new HTTPRequest('GET', '/');
+        $request->addHeader('host', 'www.example.com');
         $mockResponse = (new HTTPResponse)
             ->addHeader('WWW-Authenticate', 'basic')
             ->setStatusCode(401);
 
-        $result = $this->middleware->process($request, function () use ($mockResponse) {
+        $result = $middleware->process($request, function () use ($mockResponse) {
             return $mockResponse;
         });
 
@@ -50,15 +34,17 @@ class CanonicalURLMiddlewareTest extends SapphireTest
 
     public function testMiddlewareDelegateIsReturnedWhenBasicAuthRedirectIsDisabled()
     {
-        $this->middleware->expects($this->once())->method('getRedirect');
+        $middleware = $this->getMockedMiddleware();
-        $this->middleware->setForceBasicAuthToSSL(false);
+        $middleware->expects($this->once())->method('getRedirect');
+        $middleware->setForceBasicAuthToSSL(false);
 
         $request = new HTTPRequest('GET', '/');
+        $request->addHeader('host', 'www.example.com');
         $mockResponse = (new HTTPResponse)
             ->addHeader('WWW-Authenticate', 'basic')
             ->setStatusCode(401);
 
-        $result = $this->middleware->process($request, function () use ($mockResponse) {
+        $result = $middleware->process($request, function () use ($mockResponse) {
             return $mockResponse;
         });
         $this->assertSame($mockResponse, $result, 'Response returned verbatim with auto redirect disabled');
@@ -66,12 +52,14 @@ class CanonicalURLMiddlewareTest extends SapphireTest
 
     public function testMiddlewareDelegateIsReturnedWhenNoBasicAuthIsPresent()
     {
-        $this->middleware->expects($this->once())->method('getRedirect');
+        $middleware = $this->getMockedMiddleware();
+        $middleware->expects($this->once())->method('getRedirect');
 
         $request = new HTTPRequest('GET', '/');
+        $request->addHeader('host', 'www.example.com');
         $mockResponse = (new HTTPResponse)->addHeader('Foo', 'bar');
 
-        $result = $this->middleware->process($request, function () use ($mockResponse) {
+        $result = $middleware->process($request, function () use ($mockResponse) {
             return $mockResponse;
         });
 
@@ -80,19 +68,125 @@ class CanonicalURLMiddlewareTest extends SapphireTest
 
     public function testGetForceBasicAuthToSSL()
     {
-        $this->middleware->setForceBasicAuthToSSL(null);
+        $middleware = $this->getMockedMiddleware();
+        $middleware->setForceBasicAuthToSSL(null);
 
-        $this->middleware->setForceSSL(true);
+        $middleware->setForceSSL(true);
-        $this->assertTrue($this->middleware->getForceBasicAuthToSSL(), 'Default falls over to forceSSL');
+        $this->assertTrue($middleware->getForceBasicAuthToSSL(), 'Default falls over to forceSSL');
 
-        $this->middleware->setForceSSL(false);
+        $middleware->setForceSSL(false);
-        $this->assertFalse($this->middleware->getForceBasicAuthToSSL(), 'Default falls over to forceSSL');
+        $this->assertFalse($middleware->getForceBasicAuthToSSL(), 'Default falls over to forceSSL');
 
-        $this->middleware->setForceBasicAuthToSSL(true);
+        $middleware->setForceBasicAuthToSSL(true);
-        $this->assertTrue($this->middleware->getForceBasicAuthToSSL(), 'Explicitly set is returned');
+        $this->assertTrue($middleware->getForceBasicAuthToSSL(), 'Explicitly set is returned');
 
-        $this->middleware->setForceBasicAuthToSSL(false);
+        $middleware->setForceBasicAuthToSSL(false);
-        $this->middleware->setForceSSL(true);
+        $middleware->setForceSSL(true);
-        $this->assertFalse($this->middleware->getForceBasicAuthToSSL(), 'Explicitly set is returned');
+        $this->assertFalse($middleware->getForceBasicAuthToSSL(), 'Explicitly set is returned');
+    }
+
+    public function testRedirectTrailingSlash()
+    {
+        $testScenarios = [
+            [
+                'forceRedirect' => true,
+                'addTrailingSlash' => true,
+                'requestHasSlash' => true,
+            ],
+            [
+                'forceRedirect' => true,
+                'addTrailingSlash' => true,
+                'requestHasSlash' => false,
+            ],
+            [
+                'forceRedirect' => true,
+                'addTrailingSlash' => false,
+                'requestHasSlash' => true,
+            ],
+            [
+                'forceRedirect' => true,
+                'addTrailingSlash' => false,
+                'requestHasSlash' => false,
+            ],
+            [
+                'forceRedirect' => false,
+                'addTrailingSlash' => true,
+                'requestHasSlash' => true,
+            ],
+            [
+                'forceRedirect' => false,
+                'addTrailingSlash' => true,
+                'requestHasSlash' => false,
+            ],
+            [
+                'forceRedirect' => false,
+                'addTrailingSlash' => false,
+                'requestHasSlash' => true,
+            ],
+            [
+                'forceRedirect' => false,
+                'addTrailingSlash' => false,
+                'requestHasSlash' => false,
+            ],
+        ];
+        foreach ($testScenarios as $scenario) {
+            $forceRedirect = $scenario['forceRedirect'];
+            $addTrailingSlash = $scenario['addTrailingSlash'];
+            $requestHasSlash = $scenario['requestHasSlash'];
+
+            $middleware = $this->getMockedMiddleware(false);
+
+            $middleware->setEnforceTrailingSlashConfig($forceRedirect);
+            Controller::config()->set('add_trailing_slash', $addTrailingSlash);
+
+            $requestSlash = $requestHasSlash ? '/' : '';
+            $requestURL = "/about-us{$requestSlash}";
+
+            Environment::setEnv('REQUEST_URI', $requestURL);
+            $request = new HTTPRequest('GET', $requestURL);
+            $request->setScheme('https');
+            $request->addHeader('host', 'www.example.com');
+            $mockResponse = (new HTTPResponse)
+                ->setStatusCode(200);
+
+            $result = $middleware->process($request, function () use ($mockResponse) {
+                return $mockResponse;
+            });
+
+            $noRedirect = !$forceRedirect || ($addTrailingSlash && $requestHasSlash) || (!$addTrailingSlash && !$requestHasSlash);
+            if ($noRedirect) {
+                $this->assertNull($result->getHeader('Location'), 'No location header should be added');
+                $this->assertEquals(200, $result->getStatusCode(), 'No redirection should be made');
+            } else {
+                $this->assertEquals(301, $result->getStatusCode(), 'Responses should be redirected to include/omit trailing slash');
+                if ($addTrailingSlash) {
+                    $this->assertStringEndsWith('/', $result->getHeader('Location'), 'Trailing slash should be added');
+                } else {
+                    $this->assertStringEndsNotWith('/', $result->getHeader('Location'), 'Trailing slash should be removed');
+                }
+            }
+        }
+    }
+
+    private function getMockedMiddleware($mockGetRedirect = true): CanonicalURLMiddleware
+    {
+        $mockedMethods = ['isEnabled'];
+        if ($mockGetRedirect) {
+            $mockedMethods[] = 'getRedirect';
+        }
+
+        /** @var CanonicalURLMiddleware $middleware */
+        $middleware = $this->getMockBuilder(CanonicalURLMiddleware::class)
+            ->setMethods($mockedMethods)
+            ->getMock();
+
+        $middleware->expects($this->any())->method('isEnabled')->willReturn(true);
+        if ($mockGetRedirect) {
+            $middleware->expects($this->any())->method('getRedirect')->willReturn(false);
+        }
+
+        $middleware->setForceBasicAuthToSSL(true);
+
+        return $middleware;
     }
 }

tests/php/Control/RSS/RSSFeedTest.php

@@ -24,7 +24,7 @@ class RSSFeedTest extends SapphireTest
         $rssFeed = new RSSFeed($list, "http://www.example.com", "Test RSS Feed", "Test RSS Feed Description");
         $content = $rssFeed->outputToBrowser();
 
-        $this->assertStringContainsString('<link>http://www.example.org/item-a/</link>', $content);
+        $this->assertStringContainsString('<link>http://www.example.org/item-a</link>', $content);
         $this->assertStringContainsString('<link>http://www.example.com/item-b.html</link>', $content);
         $this->assertStringContainsString('<link>http://www.example.com/item-c.html</link>', $content);
 
@@ -60,9 +60,9 @@ class RSSFeedTest extends SapphireTest
     public function testLinkEncoding()
     {
         $list = new ArrayList();
-        $rssFeed = new RSSFeed($list, "http://www.example.com/?param1=true&param2=true", "Test RSS Feed");
+        $rssFeed = new RSSFeed($list, "http://www.example.com?param1=true&param2=true", "Test RSS Feed");
         $content = $rssFeed->outputToBrowser();
-        $this->assertStringContainsString('<link>http://www.example.com/?param1=true&amp;param2=true', $content);
+        $this->assertStringContainsString('<link>http://www.example.com?param1=true&amp;param2=true', $content);
     }
 
     public function testRSSFeedWithShortcode()

tests/php/Forms/FormFactoryTest.php

@@ -56,7 +56,7 @@ class FormFactoryTest extends SapphireTest
         $previewLink = $form->Fields()->fieldByName('PreviewLink');
         $this->assertInstanceOf(LiteralField::class, $previewLink);
         $this->assertEquals(
-            '<a href="FormFactoryTest_TestController/preview/" rel="external" target="_blank">Preview</a>',
+            '<a href="FormFactoryTest_TestController/preview" rel="external" target="_blank">Preview</a>',
             $previewLink->getContent()
         );
 

tests/php/Forms/HTMLEditor/TinyMCECombinedGeneratorTest.php

@@ -62,7 +62,7 @@ class TinyMCECombinedGeneratorTest extends SapphireTest
         $this->assertMatchesRegularExpression('#_tinymce/tinymce-testconfig-[0-9a-z]{10,10}#', $generator->generateFilename($c));
         $content = $generator->generateContent($c);
         $this->assertStringContainsString(
-            "var baseURL = baseTag.length ? baseTag[0].baseURI : 'http://www.mysite.com/basedir/';\n",
+            "var baseURL = baseTag.length ? baseTag[0].baseURI : 'http://www.mysite.com/basedir';\n",
             $content
         );
         // Main script file

tests/php/ORM/PaginatedListTest.php

@@ -344,7 +344,8 @@ class PaginatedListTest extends SapphireTest
     public function testFirstLink()
     {
         $list = new PaginatedList(new ArrayList());
-        $this->assertStringContainsString('start=0', $list->FirstLink());
+        $link = $list->FirstLink();
+        $this->assertStringContainsString('start=0', $link);
     }
 
     public function testFirstLinkContainsCurrentGetParameters()

tests/php/Security/SecurityTest.php

@@ -332,7 +332,7 @@ class SecurityTest extends FunctionalTest
         $this->get(
             Config::inst()->get(Security::class, 'logout_url'),
             null,
-            ['Referer' => Director::absoluteBaseURL() . 'testpage']
+            ['Referer' => Controller::join_links(Director::absoluteBaseURL(), 'testpage')]
         );
 
         /* Make sure the user is still logged in */
@@ -388,11 +388,11 @@ class SecurityTest extends FunctionalTest
         $response = $this->doTestLoginForm(
             'noexpiry@silverstripe.com',
             '1nitialPassword',
-            Director::absoluteBaseURL() . 'testpage'
+            Controller::join_links(Director::absoluteBaseURL(), 'testpage')
         );
         // for some reason the redirect happens to a relative URL
         $this->assertMatchesRegularExpression(
-            '/^' . preg_quote(Director::absoluteBaseURL() ?? '', '/') . 'testpage/',
+            '/^' . preg_quote(Controller::join_links(Director::absoluteBaseURL(), 'testpage'), '/') . '/',
             $response->getHeader('Location'),
             "Internal absolute BackURLs work when passed through to login form"
         );

tests/php/Security/SecurityTokenTest.php

@@ -123,14 +123,14 @@ class SecurityTokenTest extends SapphireTest
     {
         $t = new SecurityToken();
 
-        $url = 'http://absolute.tld/action/';
+        $url = 'http://absolute.tld/action';
         $this->assertEquals(
             sprintf('%s?%s=%s', $url, $t->getName(), $t->getValue()),
             $t->addToUrl($url),
             'Urls without existing GET parameters'
         );
 
-        $url = 'http://absolute.tld/?getparam=1';
+        $url = 'http://absolute.tld?getparam=1';
         $this->assertEquals(
             sprintf('%s&%s=%s', $url, $t->getName(), $t->getValue()),
             $t->addToUrl($url),