From 416da59c438b64bcc8f75453c15d34a34e3ccbc9 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Fri, 8 Apr 2022 10:44:17 +0100 Subject: [PATCH] Routing: Fix links to different port from being treated as internal links (#45192) * Add some failing tests that trigger the bug * start at refactor of stripBaseFromUrl --- .../grafana-data/src/utils/location.test.ts | 132 +++++++++++------- packages/grafana-data/src/utils/location.ts | 29 +++- 2 files changed, 108 insertions(+), 53 deletions(-) diff --git a/packages/grafana-data/src/utils/location.test.ts b/packages/grafana-data/src/utils/location.test.ts index 174a505601d..b56660bbd01 100644 --- a/packages/grafana-data/src/utils/location.test.ts +++ b/packages/grafana-data/src/utils/location.test.ts @@ -3,7 +3,7 @@ import { locationUtil } from './location'; describe('locationUtil', () => { const { location } = window; - beforeAll(() => { + beforeEach(() => { // @ts-ignore delete window.location; @@ -21,63 +21,97 @@ describe('locationUtil', () => { }; }); - afterAll(() => { + afterEach(() => { window.location = location; }); - describe('strip base when appSubUrl configured', () => { - beforeEach(() => { - locationUtil.initialize({ - config: { appSubUrl: '/subUrl' } as any, - getVariablesUrlParams: (() => {}) as any, - getTimeRangeForUrl: (() => {}) as any, + describe('stripBaseFromUrl', () => { + describe('when appSubUrl configured', () => { + beforeEach(() => { + locationUtil.initialize({ + config: { appSubUrl: '/subUrl' } as any, + getVariablesUrlParams: (() => {}) as any, + getTimeRangeForUrl: (() => {}) as any, + }); }); - }); - test('relative url', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/'); - expect(urlWithoutMaster).toBe('/thisShouldRemain/'); - }); - test('relative url with multiple subUrl in path', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/subUrl/'); - expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); - }); - test('relative url with subdirectory subUrl', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl('/thisShouldRemain/subUrl/'); - expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); - }); - test('absolute url', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/thisShouldRemain/'); - expect(urlWithoutMaster).toBe('/thisShouldRemain/'); - }); - test('absolute url with multiple subUrl in path', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl( - 'http://www.domain.com:9877/subUrl/thisShouldRemain/subUrl/' - ); - expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); - }); - test('absolute url with subdirectory subUrl', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/thisShouldRemain/subUrl/'); - expect(urlWithoutMaster).toBe('http://www.domain.com:9877/thisShouldRemain/subUrl/'); - }); - }); - - describe('strip base when appSubUrl not configured', () => { - beforeEach(() => { - locationUtil.initialize({ - config: {} as any, - getVariablesUrlParams: (() => {}) as any, - getTimeRangeForUrl: (() => {}) as any, + test('relative url', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/'); + expect(urlWithoutMaster).toBe('/thisShouldRemain/'); + }); + test('relative url with multiple subUrl in path', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/subUrl/'); + expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); + }); + test('relative url with subdirectory subUrl', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('/thisShouldRemain/subUrl/'); + expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); + }); + test('absolute url', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/thisShouldRemain/'); + expect(urlWithoutMaster).toBe('/thisShouldRemain/'); + }); + test('absolute url with multiple subUrl in path', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl( + 'http://www.domain.com:9877/subUrl/thisShouldRemain/subUrl/' + ); + expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); + }); + test('absolute url with subdirectory subUrl', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/thisShouldRemain/subUrl/'); + expect(urlWithoutMaster).toBe('http://www.domain.com:9877/thisShouldRemain/subUrl/'); }); }); - test('relative url', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/grafana/'); - expect(urlWithoutMaster).toBe('/subUrl/grafana/'); + describe('when appSubUrl not configured', () => { + beforeEach(() => { + locationUtil.initialize({ + config: {} as any, + getVariablesUrlParams: (() => {}) as any, + getTimeRangeForUrl: (() => {}) as any, + }); + }); + + test('relative url', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/grafana/'); + expect(urlWithoutMaster).toBe('/subUrl/grafana/'); + }); + + test('absolute url', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/grafana/'); + expect(urlWithoutMaster).toBe('/subUrl/grafana/'); + }); }); - test('absolute url', () => { - const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/grafana/'); - expect(urlWithoutMaster).toBe('/subUrl/grafana/'); + describe('when origin does not have a port in it', () => { + beforeEach(() => { + window.location = { + ...location, + hash: '#hash', + host: 'www.domain.com', + hostname: 'www.domain.com', + href: 'http://www.domain.com/path/b?search=a&b=c&d#hash', + origin: 'http://www.domain.com', + pathname: '/path/b', + port: '', + protocol: 'http:', + search: '?search=a&b=c&d', + }; + }); + + test('relative url', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/grafana/'); + expect(urlWithoutMaster).toBe('/subUrl/grafana/'); + }); + + test('URL with same host, different port', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/grafana/'); + expect(urlWithoutMaster).toBe('http://www.domain.com:9877/subUrl/grafana/'); + }); + + test('URL of a completely different origin', () => { + const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.another-domain.com/subUrl/grafana/'); + expect(urlWithoutMaster).toBe('http://www.another-domain.com/subUrl/grafana/'); + }); }); }); diff --git a/packages/grafana-data/src/utils/location.ts b/packages/grafana-data/src/utils/location.ts index 83f481067bb..7df8796b41b 100644 --- a/packages/grafana-data/src/utils/location.ts +++ b/packages/grafana-data/src/utils/location.ts @@ -7,22 +7,43 @@ let grafanaConfig: GrafanaConfig = { appSubUrl: '' } as any; let getTimeRangeUrlParams: () => RawTimeRange; let getVariablesUrlParams: (scopedVars?: ScopedVars) => UrlQueryMap; +const maybeParseUrl = (input: string): URL | undefined => { + try { + return new URL(input); + } catch { + return undefined; + } +}; + /** * * @param url * @internal */ -const stripBaseFromUrl = (url: string): string => { +const stripBaseFromUrl = (urlOrPath: string): string => { + // Will only return a URL object if the input is actually a valid URL + const parsedUrl = maybeParseUrl(urlOrPath); + if (parsedUrl) { + // If the input is a URL, and for a different origin that we're on, just bail + // and return it. There's no need to strip anything from it + if (parsedUrl.origin !== window.location.origin) { + return urlOrPath; + } + } + const appSubUrl = grafanaConfig.appSubUrl ?? ''; const stripExtraChars = appSubUrl.endsWith('/') ? 1 : 0; - const isAbsoluteUrl = url.startsWith('http'); + const isAbsoluteUrl = urlOrPath.startsWith('http'); + let segmentToStrip = appSubUrl; - if (!url.startsWith('/') || isAbsoluteUrl) { + if (!urlOrPath.startsWith('/') || isAbsoluteUrl) { segmentToStrip = `${window.location.origin}${appSubUrl}`; } - return url.length > 0 && url.indexOf(segmentToStrip) === 0 ? url.slice(segmentToStrip.length - stripExtraChars) : url; + return urlOrPath.length > 0 && urlOrPath.indexOf(segmentToStrip) === 0 + ? urlOrPath.slice(segmentToStrip.length - stripExtraChars) + : urlOrPath; }; /**