From 71a0f5b1b78f249cfb12bb54638c490abbf32f1a Mon Sep 17 00:00:00 2001 From: Maurice Bernard Date: Fri, 13 Oct 2023 08:39:00 +0200 Subject: [PATCH 1/2] fix: unhandled special characters in URI results in redirect to same page --- CONTRIBUTING.md | 2 +- specs/fixtures/issues/2382/app.vue | 3 ++ specs/fixtures/issues/2382/locales/en-US.json | 15 ++++++ specs/fixtures/issues/2382/nuxt.config.ts | 19 +++++++ specs/fixtures/issues/2382/package.json | 14 ++++++ specs/fixtures/issues/2382/pages/index.vue | 5 ++ .../issues/2382/pages/level-1/[id]/index.vue | 4 ++ .../issues/2382/pages/level-1/index.vue | 3 ++ specs/issues/2382.spec.ts | 49 +++++++++++++++++++ src/runtime/utils.ts | 12 ++++- 10 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 specs/fixtures/issues/2382/app.vue create mode 100755 specs/fixtures/issues/2382/locales/en-US.json create mode 100644 specs/fixtures/issues/2382/nuxt.config.ts create mode 100644 specs/fixtures/issues/2382/package.json create mode 100644 specs/fixtures/issues/2382/pages/index.vue create mode 100644 specs/fixtures/issues/2382/pages/level-1/[id]/index.vue create mode 100644 specs/fixtures/issues/2382/pages/level-1/index.vue create mode 100644 specs/issues/2382.spec.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cd95fa83e..64cfe72a8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -30,7 +30,7 @@ git checkout -b my-new-feature ```sh pnpm fix pnpm test:unit -pnpm test:e2e +pnpm test:spec ``` - Commit and push your changes diff --git a/specs/fixtures/issues/2382/app.vue b/specs/fixtures/issues/2382/app.vue new file mode 100644 index 000000000..8f62b8bf9 --- /dev/null +++ b/specs/fixtures/issues/2382/app.vue @@ -0,0 +1,3 @@ + diff --git a/specs/fixtures/issues/2382/locales/en-US.json b/specs/fixtures/issues/2382/locales/en-US.json new file mode 100755 index 000000000..cb76ec7c4 --- /dev/null +++ b/specs/fixtures/issues/2382/locales/en-US.json @@ -0,0 +1,15 @@ +{ + "Meta": { + "locale": "English" + }, + "Pages": { + "Home": { + "title": "Home page", + "description": "Home page description" + }, + "About": { + "title": "About page", + "description": "About page description" + } + } +} diff --git a/specs/fixtures/issues/2382/nuxt.config.ts b/specs/fixtures/issues/2382/nuxt.config.ts new file mode 100644 index 000000000..ebdb6a1d4 --- /dev/null +++ b/specs/fixtures/issues/2382/nuxt.config.ts @@ -0,0 +1,19 @@ +import type { LocaleObject } from '#i18n' + +const locales = [{ code: 'en', iso: 'en-US', file: 'en-US.json' }] as LocaleObject[] + +const defaultLocale = locales[0] + +export default defineNuxtConfig({ + modules: ['@nuxtjs/i18n'], + i18n: { + locales, + defaultLocale: defaultLocale.code, + langDir: 'locales/', + lazy: true, + strategy: 'no_prefix', + detectBrowserLanguage: { + fallbackLocale: defaultLocale.code + } + } +}) diff --git a/specs/fixtures/issues/2382/package.json b/specs/fixtures/issues/2382/package.json new file mode 100644 index 000000000..b6a6c3053 --- /dev/null +++ b/specs/fixtures/issues/2382/package.json @@ -0,0 +1,14 @@ +{ + "name": "nuxt3-test-issues-2382", + "private": true, + "scripts": { + "build": "nuxt build", + "dev": "nuxt dev", + "generate": "nuxt generate", + "preview": "nuxt preview" + }, + "devDependencies": { + "@nuxtjs/i18n": "latest", + "nuxt": "latest" + } +} diff --git a/specs/fixtures/issues/2382/pages/index.vue b/specs/fixtures/issues/2382/pages/index.vue new file mode 100644 index 000000000..e9616aada --- /dev/null +++ b/specs/fixtures/issues/2382/pages/index.vue @@ -0,0 +1,5 @@ + diff --git a/specs/fixtures/issues/2382/pages/level-1/[id]/index.vue b/specs/fixtures/issues/2382/pages/level-1/[id]/index.vue new file mode 100644 index 000000000..70211b130 --- /dev/null +++ b/specs/fixtures/issues/2382/pages/level-1/[id]/index.vue @@ -0,0 +1,4 @@ + diff --git a/specs/fixtures/issues/2382/pages/level-1/index.vue b/specs/fixtures/issues/2382/pages/level-1/index.vue new file mode 100644 index 000000000..6f44fff41 --- /dev/null +++ b/specs/fixtures/issues/2382/pages/level-1/index.vue @@ -0,0 +1,3 @@ + diff --git a/specs/issues/2382.spec.ts b/specs/issues/2382.spec.ts new file mode 100644 index 000000000..03bb82b3f --- /dev/null +++ b/specs/issues/2382.spec.ts @@ -0,0 +1,49 @@ +import { test, expect, describe } from 'vitest' +import { fileURLToPath } from 'node:url' +import { URL } from 'node:url' +import { setup, url, createPage } from '../utils' +import { getText } from '../helper' + +describe('#2382', async () => { + await setup({ + rootDir: fileURLToPath(new URL(`../fixtures/issues/2382`, import.meta.url)) + }) + + test('should handle navigation with dynamic routes without special character', async () => { + const home = url('/') + const page = await createPage(undefined, { locale: 'en' }) + await page.goto(home) + await page.locator('#level-1-no-special-character').click() + + expect(await getText(page, '#title')).toEqual(`sub page level-1`) + }) + + test('should handle navigation with dynamic routes with special character', async () => { + const home = url('/') + const page = await createPage(undefined, { locale: 'en' }) + await page.goto(home) + await page.locator('#level-2-with-special-character').click() + + expect(await getText(page, '#title')).toEqual(`sub page level-2 with route param id`) + }) + + test('should handle navigation from dynamic route with special character', async () => { + const home = url('/level-1/somepath-with-ö') + const page = await createPage(undefined, { locale: 'en' }) + await page.goto(home) + expect(await getText(page, '#title')).toEqual(`sub page level-2 with route param id`) + + await page.locator('#home').click() + expect(await getText(page, '#title')).toEqual(`main page with link to sub page with route param`) + }) + + test('should handle navigation from dynamic route and query parameters with special character', async () => { + const home = url('/level-1/somepath-with-ö?foo=bär') + const page = await createPage(undefined, { locale: 'en' }) + await page.goto(home) + expect(await getText(page, '#title')).toEqual(`sub page level-2 with route param id`) + + await page.locator('#home').click() + expect(await getText(page, '#title')).toEqual(`main page with link to sub page with route param`) + }) +}) diff --git a/src/runtime/utils.ts b/src/runtime/utils.ts index f27188e68..784bf7e97 100644 --- a/src/runtime/utils.ts +++ b/src/runtime/utils.ts @@ -315,12 +315,22 @@ export function detectRedirect({ const routePath = context.$switchLocalePath(targetLocale) || context.$localePath(toFullPath, targetLocale) __DEBUG__ && console.log('detectRedirect: calculate routePath -> ', routePath, toFullPath) if (isString(routePath) && routePath && !isEqual(routePath, toFullPath) && !routePath.startsWith('//')) { + /** + * NOTE: for #2382 + * If the current path contains any special characters like whitespaces or äöü we have to make sure that these are encoded properly, + * otherwise the path won't match route.from.fullPath and falsy set as redirectPath. + * Since routePath already contains properly encoded query parameters, these have to be extracted from the route to ensure that the query is not encoded multiple times. * + * (Looks like an issue within vue-i18n-routing since query parameters are properly encoded) + */ + const splitRoutePath = routePath.split('?') + splitRoutePath[0] = encodeURI(splitRoutePath[0]) + const properlyEncodedRoutePath = splitRoutePath.join('?') /** * NOTE: for #1889, #2226 * If it's the same as the previous route path, respect the current route without redirecting. * (If an empty string is set, the current route is respected. after this function return, it's pass navigate function) */ - redirectPath = !(route.from && route.from.fullPath === routePath) ? routePath : '' + redirectPath = !(route.from && route.from.fullPath === properlyEncodedRoutePath) ? properlyEncodedRoutePath : '' } } From d7c24adb5e3ee30e812eba249bb60ce08243a1b0 Mon Sep 17 00:00:00 2001 From: Bobbie Goede Date: Tue, 17 Oct 2023 19:56:18 +0200 Subject: [PATCH 2/2] test: add `waitForURL` to reduce flakiness --- specs/issues/2382.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/specs/issues/2382.spec.ts b/specs/issues/2382.spec.ts index 03bb82b3f..3efe62780 100644 --- a/specs/issues/2382.spec.ts +++ b/specs/issues/2382.spec.ts @@ -14,6 +14,7 @@ describe('#2382', async () => { const page = await createPage(undefined, { locale: 'en' }) await page.goto(home) await page.locator('#level-1-no-special-character').click() + await page.waitForURL('**/level-1') expect(await getText(page, '#title')).toEqual(`sub page level-1`) }) @@ -23,6 +24,7 @@ describe('#2382', async () => { const page = await createPage(undefined, { locale: 'en' }) await page.goto(home) await page.locator('#level-2-with-special-character').click() + await page.waitForURL(`**/level-1/${encodeURI('lövöl-2')}`) expect(await getText(page, '#title')).toEqual(`sub page level-2 with route param id`) }) @@ -31,9 +33,11 @@ describe('#2382', async () => { const home = url('/level-1/somepath-with-ö') const page = await createPage(undefined, { locale: 'en' }) await page.goto(home) + await page.waitForURL(`**/${encodeURI('level-1/somepath-with-ö')}**`) expect(await getText(page, '#title')).toEqual(`sub page level-2 with route param id`) await page.locator('#home').click() + await page.waitForURL(/\/$/) expect(await getText(page, '#title')).toEqual(`main page with link to sub page with route param`) }) @@ -41,9 +45,11 @@ describe('#2382', async () => { const home = url('/level-1/somepath-with-ö?foo=bär') const page = await createPage(undefined, { locale: 'en' }) await page.goto(home) + await page.waitForURL(`**/${encodeURI('level-1/somepath-with-ö')}**`) expect(await getText(page, '#title')).toEqual(`sub page level-2 with route param id`) await page.locator('#home').click() + await page.waitForURL(/\/$/) expect(await getText(page, '#title')).toEqual(`main page with link to sub page with route param`) }) })