Skip to content

Commit

Permalink
fix: don't apply any locale-logic to non-existent routes (#1093)
Browse files Browse the repository at this point in the history
If the resolved path for a given locale is 404 then return
resolved route based on the original path input rather than the
locale-adjusted path. This affects redirects on locale change, page
load, and the behavior of localePath and localeRoute APIs.

Avoids redirecting unnecessarily to a route that doesn't exist anyway.

That also fixes the security issue with redirecting to a different
domain but just in case added an additional measure against that.

Resolves #1092
  • Loading branch information
rchl committed Mar 7, 2021
1 parent 4043968 commit 7180412
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/templates/plugin.main.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export default async (context) => {
redirectPath = app.localePath(route.fullPath, locale)
}

if (redirectPath === route.fullPath) {
if (!redirectPath || redirectPath === route.fullPath || redirectPath.startsWith('//')) {
return ''
}

Expand Down
25 changes: 15 additions & 10 deletions src/templates/plugin.routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,6 @@ function localeRoute (route, locale) {
let localizedRoute = Object.assign({}, route)

if (route.path && !route.name) {
const isDefaultLocale = locale === defaultLocale
// if route has a path defined but no name, resolve full route using the path
const isPrefixed =
// don't prefix default locale
!(isDefaultLocale && [STRATEGIES.PREFIX_EXCEPT_DEFAULT, STRATEGIES.PREFIX_AND_DEFAULT].includes(strategy)) &&
// no prefix for any language
!(strategy === STRATEGIES.NO_PREFIX) &&
// no prefix for different domains
!i18n.differentDomains
const resolvedRoute = this.router.resolve(route.path).route
const resolvedRouteName = this.getRouteBaseName(resolvedRoute)
if (resolvedRouteName) {
Expand All @@ -71,6 +62,15 @@ function localeRoute (route, locale) {
hash: resolvedRoute.hash
}
} else {
const isDefaultLocale = locale === defaultLocale
// if route has a path defined but no name, resolve full route using the path
const isPrefixed =
// don't prefix default locale
!(isDefaultLocale && [STRATEGIES.PREFIX_EXCEPT_DEFAULT, STRATEGIES.PREFIX_AND_DEFAULT].includes(strategy)) &&
// no prefix for any language
!(strategy === STRATEGIES.NO_PREFIX) &&
// no prefix for different domains
!i18n.differentDomains
if (isPrefixed) {
localizedRoute.path = `/${locale}${route.path}`
}
Expand All @@ -89,7 +89,12 @@ function localeRoute (route, locale) {
}
}

return this.router.resolve(localizedRoute).route
const resolvedRoute = this.router.resolve(localizedRoute).route
if (resolvedRoute.name) {
return resolvedRoute
}
// If didn't resolve to an existing route then just return resolved route based on original input.
return this.router.resolve(route).route
}

function switchLocalePath (locale) {
Expand Down
2 changes: 1 addition & 1 deletion test/browser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,7 @@ describe(`${browserString} (SPA)`, () => {
page = await browser.newPage({ locale: 'fr' })
await page.goto(url(path))
expect(await (await page.$('body'))?.textContent()).toContain('page could not be found')
expect(await getRouteFullPath(page)).toBe(`/fr${path}`)
expect(await getRouteFullPath(page)).toBe(path)
})
})

Expand Down

0 comments on commit 7180412

Please sign in to comment.