fix: avoid URL-confusing default pathname in baseUrl (#565)

Fixes #517 When not set explicitly, the router constructor discovers `baseUrl` using the `<base href>` value and initial document URL. However, only the pathname from the discovered base URL was taken for the default value of the `baseUrl` property. In case of initial pathname starting with double slash `//`, such a value was then throwing `Invalid URL` error when used as the URL constructor’s first argument. This changes the default value discovery of `baseUrl`, so that it uses an actual URL instead of only pathname to avoid throwing errors.
vaadin · Feb 15, 2021 · a550589 · a550589
1 parent 8616f93
commit a550589
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 27 deletions.
diff --git a/src/resolver/resolver.js b/src/resolver/resolver.js
@@ -9,7 +9,7 @@
 
 import matchRoute from './matchRoute.js';
 import resolveRoute from './resolveRoute.js';
-import {toArray, ensureRoutes, isString, getNotFoundError, notFoundResult} from '../utils.js';
+import {ensureRoutes, getNotFoundError, isString, notFoundResult, toArray} from '../utils.js';
 
 function isChildRoute(parentRoute, childRoute) {
   let route = childRoute;
@@ -241,7 +241,11 @@ class Resolver {
     }
 
     const base = this.__effectiveBaseUrl;
-    const normalizedUrl = this.constructor.__createUrl(pathname, base).href;
+    // Convert pathname to a valid URL constructor argument
+    const url = pathname[0] === '/'
+      ? this.constructor.__createUrl(base).origin + pathname
+      : './' + pathname;
+    const normalizedUrl = this.constructor.__createUrl(url, base).href;
     if (normalizedUrl.slice(0, base.length) === base) {
       return normalizedUrl.slice(base.length);
     }

diff --git a/src/router.js b/src/router.js
@@ -180,7 +180,7 @@ export class Router extends Resolver {
     const baseHref = baseElement && baseElement.getAttribute('href');
     super([], Object.assign({
       // Default options
-      baseUrl: baseHref && Resolver.__createUrl(baseHref, document.URL).pathname.replace(/[^\/]*$/, '')
+      baseUrl: baseHref && Resolver.__createUrl(baseHref, document.URL).href.replace(/[^\/]*$/, '')
     }, options));
 
     this.resolveRoute = context => this.__resolveRoute(context);
@@ -191,8 +191,8 @@ export class Router extends Resolver {
     /**
      * The base URL for all routes in the router instance. By default,
      * if the base element exists in the `<head>`, vaadin-router
-     * takes the `<base href>` attribute value, resolves against current `document.URL`
-     * and gets the `pathname` from the result.
+     * takes the `<base href>` attribute value, resolved against the current
+     * `document.URL`.
      *
      * @public
      * @type {string}

diff --git a/test/resolver/resolver.spec.js b/test/resolver/resolver.spec.js
@@ -844,12 +844,22 @@
       expect(stub).to.be.called;
     });
 
-    it('should invoke Resolver.__createUrl(path, base) hook', () => {
+    it('should invoke Resolver.__createUrl(url, base) hook', () => {
       sinon.spy(Resolver, '__createUrl');
       try {
+        // Absolute pathname: prepend origin
         new Resolver([], {baseUrl: '/foo/bar'}).__normalizePathname('/baz/');
         expect(Resolver.__createUrl).to.be.calledWith(
-          '/baz/',
+          location.origin + '/baz/',
+          location.origin + '/foo/'
+        );
+
+        Resolver.__createUrl.reset();
+
+        // Relative pathname: prepend dot path prefix
+        new Resolver([], {baseUrl: '/foo/bar'}).__normalizePathname('baz');
+        expect(Resolver.__createUrl).to.be.calledWith(
+          './baz',
           location.origin + '/foo/'
         );
       } finally {

diff --git a/test/router/router.spec.html b/test/router/router.spec.html
@@ -108,53 +108,48 @@
         });
 
         describe('baseUrl', () => {
+          const baseElement = document.createElement('base');
+
+          beforeEach(() => {
+            baseElement.removeAttribute('href');
+            document.head.appendChild(baseElement);
+          });
+
+          afterEach(() => {
+            document.head.removeChild(baseElement);
+          });
+
           it('should accept baseUrl in options object as the 2nd argument', () => {
             router = new Vaadin.Router(null, {baseUrl: '/users/'});
             expect(router).to.have.property('baseUrl', '/users/');
           });
 
           it('should use <base href> as default baseUrl', () => {
-            const baseElement = document.createElement('base');
             baseElement.setAttribute('href', '/foo/');
-            document.head.appendChild(baseElement);
 
             router = new Vaadin.Router(null);
-            expect(router).to.have.property('baseUrl', '/foo/');
-
-            document.head.removeChild(baseElement);
+            expect(router).to.have.property('baseUrl', location.origin + '/foo/');
           });
 
           it('should resolve relative base href when setting baseUrl', async() => {
-            const baseElement = document.createElement('base');
             baseElement.setAttribute('href', './foo/../bar/asdf');
-            document.head.appendChild(baseElement);
 
             router = new Vaadin.Router(null);
-            expect(router).to.have.property('baseUrl', '/bar/');
-
-            document.head.removeChild(baseElement);
+            expect(router).to.have.property('baseUrl', location.origin + '/bar/');
           });
 
           it('should use absolute base href when setting baseUrl', async() => {
-            const baseElement = document.createElement('base');
             baseElement.setAttribute('href', '/my/base/');
-            document.head.appendChild(baseElement);
 
             router = new Vaadin.Router(null);
-            expect(router).to.have.property('baseUrl', '/my/base/');
-
-            document.head.removeChild(baseElement);
+            expect(router).to.have.property('baseUrl', location.origin + '/my/base/');
           });
 
           it('should use custom base href when setting baseUrl', async() => {
-            const baseElement = document.createElement('base');
             baseElement.setAttribute('href', 'http://localhost:8080/my/custom/base/');
-            document.head.appendChild(baseElement);
 
             router = new Vaadin.Router(null);
-            expect(router).to.have.property('baseUrl', '/my/custom/base/');
-
-            document.head.removeChild(baseElement);
+            expect(router).to.have.property('baseUrl', 'http://localhost:8080/my/custom/base/');
           });
 
           it('should use baseUrl when matching relative routes', async() => {
@@ -186,6 +181,18 @@
             await router.render('/foo/home');
             checkOutlet(['x-home-view']);
           });
+
+          it('should not throw when base path starts with double slash', async() => {
+            baseElement.setAttribute('href', location.origin + '//foo');
+
+            router = new Vaadin.Router(outlet);
+            expect(router).to.have.property('baseUrl', location.origin + '//');
+
+            router.setRoutes([{path: '(.*)', component: 'x-home-view'}]);
+
+            await router.render('//');
+            checkOutlet(['x-home-view']);
+          });
         });
 
         describe('router.render(pathname)', () => {