From a5505895d72cf36a09b428ec47dca07e650881d6 Mon Sep 17 00:00:00 2001 From: Anton Platonov Date: Mon, 15 Feb 2021 17:02:55 +0200 Subject: [PATCH] fix: avoid URL-confusing default pathname in baseUrl (#565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #517 When not set explicitly, the router constructor discovers `baseUrl` using the `` 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. --- src/resolver/resolver.js | 8 ++++-- src/router.js | 6 ++--- test/resolver/resolver.spec.js | 14 ++++++++-- test/router/router.spec.html | 47 +++++++++++++++++++--------------- 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/resolver/resolver.js b/src/resolver/resolver.js index 15fbc1ee..a2860551 100644 --- 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 index 9455aab4..66903c44 100644 --- 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 ``, vaadin-router - * takes the `` attribute value, resolves against current `document.URL` - * and gets the `pathname` from the result. + * takes the `` 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 index 71f99cfe..bd47ccb2 100644 --- 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 index 6b2ff4f7..a739e988 100644 --- 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 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)', () => {