From a2bca3225a8eda07afa970bbc43a181790689511 Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:04:29 +0200 Subject: [PATCH 01/15] wip --- packages/nuxt/src/app/composables/router.ts | 25 ++++++++++++++++--- test/basic.test.ts | 8 ++++++ .../basic/pages/navigate-to-external.vue | 7 ++++++ 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/basic/pages/navigate-to-external.vue diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 1ec7e83a62b..4f1d92862c6 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -60,19 +60,38 @@ export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {} if (!to) { to = '/' } - // Early redirect on client-side since only possible option is redirectCode and not applied + + /* TODO: External link logic here */ + const path = typeof to === 'string' + ? to + : 'path' in to + ? to.path + : '' + + const isExternalLink = path.startsWith('http') + if (process.client && isProcessingMiddleware()) { - return to + // TODO: What should we do here when link is external? + return isExternalLink ? {} : to } const router = useRouter() if (process.server) { const nuxtApp = useNuxtApp() if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) { - const redirectLocation = joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') + const redirectLocation = isExternalLink ? path : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') return nuxtApp.callHook('app:redirected').then(() => sendRedirect(nuxtApp.ssrContext.event, redirectLocation, options.redirectCode || 302)) } } // Client-side redirection using vue-router + if (isExternalLink) { + if (options.replace) { + location.replace(path) + return + } + location.href = path + return + } + return options.replace ? router.replace(to) : router.push(to) } diff --git a/test/basic.test.ts b/test/basic.test.ts index b4fcb492bf6..34278092266 100644 --- a/test/basic.test.ts +++ b/test/basic.test.ts @@ -155,6 +155,14 @@ describe('navigate', () => { }) }) +describe('navigate external', () => { + it('should redirect to example.com', async () => { + const { headers } = await fetch('/navigate-to-external/', { redirect: 'manual' }) + + expect(headers.get('location')).toEqual('https://example.com/') + }) +}) + describe('errors', () => { it('should render a JSON error page', async () => { const res = await fetch('/error', { diff --git a/test/fixtures/basic/pages/navigate-to-external.vue b/test/fixtures/basic/pages/navigate-to-external.vue new file mode 100644 index 00000000000..544b78278ba --- /dev/null +++ b/test/fixtures/basic/pages/navigate-to-external.vue @@ -0,0 +1,7 @@ + + + From 9bb7c33a8f1e22fd41252b066c9bc4c13ba154a9 Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:04:34 +0200 Subject: [PATCH 02/15] wip --- packages/nuxt/src/app/composables/router.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 4f1d92862c6..c000ddfbeed 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -56,18 +56,22 @@ export interface NavigateToOptions { redirectCode?: number } +const getPath = (to: RouteLocationRaw): string => { + if (typeof to === 'string') { + return to + } + if ('path' in to) { + return to.path + } + return '' +} + export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {}): Promise | RouteLocationRaw => { if (!to) { to = '/' } - /* TODO: External link logic here */ - const path = typeof to === 'string' - ? to - : 'path' in to - ? to.path - : '' - + const path = getPath(to) const isExternalLink = path.startsWith('http') if (process.client && isProcessingMiddleware()) { From 37d150da5b2d65b123231d7ee7d28abb2839723a Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:05:47 +0200 Subject: [PATCH 03/15] feat: improve external url detection Co-authored-by: Koen van Staveren --- packages/nuxt/src/app/composables/router.ts | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index c000ddfbeed..798d77d023f 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -66,28 +66,38 @@ const getPath = (to: RouteLocationRaw): string => { return '' } +const isExternalLink = (str: string): boolean => { + try { + // eslint-disable-next-line no-new + new URL(str) + return true + } catch { + return false + } +} + export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {}): Promise | RouteLocationRaw => { if (!to) { to = '/' } const path = getPath(to) - const isExternalLink = path.startsWith('http') + const isExternal = isExternalLink(path) if (process.client && isProcessingMiddleware()) { // TODO: What should we do here when link is external? - return isExternalLink ? {} : to + return isExternal ? {} : to } const router = useRouter() if (process.server) { const nuxtApp = useNuxtApp() if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) { - const redirectLocation = isExternalLink ? path : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') - return nuxtApp.callHook('app:redirected').then(() => sendRedirect(nuxtApp.ssrContext.event, redirectLocation, options.redirectCode || 302)) + const redirectLocation = isExternal ? path : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') + return nuxtApp.callHook('app:redirected').then(() => sendRedirect(nuxtApp.ssrContext.event, redirectLocation, options.redirectCode || 301)) } } // Client-side redirection using vue-router - if (isExternalLink) { + if (isExternal) { if (options.replace) { location.replace(path) return From fb67669582d504174d20b1f60859a59a7cece19c Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:06:10 +0200 Subject: [PATCH 04/15] fix: handle external link when processing middleware Co-authored-by: Daniel Roe --- packages/nuxt/src/app/composables/router.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 798d77d023f..4badc05a5a4 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -84,10 +84,10 @@ export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {} const path = getPath(to) const isExternal = isExternalLink(path) - if (process.client && isProcessingMiddleware()) { - // TODO: What should we do here when link is external? - return isExternal ? {} : to + if (!isExternal && isProcessingMiddleware()) { + return to } + const router = useRouter() if (process.server) { const nuxtApp = useNuxtApp() From c69ddd09d1a07699c67de63676bb31fe8588391f Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:07:11 +0200 Subject: [PATCH 05/15] fix: return fallback '/' for getPath --- packages/nuxt/src/app/composables/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 4badc05a5a4..85abb0f720c 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -63,7 +63,7 @@ const getPath = (to: RouteLocationRaw): string => { if ('path' in to) { return to.path } - return '' + return '/' } const isExternalLink = (str: string): boolean => { From ca0db61c0f5f7971e5afa6151bb314c03b12497b Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:07:11 +0200 Subject: [PATCH 06/15] refactor: code style --- packages/nuxt/src/app/composables/router.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 85abb0f720c..8280d2337c6 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -81,8 +81,8 @@ export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {} to = '/' } - const path = getPath(to) - const isExternal = isExternalLink(path) + const toPath = getPath(to) + const isExternal = isExternalLink(toPath) if (!isExternal && isProcessingMiddleware()) { return to @@ -92,17 +92,17 @@ export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {} if (process.server) { const nuxtApp = useNuxtApp() if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) { - const redirectLocation = isExternal ? path : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') + const redirectLocation = isExternal ? toPath : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') return nuxtApp.callHook('app:redirected').then(() => sendRedirect(nuxtApp.ssrContext.event, redirectLocation, options.redirectCode || 301)) } } // Client-side redirection using vue-router if (isExternal) { if (options.replace) { - location.replace(path) + location.replace(toPath) return } - location.href = path + location.href = toPath return } From 4e3207fad57cd9392f0b7a400811f166dc8affc2 Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:07:11 +0200 Subject: [PATCH 07/15] refactor: use ufo for protocol check --- packages/nuxt/src/app/composables/router.ts | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 8280d2337c6..76bf7b2eb7f 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -1,6 +1,6 @@ import type { Router, RouteLocationNormalizedLoaded, NavigationGuard, RouteLocationNormalized, RouteLocationRaw, NavigationFailure } from 'vue-router' import { sendRedirect } from 'h3' -import { joinURL } from 'ufo' +import { hasProtocol, joinURL } from 'ufo' import { useNuxtApp, useRuntimeConfig } from '#app' export const useRouter = () => { @@ -66,23 +66,13 @@ const getPath = (to: RouteLocationRaw): string => { return '/' } -const isExternalLink = (str: string): boolean => { - try { - // eslint-disable-next-line no-new - new URL(str) - return true - } catch { - return false - } -} - export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {}): Promise | RouteLocationRaw => { if (!to) { to = '/' } const toPath = getPath(to) - const isExternal = isExternalLink(toPath) + const isExternal = hasProtocol(toPath, true) if (!isExternal && isProcessingMiddleware()) { return to From 59a35498cbff38be8c4f5bde2876715a6e25a15d Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:49:11 +0200 Subject: [PATCH 08/15] refactor: use `allowExternal` and check for scripts --- packages/nuxt/src/app/composables/router.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 76bf7b2eb7f..e1bd451bc48 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -1,6 +1,6 @@ import type { Router, RouteLocationNormalizedLoaded, NavigationGuard, RouteLocationNormalized, RouteLocationRaw, NavigationFailure } from 'vue-router' import { sendRedirect } from 'h3' -import { hasProtocol, joinURL } from 'ufo' +import { hasProtocol, joinURL, parseURL } from 'ufo' import { useNuxtApp, useRuntimeConfig } from '#app' export const useRouter = () => { @@ -53,7 +53,8 @@ const isProcessingMiddleware = () => { export interface NavigateToOptions { replace?: boolean - redirectCode?: number + redirectCode?: number, + allowExternal?: boolean } const getPath = (to: RouteLocationRaw): string => { @@ -73,6 +74,11 @@ export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {} const toPath = getPath(to) const isExternal = hasProtocol(toPath, true) + const hasScriptProtocol = parseURL(toPath).protocol === 'script:' + + if (hasScriptProtocol) { + throw new Error('Cannot navigate to an URL with script protocol') + } if (!isExternal && isProcessingMiddleware()) { return to @@ -82,12 +88,22 @@ export const navigateTo = (to: RouteLocationRaw, options: NavigateToOptions = {} if (process.server) { const nuxtApp = useNuxtApp() if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) { + if (isExternal && !options.allowExternal) { + throw new Error('Redirecting to external URL without `allowExternal: true` is not allowed.') + } const redirectLocation = isExternal ? toPath : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') return nuxtApp.callHook('app:redirected').then(() => sendRedirect(nuxtApp.ssrContext.event, redirectLocation, options.redirectCode || 301)) } } + // Client-side redirection using vue-router if (isExternal) { + if (!options.allowExternal) { + const response = window.confirm(`Do you want to get redirected to ${toPath}?`) + if (!response) { + return + } + } if (options.replace) { location.replace(toPath) return From 633e6e31bdcf0a45fd222c8224b8eab04d03f18d Mon Sep 17 00:00:00 2001 From: Alexander Lichter Date: Fri, 17 Jun 2022 10:53:27 +0200 Subject: [PATCH 09/15] fix: test --- test/fixtures/basic/pages/navigate-to-external.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/basic/pages/navigate-to-external.vue b/test/fixtures/basic/pages/navigate-to-external.vue index 544b78278ba..d37a6c73a57 100644 --- a/test/fixtures/basic/pages/navigate-to-external.vue +++ b/test/fixtures/basic/pages/navigate-to-external.vue @@ -3,5 +3,5 @@ From dea1f129afe67454a8a0d508d2acce4cda4165c4 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Wed, 24 Aug 2022 17:11:59 +0200 Subject: [PATCH 10/15] fix: always return promise. --- packages/nuxt/src/app/composables/router.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 41e898c7d45..3d2867279b1 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -106,15 +106,15 @@ export const navigateTo = (to: RouteLocationRaw | undefined | null, options: Nav if (!options.allowExternal) { const response = window.confirm(`Do you want to get redirected to ${toPath}?`) if (!response) { - return + return Promise.resolve() } } if (options.replace) { location.replace(toPath) - return + return Promise.resolve() } location.href = toPath - return + return Promise.resolve() } return options.replace ? router.replace(to) : router.push(to) From 89c6772c4203b89764f4381efc9e3c75bc39b2fc Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Wed, 24 Aug 2022 17:20:26 +0200 Subject: [PATCH 11/15] refactor: update error message and always throw error --- packages/nuxt/src/app/composables/router.ts | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 3d2867279b1..84bafa576b1 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -59,7 +59,7 @@ const isProcessingMiddleware = () => { export interface NavigateToOptions { replace?: boolean redirectCode?: number, - allowExternal?: boolean + external?: boolean } const getPath = (to: RouteLocationRaw): string => { @@ -89,13 +89,14 @@ export const navigateTo = (to: RouteLocationRaw | undefined | null, options: Nav return to } + if (isExternal && !options.external) { + throw new Error('Navigating to external URL is not allowed by default. Use `nagivateTo(url, { external: true })`.') + } + const router = useRouter() if (process.server) { const nuxtApp = useNuxtApp() if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) { - if (isExternal && !options.allowExternal) { - throw new Error('Redirecting to external URL without `allowExternal: true` is not allowed.') - } const redirectLocation = isExternal ? toPath : joinURL(useRuntimeConfig().app.baseURL, router.resolve(to).fullPath || '/') return nuxtApp.callHook('app:redirected').then(() => sendRedirect(nuxtApp.ssrContext!.event, redirectLocation, options.redirectCode || 301)) } @@ -103,17 +104,11 @@ export const navigateTo = (to: RouteLocationRaw | undefined | null, options: Nav // Client-side redirection using vue-router if (isExternal) { - if (!options.allowExternal) { - const response = window.confirm(`Do you want to get redirected to ${toPath}?`) - if (!response) { - return Promise.resolve() - } - } if (options.replace) { location.replace(toPath) - return Promise.resolve() + } else { + location.href = toPath } - location.href = toPath return Promise.resolve() } From 68b7f274b368ca37c07dd3ece467fda5cd530b9c Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Wed, 24 Aug 2022 17:36:54 +0200 Subject: [PATCH 12/15] refactor: inline toPath computation and avoid extra url parsing --- packages/nuxt/src/app/composables/router.ts | 27 ++++++--------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 84bafa576b1..5e7b6364dae 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -1,5 +1,5 @@ import { getCurrentInstance, inject } from 'vue' -import type { Router, RouteLocationNormalizedLoaded, NavigationGuard, RouteLocationNormalized, RouteLocationRaw, NavigationFailure } from 'vue-router' +import type { Router, RouteLocationNormalizedLoaded, NavigationGuard, RouteLocationNormalized, RouteLocationRaw, NavigationFailure, RouteLocationPathRaw } from 'vue-router' import { sendRedirect } from 'h3' import { hasProtocol, joinURL, parseURL } from 'ufo' import { useNuxtApp, useRuntimeConfig } from '#app' @@ -62,38 +62,27 @@ export interface NavigateToOptions { external?: boolean } -const getPath = (to: RouteLocationRaw): string => { - if (typeof to === 'string') { - return to - } - if ('path' in to) { - return to.path - } - return '/' -} - export const navigateTo = (to: RouteLocationRaw | undefined | null, options: NavigateToOptions = {}): Promise | RouteLocationRaw => { if (!to) { to = '/' } - const toPath = getPath(to) + const toPath = typeof to === 'string' ? to : ((to as RouteLocationPathRaw).path || '/') const isExternal = hasProtocol(toPath, true) - const hasScriptProtocol = parseURL(toPath).protocol === 'script:' - - if (hasScriptProtocol) { + if (isExternal && !options.external) { + throw new Error('Navigating to external URL is not allowed by default. Use `nagivateTo(url, { external: true })`.') + } + if (isExternal && parseURL(toPath).protocol === 'script:') { throw new Error('Cannot navigate to an URL with script protocol') } + // Early redirect on client-side if (!isExternal && isProcessingMiddleware()) { return to } - if (isExternal && !options.external) { - throw new Error('Navigating to external URL is not allowed by default. Use `nagivateTo(url, { external: true })`.') - } - const router = useRouter() + if (process.server) { const nuxtApp = useNuxtApp() if (nuxtApp.ssrContext && nuxtApp.ssrContext.event) { From aeeecdc1772fa406691258fed27aa9ce878bb1c0 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Wed, 24 Aug 2022 17:39:08 +0200 Subject: [PATCH 13/15] small change --- packages/nuxt/src/app/composables/router.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/nuxt/src/app/composables/router.ts b/packages/nuxt/src/app/composables/router.ts index 5e7b6364dae..188d8c0bca9 100644 --- a/packages/nuxt/src/app/composables/router.ts +++ b/packages/nuxt/src/app/composables/router.ts @@ -70,10 +70,10 @@ export const navigateTo = (to: RouteLocationRaw | undefined | null, options: Nav const toPath = typeof to === 'string' ? to : ((to as RouteLocationPathRaw).path || '/') const isExternal = hasProtocol(toPath, true) if (isExternal && !options.external) { - throw new Error('Navigating to external URL is not allowed by default. Use `nagivateTo(url, { external: true })`.') + throw new Error('Navigating to external URL is not allowed by default. Use `nagivateTo (url, { external: true })`.') } if (isExternal && parseURL(toPath).protocol === 'script:') { - throw new Error('Cannot navigate to an URL with script protocol') + throw new Error('Cannot navigate to an URL with script protocol.') } // Early redirect on client-side From 6ddf63b60a67786ffa40fd414a9f53272f46ec94 Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Wed, 24 Aug 2022 17:42:21 +0200 Subject: [PATCH 14/15] update fixture --- test/fixtures/basic/pages/navigate-to-external.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/basic/pages/navigate-to-external.vue b/test/fixtures/basic/pages/navigate-to-external.vue index d37a6c73a57..c7aac9d2c59 100644 --- a/test/fixtures/basic/pages/navigate-to-external.vue +++ b/test/fixtures/basic/pages/navigate-to-external.vue @@ -3,5 +3,5 @@ From d97e68a1df4f23affbdf1e1d9c71766919d5776e Mon Sep 17 00:00:00 2001 From: Pooya Parsa Date: Wed, 24 Aug 2022 17:46:38 +0200 Subject: [PATCH 15/15] t y p o --- test/fixtures/basic/pages/navigate-to-external.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/basic/pages/navigate-to-external.vue b/test/fixtures/basic/pages/navigate-to-external.vue index c7aac9d2c59..f3a8b548a89 100644 --- a/test/fixtures/basic/pages/navigate-to-external.vue +++ b/test/fixtures/basic/pages/navigate-to-external.vue @@ -3,5 +3,5 @@