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
This commit is contained in:
Josh Hunt 2022-04-08 10:44:17 +01:00 committed by GitHub
parent beed3fd5de
commit 416da59c43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 108 additions and 53 deletions

View File

@ -3,7 +3,7 @@ import { locationUtil } from './location';
describe('locationUtil', () => { describe('locationUtil', () => {
const { location } = window; const { location } = window;
beforeAll(() => { beforeEach(() => {
// @ts-ignore // @ts-ignore
delete window.location; delete window.location;
@ -21,63 +21,97 @@ describe('locationUtil', () => {
}; };
}); });
afterAll(() => { afterEach(() => {
window.location = location; window.location = location;
}); });
describe('strip base when appSubUrl configured', () => { describe('stripBaseFromUrl', () => {
beforeEach(() => { describe('when appSubUrl configured', () => {
locationUtil.initialize({ beforeEach(() => {
config: { appSubUrl: '/subUrl' } as any, locationUtil.initialize({
getVariablesUrlParams: (() => {}) as any, config: { appSubUrl: '/subUrl' } as any,
getTimeRangeForUrl: (() => {}) as any, getVariablesUrlParams: (() => {}) as any,
getTimeRangeForUrl: (() => {}) as any,
});
}); });
}); test('relative url', () => {
test('relative url', () => { const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/');
const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/'); expect(urlWithoutMaster).toBe('/thisShouldRemain/');
expect(urlWithoutMaster).toBe('/thisShouldRemain/'); });
}); test('relative url with multiple subUrl in path', () => {
test('relative url with multiple subUrl in path', () => { const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/subUrl/');
const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/thisShouldRemain/subUrl/'); expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/');
expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); });
}); test('relative url with subdirectory subUrl', () => {
test('relative url with subdirectory subUrl', () => { const urlWithoutMaster = locationUtil.stripBaseFromUrl('/thisShouldRemain/subUrl/');
const urlWithoutMaster = locationUtil.stripBaseFromUrl('/thisShouldRemain/subUrl/'); expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/');
expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); });
}); test('absolute url', () => {
test('absolute url', () => { const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/thisShouldRemain/');
const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/thisShouldRemain/'); expect(urlWithoutMaster).toBe('/thisShouldRemain/');
expect(urlWithoutMaster).toBe('/thisShouldRemain/'); });
}); test('absolute url with multiple subUrl in path', () => {
test('absolute url with multiple subUrl in path', () => { const urlWithoutMaster = locationUtil.stripBaseFromUrl(
const urlWithoutMaster = locationUtil.stripBaseFromUrl( 'http://www.domain.com:9877/subUrl/thisShouldRemain/subUrl/'
'http://www.domain.com:9877/subUrl/thisShouldRemain/subUrl/' );
); expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/');
expect(urlWithoutMaster).toBe('/thisShouldRemain/subUrl/'); });
}); test('absolute url with subdirectory subUrl', () => {
test('absolute url with subdirectory subUrl', () => { const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/thisShouldRemain/subUrl/');
const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/thisShouldRemain/subUrl/'); expect(urlWithoutMaster).toBe('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', () => { describe('when appSubUrl not configured', () => {
const urlWithoutMaster = locationUtil.stripBaseFromUrl('/subUrl/grafana/'); beforeEach(() => {
expect(urlWithoutMaster).toBe('/subUrl/grafana/'); 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', () => { describe('when origin does not have a port in it', () => {
const urlWithoutMaster = locationUtil.stripBaseFromUrl('http://www.domain.com:9877/subUrl/grafana/'); beforeEach(() => {
expect(urlWithoutMaster).toBe('/subUrl/grafana/'); 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/');
});
}); });
}); });

View File

@ -7,22 +7,43 @@ let grafanaConfig: GrafanaConfig = { appSubUrl: '' } as any;
let getTimeRangeUrlParams: () => RawTimeRange; let getTimeRangeUrlParams: () => RawTimeRange;
let getVariablesUrlParams: (scopedVars?: ScopedVars) => UrlQueryMap; let getVariablesUrlParams: (scopedVars?: ScopedVars) => UrlQueryMap;
const maybeParseUrl = (input: string): URL | undefined => {
try {
return new URL(input);
} catch {
return undefined;
}
};
/** /**
* *
* @param url * @param url
* @internal * @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 appSubUrl = grafanaConfig.appSubUrl ?? '';
const stripExtraChars = appSubUrl.endsWith('/') ? 1 : 0; const stripExtraChars = appSubUrl.endsWith('/') ? 1 : 0;
const isAbsoluteUrl = url.startsWith('http'); const isAbsoluteUrl = urlOrPath.startsWith('http');
let segmentToStrip = appSubUrl; let segmentToStrip = appSubUrl;
if (!url.startsWith('/') || isAbsoluteUrl) { if (!urlOrPath.startsWith('/') || isAbsoluteUrl) {
segmentToStrip = `${window.location.origin}${appSubUrl}`; 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;
}; };
/** /**