From 9cac06a00109cb74fdbe74aab0e9bff1d62fecc3 Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Thu, 13 Jun 2024 16:41:38 +0200 Subject: [PATCH] fix(richText): don't resolve relative links without leading slash Signed-off-by: Grigorii K. Shartsev [skip ci] --- src/components/NcRichText/autolink.ts | 15 ++++++ .../components/NcRichText/autoLink.spec.js | 47 ++++++++++++++++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/components/NcRichText/autolink.ts b/src/components/NcRichText/autolink.ts index 5b3911d57a..3cb1ef02af 100644 --- a/src/components/NcRichText/autolink.ts +++ b/src/components/NcRichText/autolink.ts @@ -123,11 +123,26 @@ export const getRoute = (router: Router, url: string): string | null => { const isAbsoluteURL = /^https?:\/\//.test(url) + // URL is a "mailto:", "tel:" or any custom app protocol link + const isNonHttpLink = /^[a-z][a-z0-9+.-]*:.+/.test(url) + if (!isAbsoluteURL && isNonHttpLink) { + return null + } + // URL is not a link to this Nextcloud server instance => not an app route if (isAbsoluteURL && !url.startsWith(getBaseUrl())) { return null } + if (!isAbsoluteURL && !url.startsWith('/')) { + // Relative URL without a leading slash are not allowed + // VueRouter handles such urls according to the URL RFC, + // for example, being on /call/abc page, link "def" is resolved as "/call/def" relative to "/call/". + // We don't want to support such links, + // so relative URL MUST start with a slash. + return null + } + // URL relative to the instance root const relativeUrl = isAbsoluteURL ? removePrefixes(url, getBaseUrl(), '/index.php') : url diff --git a/tests/unit/components/NcRichText/autoLink.spec.js b/tests/unit/components/NcRichText/autoLink.spec.js index ea1d1adcb8..5843f58d0b 100644 --- a/tests/unit/components/NcRichText/autoLink.spec.js +++ b/tests/unit/components/NcRichText/autoLink.spec.js @@ -10,6 +10,8 @@ import { getBaseUrl, getRootUrl } from '@nextcloud/router' vi.mock('@nextcloud/router') +Vue.use(VueRouter) + describe('autoLink', () => { describe('getRoute', () => { describe.each([ @@ -108,7 +110,7 @@ describe('autoLink', () => { }) }) - it('should not get route from relative link with base /nextcloud/index.php/apps/test/foo', () => { + it('should not get route from relative link with base in the link /nextcloud/index.php/apps/test/foo', () => { getBaseUrl.mockReturnValue('https://cloud.ltd/nextcloud') getRootUrl.mockReturnValue('/nextcloud') const router = createRouter({ @@ -122,6 +124,49 @@ describe('autoLink', () => { expect(getRoute(router, '/nextcloud/index.php/apps/test/foo')).toBe(null) }) + it('should not get route from relative link without a leading slash', async () => { + getBaseUrl.mockReturnValue('https://cloud.ltd/') + getRootUrl.mockReturnValue('') + const routerTalk = new VueRouter({ + mode: 'abstract', + base: '', + routes: [ + { path: '/apps/spreed', name: 'root', component: {} }, + { path: '/call/:id', name: 'call', component: {} }, + ], + }) + routerTalk.push('/call/12345678') + + expect(getRoute(routerTalk, 'abcdefgh')).toBe(null) + expect(getRoute(routerTalk, 'call/abcdefgh')).toBe(null) + }) + + describe('Non-HTTP links', () => { + const routerTalk = new VueRouter({ + mode: 'abstract', + base: '', + routes: [ + { path: '/apps/spreed', name: 'root' }, + { path: '/call/:id', name: 'call' }, + ], + }) + getBaseUrl.mockReturnValue('https://cloud.ltd/') + getRootUrl.mockReturnValue('') + routerTalk.push('/call/12345678') + + it('should not handle mailto: link as a relative link', () => { + expect(getRoute(routerTalk, 'mailto:email@nextcloud.ltd')).toBe(null) + }) + + it('should not handle nc: link as a relative link', () => { + expect(getRoute(routerTalk, 'nc://login')).toBe(null) + }) + + it('should not handle a1.b-c+d: link as a relative link', () => { + expect(getRoute(routerTalk, 'a1.b-c+d://action')).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([