Skip to content

Commit

Permalink
fix(richText): don't resolve relative links without leading slash
Browse files Browse the repository at this point in the history
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>

[skip ci]
  • Loading branch information
ShGKme authored and backportbot[bot] committed Jun 13, 2024
1 parent 27dbb98 commit 9cac06a
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
15 changes: 15 additions & 0 deletions src/components/NcRichText/autolink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
47 changes: 46 additions & 1 deletion tests/unit/components/NcRichText/autoLink.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { getBaseUrl, getRootUrl } from '@nextcloud/router'

vi.mock('@nextcloud/router')

Vue.use(VueRouter)

describe('autoLink', () => {
describe('getRoute', () => {
describe.each([
Expand Down Expand Up @@ -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({
Expand All @@ -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([
Expand Down

0 comments on commit 9cac06a

Please sign in to comment.