Skip to content

[next] fix(autolink): correctly handle relative links without webroot #5503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
susnux merged 1 commit into from
Apr 19, 2024
Merged

[next] fix(autolink): correctly handle relative links without webroot #5503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 16 additions & 20 deletions src/components/NcRichText/autolink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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')
Copy link
Contributor

@Antreesy Antreesy Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Known change for Vue 3


// 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
Expand Down
39 changes: 29 additions & 10 deletions tests/unit/components/NcRichText/autoLink.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -147,7 +166,7 @@ describe('autoLink', () => {
],
})

expect(getRoute(routerTalkDesktop, link)).toBe(null)
expect(getRoute(routerTalkDesktop, link)).toBe(expectedRoute)
})
})
})
Expand Down