Skip to content

Commit

Permalink
fix parseAppUrl logic to handle app ids including other app ids (#123594
Browse files Browse the repository at this point in the history
)

(cherry picked from commit 531f545)
  • Loading branch information
pgayvallet authored and kibanamachine committed Jan 25, 2022
1 parent 659a73e commit 2949326
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 2 deletions.
105 changes: 104 additions & 1 deletion src/core/public/application/utils/parse_app_url.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ describe('parseAppUrl', () => {
id: 'bar',
appRoute: '/custom-bar',
});
createApp({
id: 're',
});
createApp({
id: 'retail',
});
});

describe('with absolute paths', () => {
Expand All @@ -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({
Expand All @@ -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({
Expand All @@ -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({
Expand Down Expand Up @@ -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(
Expand All @@ -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', () => {
Expand All @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
15 changes: 14 additions & 1 deletion src/core/public/application/utils/parse_app_url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
};

0 comments on commit 2949326

Please sign in to comment.