Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

SS5: add trailing slash config and redirect #10538

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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/Control/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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] != '/') {
Expand All @@ -658,6 +664,8 @@ public static function join_links($arg = null)
}
}

$result = static::normaliseTrailingSlash($result);

if ($queryargs) {
$result .= '?' . http_build_query($queryargs ?? []);
}
Expand All @@ -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
*/
Expand Down
9 changes: 5 additions & 4 deletions src/Control/Director.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -686,7 +686,7 @@ public static function makeRelative($url)
$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'];
}

Expand Down Expand Up @@ -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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/Control/HTTP.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -212,7 +212,7 @@ public static function setGetVar($varname, $varvalue, $currentURL = null, $separ
$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;
Expand Down
136 changes: 133 additions & 3 deletions src/Control/Middleware/CanonicalURLMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.
*/
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
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -214,6 +269,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)) {
Expand All @@ -228,8 +284,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;
}

Expand Down Expand Up @@ -309,6 +373,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 ($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
*/
Expand Down Expand Up @@ -357,7 +487,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() && $this->getEnforceTrailingSlashConfig() !== true) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Control/RequestHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/Dev/DebugView.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,9 @@ public function Breadcrumbs()
$pathLinks = [];
foreach ($parts as $part) {
if ($part != '') {
$pathPart .= "$part/";
$pathLinks[] = "<a href=\"$base$pathPart\">$part</a>";
$pathPart = Controller::join_links($pathPart, $part);
$href = Controller::join_links($base, $pathPart);
$pathLinks[] = "<a href=\"$href\">$part</a>";
}
}
return implode('&nbsp;&rarr;&nbsp;', $pathLinks);
Expand Down
2 changes: 1 addition & 1 deletion src/Dev/TaskRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
]));
Expand Down
2 changes: 1 addition & 1 deletion src/Forms/HTMLEditor/TinyMCEConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion src/View/SSViewer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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('/<!DOCTYPE[^>]+xhtml/i', $contentGeneratedSoFar ?? '')) {
Expand Down
Loading