From ad49b6cc6976c180566ba2ed1334b1d8d0e1302d Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Thu, 11 Apr 2024 19:12:48 +0200 Subject: [PATCH] fix(autolink): correctly handle relative links without base Signed-off-by: Grigorii K. Shartsev --- src/components/NcRichText/autolink.ts | 36 ++++++++--------- .../components/NcRichText/autoLink.spec.js | 39 ++++++++++++++----- 2 files changed, 45 insertions(+), 30 deletions(-) diff --git a/src/components/NcRichText/autolink.ts b/src/components/NcRichText/autolink.ts index f913acfb6e..7bdc9a994d 100644 --- a/src/components/NcRichText/autolink.ts +++ b/src/components/NcRichText/autolink.ts @@ -122,17 +122,20 @@ export const parseUrl = (text: string) => { * * @param {Router} router - VueRouter instance of the router link * @param {string} url - absolute URL to parse - * @return {string|null} a path that can be useed in the router link or null if this URL doesn't match this router config - * @example http://cloud.ltd/nextcloud/index.php/app/files/favorites?fileid=2#fragment => /files/favorites?fileid=2#fragment + * @return {string|null} a path that can be used in the router link or null if this URL doesn't match this router config + * @example http://cloud.ltd/nextcloud/index.php/apps/files/favorites?fileid=2#fragment => /files/favorites?fileid=2#fragment */ export const getRoute = (router: Router, url: string): string | null => { /** - * http://cloud.ltd /nextcloud /index.php/app/files /favorites?fileid=2#fragment - * |_____origin____|__________router-base__________|_________router-path________| - * |__________base____________| - * |___root___| + * http://cloud.ltd /nextcloud /index.php /apps/files /favorites?fileid=2#fragment + * |_____origin____|___________router-base____________| | + * |__________base____________| |______________relative-url______________| + * | |___root___|_optional_|__app-base__|_________router-path_______| */ + const removePrefix = (str, prefix) => str.startsWith(prefix) ? str.slice(prefix.length) : str + const removePrefixes = (str, ...prefixes) => prefixes.reduce((acc, prefix) => removePrefix(acc, prefix), str) + // Router is not defined in the app => not an app route if (!router) { return null @@ -141,28 +144,21 @@ export const getRoute = (router: Router, url: string): string | null => { const isAbsoluteURL = /^https?:\/\//.test(url) // URL is not a link to this Nextcloud server instance => not an app route - if ((isAbsoluteURL && !url.startsWith(getBaseUrl())) || (!isAbsoluteURL && !url.startsWith(getRootUrl()))) { + if (isAbsoluteURL && !url.startsWith(getBaseUrl())) { return null } - const routerBase = router.options.history.base - - const urlWithoutOrigin = isAbsoluteURL ? url.slice(new URL(url).origin.length) : url - - // Remove index.php - it is optional in general case in both, VueRouter base and the URL - const urlWithoutOriginAndIndexPhp = url.startsWith((isAbsoluteURL ? getBaseUrl() : getRootUrl()) + '/index.php') ? urlWithoutOrigin.replace('/index.php', '') : urlWithoutOrigin - const routerBaseWithoutIndexPhp = routerBase.replace('/index.php', '') + // URL relative to the instance root + const relativeUrl = isAbsoluteURL ? removePrefixes(url, getBaseUrl(), '/index.php') : url - // This URL is not a part of this router by base - if (!urlWithoutOriginAndIndexPhp.startsWith(routerBaseWithoutIndexPhp)) { - return null - } + // Router base relative to the instance root (app-base above) + const relativeRouterBase = removePrefixes(router.options.history.base, getRootUrl(), '/index.php') // Root route may have an empty '' path, fallback to '/' - const routerPath = urlWithoutOriginAndIndexPhp.replace(routerBaseWithoutIndexPhp, '') || '/' + const potentialRouterPath = removePrefixes(relativeUrl, relativeRouterBase) || '/' // Check if there is actually matching route in the router for this path - const route = router.resolve(routerPath) + const route = router.resolve(potentialRouterPath) if (!route.matched.length) { return null diff --git a/tests/unit/components/NcRichText/autoLink.spec.js b/tests/unit/components/NcRichText/autoLink.spec.js index 50a1e67a99..c678a868af 100644 --- a/tests/unit/components/NcRichText/autoLink.spec.js +++ b/tests/unit/components/NcRichText/autoLink.spec.js @@ -41,7 +41,7 @@ describe('autoLink', () => { ['with', '/index.php'], ['without', ''], ])('%s /index.php in link', (_, indexPhp) => { - const linkBase = origin + root + indexPhp + const linkBase = origin ? origin + root + indexPhp : '' beforeAll(() => { getBaseUrl.mockReturnValue(`https://cloud.ltd${root}`) getRootUrl.mockReturnValue(root) @@ -125,17 +125,36 @@ describe('autoLink', () => { }) }) - // getRoute doesn't have to guarantee Talk Desktop compatiblity, but checking just in case + it('should not get route from relative link with base /nextcloud/index.php/apps/test/foo', () => { + getBaseUrl.mockReturnValue('https://cloud.ltd/nextcloud') + getRootUrl.mockReturnValue('/nextcloud') + const router = createRouter({ + mode: 'history', + history: createMemoryHistory('/nextcloud/index.php/apps/test'), + routes: [ + { path: '/foo', name: 'foo' }, + ], + }) + + expect(getRoute(router, '/nextcloud/index.php/apps/test/foo')).toBe(null) + }) + + // getRoute doesn't have to guarantee Talk Desktop compatibility, but checking just in case describe('with Talk Desktop router - no router base and invalid getRootUrl', () => { it.each([ - ['https://cloud.ltd/nextcloud/index.php/apps/spreed?callTo=alice'], - ['https://cloud.ltd/nextcloud/index.php/call/abc123ef'], - ['https://cloud.ltd/nextcloud/index.php/apps/files'], - ['https://cloud.ltd/nextcloud/'], - ])('should not get route for %s', (link) => { - // On Talk Desktop both Base and Root URL returns an absolute path because there is no location + ['https://cloud.ltd/nextcloud/index.php/apps/spreed', '/apps/spreed'], + ['https://cloud.ltd/nextcloud/index.php/apps/spreed?callTo=alice', '/apps/spreed?callTo=alice'], + ['https://cloud.ltd/nextcloud/index.php/call/abc123ef', '/call/abc123ef'], + ['https://cloud.ltd/nextcloud/index.php/apps/files', null], + ['https://cloud.ltd/nextcloud/', null], + ['/apps/spreed', '/apps/spreed'], + ['/apps/spreed?callTo=alice', '/apps/spreed?callTo=alice'], + ['/call/abc123ef', '/call/abc123ef'], + ['/apps/files', null], + ['/', null], + ])('should get route %s => %s', (link, expectedRoute) => { getBaseUrl.mockReturnValue('https://cloud.ltd/nextcloud') - getRootUrl.mockReturnValue('https://cloud.ltd/nextcloud') + getRootUrl.mockReturnValue('/nextcloud') const routerTalkDesktop = createRouter({ // On Talk Desktop we use hash mode, because it works on file:// protocol @@ -147,7 +166,7 @@ describe('autoLink', () => { ], }) - expect(getRoute(routerTalkDesktop, link)).toBe(null) + expect(getRoute(routerTalkDesktop, link)).toBe(expectedRoute) }) }) })