ENH Allow base URL to not have trailing slash

client/src/boot/BootRoutes.js

@@ -16,6 +16,7 @@ import i18n from 'i18n';
 import { isDirty } from 'redux-form';
 import getFormState from 'lib/getFormState';
 import { Routes, Route } from 'react-router';
+import { joinUrlPaths } from 'lib/urls';
 
 /**
  * Bootstraps routes
@@ -115,7 +116,7 @@ class BootRoutes {
     ReactDOM.createRoot(document.getElementsByClassName('cms-content')[0]).render(
       <ApolloProvider client={this.client}>
         <ReduxProvider store={this.store}>
-          <BrowserRouter basename={`${Config.get('baseUrl')}${Config.get('adminUrl')}`}>
+          <BrowserRouter basename={joinUrlPaths(Config.get('baseUrl'), Config.get('adminUrl'))}>
             <Routes>
               <Route path={rootRoute.path} element={<rootRoute.component />}>{routes}</Route>
             </Routes>

client/src/boot/apollo/buildNetworkComponents.js

@@ -1,10 +1,11 @@
 import { ApolloLink, HttpLink } from '@apollo/client';
 import { onError } from '@apollo/client/link/error';
 import Config from 'lib/Config';
+import { joinUrlPaths } from 'lib/urls';
 
 const buildNetworkComponents = (baseUrl) => {
   const httpLink = new HttpLink({
-    uri: `${baseUrl}admin/graphql`,
+    uri: joinUrlPaths(baseUrl, 'admin/graphql'),
     fetchOptions: {
       credentials: 'same-origin',
       headers: {

client/src/boot/apollo/getGraphqlFragments.js

@@ -1,4 +1,5 @@
 import fetch from 'isomorphic-fetch';
+import { joinUrlPaths } from 'lib/urls';
 
 const parseResponse = result => {
   const fragmentData = result.data;
@@ -25,13 +26,13 @@ const getGraphqlFragments = async (baseUrl, preferStatic = true) => {
   const isLegacy = !!document.body.getAttribute('data-graphql-legacy');
 
   const urls = [
-    `${baseUrl}_graphql/admin.types.graphql`,
-    `${baseUrl}admin.types.graphql`,
+    joinUrlPaths(baseUrl, '_graphql/admin.types.graphql'),
+    joinUrlPaths(baseUrl, 'admin.types.graphql'),
   ];
 
   const legacyURLs = [
-    `${baseUrl}admin/graphql/types`,
-    `${baseUrl}assets/admin.types.graphql`,
+    joinUrlPaths(baseUrl, 'admin/graphql/types'),
+    joinUrlPaths(baseUrl, 'assets/admin.types.graphql'),
   ];
 
   let primaryURL;
@@ -51,7 +52,7 @@ const getGraphqlFragments = async (baseUrl, preferStatic = true) => {
     headers: {
       'Content-Type': 'application/json'
     },
-    uri: `${baseUrl}`,
+    uri: baseUrl,
     credentials: 'same-origin',
   };
 

client/src/bundles/bundle.js

@@ -78,6 +78,7 @@ import 'expose-loader?exposes=ShortcodeSerialiser!lib/ShortcodeSerialiser';
 import 'expose-loader?exposes=formatWrittenNumber!lib/formatWrittenNumber';
 import 'expose-loader?exposes=withDragDropContext!lib/withDragDropContext';
 import 'expose-loader?exposes=withRouter!lib/withRouter';
+import 'expose-loader?exposes=ssUrlLib!lib/urls';
 
 // Legacy CMS
 import '../legacy/jquery.changetracker';

client/src/legacy/HtmlEditorField.js

@@ -8,6 +8,7 @@
 
 import jQuery from 'jquery';
 import 'events-polyfill';
+import escapeRegExp from 'lodash.escaperegexp';
 
 var ss = typeof window.ss !== 'undefined' ? window.ss : {};
 
@@ -289,7 +290,7 @@ ss.editorWrappers.tinyMCE = (function() {
       if(cb) href = eval(cb + "(href, node, true);");
 
       // Turn into relative, if set in TinyMCE config
-      if(cu && href.match(new RegExp('^' + tinyMCE.settings['document_base_url'] + '(.*)$'))) {
+      if(cu && href.match(new RegExp('^' + escapeRegExp(tinyMCE.settings['document_base_url']) + '(.*)$'))) {
         href = RegExp.$1;
       }
 

client/src/legacy/LeftAndMain.Menu.js

@@ -1,4 +1,5 @@
 import $ from 'jquery';
+import { joinUrlPaths } from 'lib/urls';
 
 $.entwine('ss', function($){
 
@@ -296,7 +297,7 @@ $.entwine('ss', function($){
       var item = this.getMenuItem();
 
       var url = this.attr('href');
-      if(!isExternal) url = $('base').attr('href') + url;
+      if(!isExternal) url = joinUrlPaths($('base').attr('href'), url);
 
       var children = item.find('li');
       if(children.length) {

client/src/legacy/LeftAndMain.js

@@ -10,6 +10,7 @@ import Search from 'components/Search/Search';
 import Loading from 'components/Loading/Loading';
 import { schemaMerge } from 'lib/schemaFieldValues';
 import { loadComponent } from 'lib/Injector';
+import escapeRegExp from 'lodash.escaperegexp';
 
 import '../legacy/ssui.core.js';
 
@@ -55,7 +56,7 @@ window.ss.tabStateUrl = function() {
   return window.location.href
     .replace(/\?.*/, '')
     .replace(/#.*/, '')
-    .replace($('base').attr('href'), '');
+    .replace(new RegExp(`^${escapeRegExp($('base').attr('href'))}/?`), '');
 },
 
 $(window).on('resize.leftandmain', function(e) {

client/src/lib/Router.js

@@ -5,6 +5,7 @@
  */
 import page from 'page.js';
 import url from 'url';
+import escapeRegExp from 'lodash.escaperegexp';
 
 /**
  * Add leading slash to base-relative urls, as required by Page.js
@@ -27,8 +28,9 @@ function resolveURLToBase(path) {
     return absolutePath;
   }
 
-  // Remove base url from absolute path, save for trailing `/` which Page.js requires
-  return absolutePath.substring(absoluteBase.length - 1);
+  // Remove base url from absolute path, save for leading `/` which Page.js requires
+  const regex = new RegExp(`^${escapeRegExp(absoluteBase)}/?`);
+  return absolutePath.replace(regex, '/');
 }
 
 /**

client/src/lib/tests/urls-test.js

@@ -0,0 +1,57 @@
+/* global jest, describe, beforeEach, it, expect, console */
+import { joinUrlPaths } from 'lib/urls';
+
+describe('urls', () => {
+  describe('joinUrlPaths()', () => {
+    it('should add a slash between paths', () => {
+      const value = joinUrlPaths('some', 'path');
+      expect(value).toBe('some/path');
+    });
+
+    it('should only include a single slash between paths', () => {
+      const value1 = joinUrlPaths('some/', 'path');
+      const value2 = joinUrlPaths('some', '/path');
+      const value3 = joinUrlPaths('some/', '/path');
+      expect(value1).toBe('some/path');
+      expect(value2).toBe('some/path');
+      expect(value3).toBe('some/path');
+    });
+
+    it('should retain leading and trailing slashes', () => {
+      const value = joinUrlPaths('/some', 'path/');
+      expect(value).toBe('/some/path/');
+    });
+
+    it('should retain slashes in the middle of a given path', () => {
+      const value = joinUrlPaths('some/path', 'another-path');
+      expect(value).toBe('some/path/another-path');
+    });
+
+    it('should join an arbitrarily large number of paths', () => {
+      const value = joinUrlPaths('some', 'path', 'another', 'path');
+      expect(value).toBe('some/path/another/path');
+    });
+
+    it('should not alter the result if only one path is passed in', () => {
+      const value1 = joinUrlPaths('some-path');
+      const value2 = joinUrlPaths('/some-path/');
+      expect(value1).toBe('some-path');
+      expect(value2).toBe('/some-path/');
+    });
+
+    it('should allow easily ensuring a preceding or leading slash', () => {
+      const value = joinUrlPaths('/', 'some-path', '/');
+      expect(value).toBe('/some-path/');
+    });
+
+    it('should retain URL scheme, host, query string, and fragment identifier', () => {
+      const value = joinUrlPaths('https://www.example.com/', 'some-path?arg=val#some-id');
+      expect(value).toBe('https://www.example.com/some-path?arg=val#some-id');
+    });
+
+    it('should return an empty string if no path is passed in', () => {
+      const value = joinUrlPaths();
+      expect(value).toBe('');
+    });
+  });
+});

client/src/lib/urls.js

@@ -0,0 +1,14 @@
+export const joinUrlPaths = (...urlPaths) => {
+  // Just return a blank string if there's no paths passed in
+  if (!urlPaths.length) {
+    return '';
+  }
+
+  // Combine paths with a single '/' between them.
+  let result = urlPaths.shift();
+  // eslint-disable-next-line no-restricted-syntax
+  for (const path of urlPaths) {
+    result = `${result.replace(/\/$/, '')}/${path.replace(/^\//, '')}`;
+  }
+  return result;
+};

code/AdminRootController.php

@@ -39,9 +39,9 @@ public static function get_admin_route()
      *
      * @return string
      */
-    public static function admin_url()
+    public static function admin_url(string $action = '')
     {
-        return self::get_admin_route() . '/';
+        return Controller::join_links(self::get_admin_route(), $action);
     }
 
     /**

code/CMSMenu.php

@@ -98,7 +98,7 @@ protected static function menuitem_for_controller($controllerClass)
             return null;
         }
 
-        $link = Controller::join_links($urlBase, $urlSegment) . '/';
+        $link = Controller::join_links($urlBase, $urlSegment);
 
         // doesn't work if called outside of a controller context (e.g. in _config.php)
         // as the locale won't be detected properly. Use {@link LeftAndMain->MainMenu()} to update

code/LeftAndMain.php

@@ -897,7 +897,6 @@ public function Link($action = null)
         $link = Controller::join_links(
             AdminRootController::admin_url(),
             $segment,
-            '/', // trailing slash needed if $action is null!
             "$action"
         );
         $this->extend('updateLink', $link);
@@ -1664,7 +1663,7 @@ public function currentPageID()
         if (is_numeric($this->getRequest()->param('ID'))) {
             return $this->getRequest()->param('ID');
         }
-        
+
         /** @deprecated */
         $session = $this->getRequest()->getSession();
         return $session->get($this->sessionNamespace() . ".currentPage") ?: null;

package.json

@@ -58,6 +58,7 @@
     "jquery-form": "^4.3.0",
     "jquery-sizes": "^0.33.0",
     "lodash.debounce": "^4.0.8",
+    "lodash.escaperegexp": "^4.1.2",
     "merge": "^2.1.1",
     "modernizr": "^3.12.0",
     "moment": "^2.29.4",

templates/SilverStripe/Admin/Includes/LeftAndMain_MenuStatus.ss

@@ -1,6 +1,6 @@
 <div class="cms-login-status">
 	<% with $CurrentMember %>
-        <a href="{$AdminURL}myprofile" class="cms-login-status__profile-link" title="<%t SilverStripe\Admin\LeftAndMain.PROFILE '{name} profile' name=$Name %>">
+        <a href="$AdminURL('myprofile')" class="cms-login-status__profile-link" title="<%t SilverStripe\Admin\LeftAndMain.PROFILE '{name} profile' name=$Name %>">
             <i class="font-icon-torso"></i>
             <span>
                 <% if $FirstName && $Surname %>$FirstName $Surname<% else_if $FirstName %>$FirstName<% else %>$Email<% end_if %>

tests/php/LeftAndMainTest.php

@@ -6,6 +6,7 @@
 use SilverStripe\Admin\CMSMenu;
 use SilverStripe\Admin\LeftAndMain;
 use SilverStripe\Assets\File;
+use SilverStripe\Control\Controller;
 use SilverStripe\Control\HTTPResponse;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Manifest\ModuleLoader;
@@ -107,7 +108,7 @@ public function testLeftAndMainSubclasses()
         $this->assertGreaterThan(0, count($menuItems ?? []));
 
         $adminUrl = AdminRootController::admin_url();
-        $menuItem = $menuItems->find('Link', $adminUrl . 'security/');
+        $menuItem = $menuItems->find('Link', Controller::join_links($adminUrl, 'security/'));
         $this->assertNotEmpty($menuItem, 'Security not found in the menu items list');
 
         $link = $menuItem->Link;

yarn.lock

@@ -6089,6 +6089,11 @@ lodash.escape@^4.0.1:
   resolved "https://registry.yarnpkg.com/lodash.escape/-/lodash.escape-4.0.1.tgz#c9044690c21e04294beaa517712fded1fa88de98"
   integrity sha512-nXEOnb/jK9g0DYMr1/Xvq6l5xMD7GDG55+GSYIYmS0G4tBk/hURD4JR9WCavs04t33WmJx9kCyp9vJ+mr4BOUw==
 
+lodash.escaperegexp@^4.1.2:
+  version "4.1.2"
+  resolved "https://registry.yarnpkg.com/lodash.escaperegexp/-/lodash.escaperegexp-4.1.2.tgz#64762c48618082518ac3df4ccf5d5886dae20347"
+  integrity sha512-TM9YBvyC84ZxE3rgfefxUWiQKLilstD6k7PTGt6wfbtXF8ixIJLOL3VYyV/z+ZiPLsVxAsKAFVwWlWeb2Y8Yyw==
+
 lodash.flatten@^4.2.0:
   version "4.4.0"
   resolved "https://registry.yarnpkg.com/lodash.flatten/-/lodash.flatten-4.4.0.tgz#f31c22225a9632d2bbf8e4addbef240aa765a61f"