diff --git a/src/Control/Controller.php b/src/Control/Controller.php
index fa3ae3f3666..a69b269f6d6 100644
--- a/src/Control/Controller.php
+++ b/src/Control/Controller.php
@@ -89,6 +89,11 @@ public function __construct()
$this->setResponse(HTTPResponse::create());
}
+ /**
+ * 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;
+
/**
* Initialisation function that is run before any action on the controller is called.
*
@@ -648,6 +653,7 @@ public static function join_links($arg = null)
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 @@ public static function join_links($arg = null)
}
}
+ $result = static::normaliseTrailingSlash($result);
+
if ($queryargs) {
$result .= '?' . http_build_query($queryargs ?? []);
}
@@ -669,6 +677,52 @@ public static function join_links($arg = null)
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
*/
diff --git a/src/Control/Director.php b/src/Control/Director.php
index cd63dc5e351..fe7fd66f7b8 100644
--- a/src/Control/Director.php
+++ b/src/Control/Director.php
@@ -400,13 +400,13 @@ public static function absoluteURL(string $url, string $relativeParent = self::B
{
// 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
@@ -878,10 +878,11 @@ public static function fileExists($file)
*/
public static function absoluteBaseURL()
{
- return self::absoluteURL(
+ $baseURL = self::absoluteURL(
self::baseURL(),
self::ROOT
);
+ return Controller::normaliseTrailingSlash($baseURL);
}
/**
diff --git a/src/Control/HTTP.php b/src/Control/HTTP.php
index 4ccf5d7b7f3..94bed387037 100644
--- a/src/Control/HTTP.php
+++ b/src/Control/HTTP.php
@@ -172,7 +172,7 @@ public static function setGetVar($varname, $varvalue, $currentURL = null, $separ
$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;
}
diff --git a/src/Control/Middleware/CanonicalURLMiddleware.php b/src/Control/Middleware/CanonicalURLMiddleware.php
index 976f33ca653..34a697f6022 100644
--- a/src/Control/Middleware/CanonicalURLMiddleware.php
+++ b/src/Control/Middleware/CanonicalURLMiddleware.php
@@ -4,11 +4,12 @@
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 @@
* - 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.
+ */
+ private static bool $force_trailing_slash = true;
+
+ /**
+ * If force_trailing_slash is enabled, this is the list of paths that are ignored
+ */
+ private static array $force_trailing_slash_ignore_paths = [
+ 'admin/',
+ 'dev/',
+ ];
+
+ /**
+ * If force_trailing_slash is enabled, this is the list of user agents that are ignored
+ */
+ private static array $force_trailing_slash_ignore_user_agents = [];
/**
* Set if we should redirect to WWW
@@ -214,6 +236,7 @@ protected function getRedirect(HTTPRequest $request)
// 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 +251,16 @@ protected function getRedirect(HTTPRequest $request)
$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 +340,72 @@ protected function requiresSSL(HTTPRequest $request)
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 (self::config()->uninherited('force_trailing_slash') !== 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) self::config()->uninherited('force_trailing_slash_ignore_paths');
+ 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) self::config()->uninherited('force_trailing_slash_ignore_user_agents');
+ 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 +454,7 @@ public function setEnabledEnvs($enabledEnvs)
protected function isEnabled()
{
// At least one redirect must be enabled
- if (!$this->getForceWWW() && !$this->getForceSSL()) {
+ if (!$this->getForceWWW() && !$this->getForceSSL() && self::config()->uninherited('force_trailing_slash') !== true) {
return false;
}
diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php
index 2052b01bae8..2c47efa6de6 100644
--- a/src/Control/RequestHandler.php
+++ b/src/Control/RequestHandler.php
@@ -561,7 +561,7 @@ public function Link($action = null)
// 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);
diff --git a/src/Dev/DebugView.php b/src/Dev/DebugView.php
index b6bd389d169..5c380a7bf6f 100644
--- a/src/Dev/DebugView.php
+++ b/src/Dev/DebugView.php
@@ -135,8 +135,9 @@ public function Breadcrumbs()
$pathLinks = [];
foreach ($parts as $part) {
if ($part != '') {
- $pathPart .= "$part/";
- $pathLinks[] = "$part";
+ $pathPart = Controller::join_links($pathPart, $part);
+ $href = Controller::join_links($base, $pathPart);
+ $pathLinks[] = "$part";
}
}
return implode(' → ', $pathLinks);
diff --git a/src/Dev/TaskRunner.php b/src/Dev/TaskRunner.php
index c8ae71a6817..203030fc42e 100644
--- a/src/Dev/TaskRunner.php
+++ b/src/Dev/TaskRunner.php
@@ -76,7 +76,7 @@ public function index()
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'],
]));
diff --git a/src/Forms/HTMLEditor/TinyMCEConfig.php b/src/Forms/HTMLEditor/TinyMCEConfig.php
index 80379fc5046..8e85d8e7430 100644
--- a/src/Forms/HTMLEditor/TinyMCEConfig.php
+++ b/src/Forms/HTMLEditor/TinyMCEConfig.php
@@ -651,7 +651,7 @@ protected function getConfig()
$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();
diff --git a/src/View/SSViewer.php b/src/View/SSViewer.php
index 61e74affe9b..c74766515d3 100644
--- a/src/View/SSViewer.php
+++ b/src/View/SSViewer.php
@@ -825,7 +825,8 @@ public function setTemplateFile($type, $file)
*/
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('/]+xhtml/i', $contentGeneratedSoFar ?? '')) {
diff --git a/tests/php/Control/ControllerTest.php b/tests/php/Control/ControllerTest.php
index 34559386e9b..ee1222dda08 100644
--- a/tests/php/Control/ControllerTest.php
+++ b/tests/php/Control/ControllerTest.php
@@ -59,7 +59,7 @@ protected function setUp(): void
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 @@ public function testUndefinedActions()
public function testAllowedActions()
{
- $response = $this->get("UnsecuredController/");
+ $response = $this->get("UnsecuredController");
$this->assertEquals(
200,
$response->getStatusCode(),
@@ -115,7 +115,7 @@ public function testAllowedActions()
'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 @@ public function testAllowedActions()
'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 @@ public function testAllowedActions()
"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 @@ public function testAllowedActions()
);
$this->logInAs('admin');
- $response = $this->get('IndexSecuredController/');
+ $response = $this->get('IndexSecuredController');
$this->assertEquals(
200,
$response->getStatusCode(),
@@ -276,18 +276,41 @@ public function testJoinLinks()
{
/* 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 @@ public function testJoinLinks()
);
/* 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",
- Controller::join_links(["?a=1", "admin/crm", "?b=2", "MyForm?c=3"])
+ "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"])
+ );
+ 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 @@ public function testRedirectBackByBackUrl()
);
// 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'));
diff --git a/tests/php/Control/DirectorTest.php b/tests/php/Control/DirectorTest.php
index 85a0979b185..bd97877e73c 100644
--- a/tests/php/Control/DirectorTest.php
+++ b/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 @@ public function testFileExists()
public function testAbsoluteURL()
{
- Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/mysite/');
- $_SERVER['REQUEST_URI'] = "http://www.mysite.com:9090/mysite/sub-page/";
+ foreach ([true, false] as $withTrailingSlash) {
+ Controller::config()->set('add_trailing_slash', $withTrailingSlash);
+ $slash = $withTrailingSlash ? '/' : '';
- //test empty / local urls
- foreach (['', './', '.'] as $url) {
- $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));
- $this->assertEquals("http://www.mysite.com:9090/mysite/sub-page/", Director::absoluteURL($url, Director::REQUEST));
- }
+ Director::config()->set('alternate_base_url', 'http://www.mysite.com:9090/mysite/');
+ $_SERVER['REQUEST_URI'] = "http://www.mysite.com:9090/mysite/sub-page/";
+
+ //test empty / local urls
+ 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/", 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")'));
+ // 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")'));
+ }
}
public function testAlternativeBaseURL()
{
- // relative base URLs - you should end them in a /
- Director::config()->set('alternate_base_url', '/relativebase/');
- $_SERVER['HTTP_HOST'] = 'www.somesite.com';
- $_SERVER['REQUEST_URI'] = "/relativebase/sub-page/";
-
- $this->assertEquals('/relativebase/', Director::baseURL());
- $this->assertEquals('http://www.somesite.com/relativebase/', Director::absoluteBaseURL());
- $this->assertEquals(
- 'http://www.somesite.com/relativebase/subfolder/test',
- Director::absoluteURL('subfolder/test')
- );
-
- // absolute base URLS with subdirectory - You should end them in a /
- Director::config()->set('alternate_base_url', 'http://www.example.org/relativebase/');
- $_SERVER['REQUEST_URI'] = "http://www.example.org/relativebase/sub-page/";
- $this->assertEquals('/relativebase/', Director::baseURL()); // Non-absolute url
- $this->assertEquals('http://www.example.org/relativebase/', Director::absoluteBaseURL());
- $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 /
- 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/', Director::absoluteBaseURL());
- $this->assertEquals('http://www.example.org/sub-page/', Director::absoluteURL('', Director::REQUEST));
- $this->assertEquals('http://www.example.org/', Director::absoluteURL('', Director::BASE));
- $this->assertEquals('http://www.example.org/', Director::absoluteURL('', Director::ROOT));
- $this->assertEquals(
- 'http://www.example.org/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/subfolder/test',
- Director::absoluteURL('subfolder/test', Director::BASE)
- );
+ foreach ([true, false] as $withTrailingSlash) {
+ Controller::config()->set('add_trailing_slash', $withTrailingSlash);
+ $slash = $withTrailingSlash ? '/' : '';
+
+ // relative base URLs - you should end them in a /
+ Director::config()->set('alternate_base_url', '/relativebase/');
+ $_SERVER['HTTP_HOST'] = 'www.somesite.com';
+ $_SERVER['REQUEST_URI'] = "/relativebase/sub-page/";
+
+ $this->assertEquals('/relativebase/', Director::baseURL());
+ $this->assertEquals("http://www.somesite.com/relativebase{$slash}", Director::absoluteBaseURL());
+ $this->assertEquals(
+ "http://www.somesite.com/relativebase/subfolder/test{$slash}",
+ Director::absoluteURL('subfolder/test')
+ );
+
+ // absolute base URLS with subdirectory - You should end them in a /
+ Director::config()->set('alternate_base_url', 'http://www.example.org/relativebase/');
+ $_SERVER['REQUEST_URI'] = 'http://www.example.org/relativebase/sub-page/';
+ $this->assertEquals('/relativebase/', Director::baseURL()); // Non-absolute url
+ $this->assertEquals("http://www.example.org/relativebase{$slash}", Director::absoluteBaseURL());
+ $this->assertEquals("http://www.example.org/relativebase/sub-page{$slash}", Director::absoluteURL('', Director::REQUEST));
+ $this->assertEquals("http://www.example.org/relativebase{$slash}", Director::absoluteURL('', Director::BASE));
+ $this->assertEquals("http://www.example.org{$slash}", Director::absoluteURL('', Director::ROOT));
+ $this->assertEquals(
+ "http://www.example.org/relativebase/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/relativebase/subfolder/test{$slash}",
+ 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)
+ );
+ }
}
/**
diff --git a/tests/php/Control/HTTPTest.php b/tests/php/Control/HTTPTest.php
index 5f748771c7d..a1b13879efa 100644
--- a/tests/php/Control/HTTPTest.php
+++ b/tests/php/Control/HTTPTest.php
@@ -307,7 +307,7 @@ function () {
// links
$this->assertEquals(
- 'SS Blog',
+ 'SS Blog',
HTTP::absoluteURLs('SS Blog')
);
diff --git a/tests/php/Control/RSS/RSSFeedTest.php b/tests/php/Control/RSS/RSSFeedTest.php
index 45d772ac54a..1e86e87cc38 100644
--- a/tests/php/Control/RSS/RSSFeedTest.php
+++ b/tests/php/Control/RSS/RSSFeedTest.php
@@ -24,7 +24,7 @@ public function testRSSFeed()
$rssFeed = new RSSFeed($list, "http://www.example.com", "Test RSS Feed", "Test RSS Feed Description");
$content = $rssFeed->outputToBrowser();
- $this->assertStringContainsString('http://www.example.org/item-a/', $content);
+ $this->assertStringContainsString('http://www.example.org/item-a', $content);
$this->assertStringContainsString('http://www.example.com/item-b.html', $content);
$this->assertStringContainsString('http://www.example.com/item-c.html', $content);
diff --git a/tests/php/Forms/FormFactoryTest.php b/tests/php/Forms/FormFactoryTest.php
index 8cac9edc463..6ab9d99427c 100644
--- a/tests/php/Forms/FormFactoryTest.php
+++ b/tests/php/Forms/FormFactoryTest.php
@@ -56,7 +56,7 @@ public function testVersionedForm()
$previewLink = $form->Fields()->fieldByName('PreviewLink');
$this->assertInstanceOf(LiteralField::class, $previewLink);
$this->assertEquals(
- 'Preview',
+ 'Preview',
$previewLink->getContent()
);
diff --git a/tests/php/Security/SecurityTest.php b/tests/php/Security/SecurityTest.php
index 8ab99e0cc7b..ec3d6723331 100644
--- a/tests/php/Security/SecurityTest.php
+++ b/tests/php/Security/SecurityTest.php
@@ -332,7 +332,7 @@ public function testLogout()
$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 @@ public function testExternalBackUrlRedirectionDisallowed()
$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"
);
diff --git a/tests/php/Security/SecurityTokenTest.php b/tests/php/Security/SecurityTokenTest.php
index df2cee25c47..cb8f8ae19da 100644
--- a/tests/php/Security/SecurityTokenTest.php
+++ b/tests/php/Security/SecurityTokenTest.php
@@ -123,7 +123,7 @@ public function testAddToUrl()
{
$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),