From 531f545c15d2b557fa2229ca208249b9b6ffa039 Mon Sep 17 00:00:00 2001 From: Pierre Gayvallet Date: Tue, 25 Jan 2022 11:59:47 +0100 Subject: [PATCH] fix parseAppUrl logic to handle app ids including other app ids (#123594) --- .../application/utils/parse_app_url.test.ts | 105 +++++++++++++++++- .../public/application/utils/parse_app_url.ts | 15 ++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/core/public/application/utils/parse_app_url.test.ts b/src/core/public/application/utils/parse_app_url.test.ts index 71137900d6748..e43fd1045e435 100644 --- a/src/core/public/application/utils/parse_app_url.test.ts +++ b/src/core/public/application/utils/parse_app_url.test.ts @@ -39,6 +39,12 @@ describe('parseAppUrl', () => { id: 'bar', appRoute: '/custom-bar', }); + createApp({ + id: 're', + }); + createApp({ + id: 'retail', + }); }); describe('with absolute paths', () => { @@ -51,6 +57,14 @@ describe('parseAppUrl', () => { app: 'bar', path: undefined, }); + expect(parseAppUrl('/base-path/app/re', basePath, apps, currentUrl)).toEqual({ + app: 're', + path: undefined, + }); + expect(parseAppUrl('/base-path/app/retail', basePath, apps, currentUrl)).toEqual({ + app: 'retail', + path: undefined, + }); }); it('parses the path', () => { expect(parseAppUrl('/base-path/app/foo/some/path', basePath, apps, currentUrl)).toEqual({ @@ -63,6 +77,14 @@ describe('parseAppUrl', () => { app: 'bar', path: '/another/path/', }); + expect(parseAppUrl('/base-path/app/re/tail', basePath, apps, currentUrl)).toEqual({ + app: 're', + path: '/tail', + }); + expect(parseAppUrl('/base-path/app/retail/path', basePath, apps, currentUrl)).toEqual({ + app: 'retail', + path: '/path', + }); }); it('includes query and hash in the path for default app route', () => { expect(parseAppUrl('/base-path/app/foo#hash/bang', basePath, apps, currentUrl)).toEqual({ @@ -89,6 +111,18 @@ describe('parseAppUrl', () => { app: 'foo', path: '/path#hash/bang?hello=dolly', }); + expect(parseAppUrl('/base-path/app/re#hash/bang', basePath, apps, currentUrl)).toEqual({ + app: 're', + path: '#hash/bang', + }); + expect(parseAppUrl('/base-path/app/retail?hello=dolly', basePath, apps, currentUrl)).toEqual({ + app: 'retail', + path: '?hello=dolly', + }); + expect(parseAppUrl('/base-path/app/retail#hash/bang', basePath, apps, currentUrl)).toEqual({ + app: 'retail', + path: '#hash/bang', + }); }); it('includes query and hash in the path for custom app route', () => { expect(parseAppUrl('/base-path/custom-bar#hash/bang', basePath, apps, currentUrl)).toEqual({ @@ -190,7 +224,6 @@ describe('parseAppUrl', () => { path: '/path#hash?hello=dolly', }); }); - it('returns undefined if the relative path redirect outside of the basePath', () => { expect( parseAppUrl( @@ -201,6 +234,19 @@ describe('parseAppUrl', () => { ) ).toBeUndefined(); }); + it('works with inclusive app ids', () => { + expect( + parseAppUrl( + './retail/path', + basePath, + apps, + 'https://kibana.local:8080/base-path/app/current' + ) + ).toEqual({ + app: 'retail', + path: '/path', + }); + }); }); describe('with absolute urls', () => { @@ -217,6 +263,18 @@ describe('parseAppUrl', () => { app: 'bar', path: undefined, }); + expect( + parseAppUrl('https://kibana.local:8080/base-path/app/re', basePath, apps, currentUrl) + ).toEqual({ + app: 're', + path: undefined, + }); + expect( + parseAppUrl('https://kibana.local:8080/base-path/app/retail', basePath, apps, currentUrl) + ).toEqual({ + app: 'retail', + path: undefined, + }); }); it('parses the path', () => { expect( @@ -241,6 +299,29 @@ describe('parseAppUrl', () => { app: 'bar', path: '/another/path/', }); + + expect( + parseAppUrl( + 'https://kibana.local:8080/base-path/app/re/some/path', + basePath, + apps, + currentUrl + ) + ).toEqual({ + app: 're', + path: '/some/path', + }); + expect( + parseAppUrl( + 'https://kibana.local:8080/base-path/app/retail/another/path/', + basePath, + apps, + currentUrl + ) + ).toEqual({ + app: 'retail', + path: '/another/path/', + }); }); it('includes query and hash in the path for default app routes', () => { expect( @@ -298,6 +379,28 @@ describe('parseAppUrl', () => { app: 'foo', path: '/path#hash/bang?hello=dolly', }); + expect( + parseAppUrl( + 'https://kibana.local:8080/base-path/app/re#hash/bang', + basePath, + apps, + currentUrl + ) + ).toEqual({ + app: 're', + path: '#hash/bang', + }); + expect( + parseAppUrl( + 'https://kibana.local:8080/base-path/app/re?hello=dolly', + basePath, + apps, + currentUrl + ) + ).toEqual({ + app: 're', + path: '?hello=dolly', + }); }); it('includes query and hash in the path for custom app route', () => { expect( diff --git a/src/core/public/application/utils/parse_app_url.ts b/src/core/public/application/utils/parse_app_url.ts index 099e8a550920c..db4f3dc6d2420 100644 --- a/src/core/public/application/utils/parse_app_url.ts +++ b/src/core/public/application/utils/parse_app_url.ts @@ -61,7 +61,7 @@ export const parseAppUrl = ( for (const app of apps.values()) { const appPath = app.appRoute || `/app/${app.id}`; - if (url.startsWith(appPath)) { + if (urlInApp(url, appPath)) { const path = url.substr(appPath.length); return { app: app.id, @@ -70,3 +70,16 @@ export const parseAppUrl = ( } } }; + +const separators = ['/', '?', '#']; + +const urlInApp = (url: string, appPath: string) => { + if (url === appPath) { + return true; + } + if (url.startsWith(appPath)) { + const nextChar = url.substring(appPath.length, appPath.length + 1); + return separators.includes(nextChar); + } + return false; +};