Skip to content

Commit

Permalink
fix: crash with invalid locale cookie + no_prefix strategy + SEO (#666)
Browse files Browse the repository at this point in the history
The SEO code has crashed due to trying to get `iso` property for locale
that didn't exist. It's because we've allowed to set `$i18n.locale` to
an invalid value due to not validating the locale cookie in all cases.

This crash only triggered when certain conditions have aligned:
 - using `no_prefix` strategy as then middleware can't get locale from route
 - using `detectBrowserLanguage.alwaysRedirect` as then we try to redirect
   even when cookie is already set
 - using `seo = true` or manual SEO integration as the crash happened
   in SEO code

Change `getLocaleCookie` to only return locale code if it's valid.
  • Loading branch information
rchl committed Apr 14, 2020
1 parent 07e0338 commit 2ec72bc
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 12 deletions.
16 changes: 9 additions & 7 deletions src/plugins/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,17 @@ export default async (context) => {

const getLocaleCookie = () => {
if (useCookie) {
let localeCode

if (process.client) {
return JsCookie.get(cookieKey)
localeCode = JsCookie.get(cookieKey)
} else if (req && typeof req.headers.cookie !== 'undefined') {
const cookies = req.headers && req.headers.cookie ? Cookie.parse(req.headers.cookie) : {}
return cookies[cookieKey]
localeCode = cookies[cookieKey]
}

if (localeCodes.includes(localeCode)) {
return localeCode
}
}
}
Expand Down Expand Up @@ -223,11 +229,7 @@ export default async (context) => {
const routeLocale = getLocaleFromRoute(route)
locale = routeLocale || locale
} else if (useCookie) {
const localeCookie = getLocaleCookie()

if (localeCodes.includes(localeCookie)) {
locale = localeCookie
}
locale = getLocaleCookie() || locale
}

await loadAndSetLocale(locale, { initialSetup: true })
Expand Down
42 changes: 37 additions & 5 deletions test/module.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,8 @@ describe('differentDomains enabled', () => {
}
const html = await get('/', requestOptions)
const dom = getDom(html)
await expect(dom.querySelector('body').textContent).toContain('page: Homepage')
await expect(dom.querySelector('head meta[property="og-locale"]')).toBe(null)
expect(dom.querySelector('body').textContent).toContain('page: Homepage')
expect(dom.querySelector('head meta[property="og-locale"]')).toBe(null)
})

test('host matches locale\'s domain (fr)', async () => {
Expand All @@ -890,7 +890,7 @@ describe('differentDomains enabled', () => {
}
const html = await get('/', requestOptions)
const dom = getDom(html)
await expect(dom.querySelector('body').textContent).toContain('page: Accueil')
expect(dom.querySelector('body').textContent).toContain('page: Accueil')
})

test('x-forwarded-host does not match locale\'s domain', async () => {
Expand All @@ -902,7 +902,7 @@ describe('differentDomains enabled', () => {
const html = await get('/', requestOptions)
const dom = getDom(html)
// Falls back to english.
await expect(dom.querySelector('body').textContent).toContain('page: Homepage')
expect(dom.querySelector('body').textContent).toContain('page: Homepage')
})

test('x-forwarded-host does match locale\'s domain (fr)', async () => {
Expand All @@ -913,7 +913,7 @@ describe('differentDomains enabled', () => {
}
const html = await get('/', requestOptions)
const dom = getDom(html)
await expect(dom.querySelector('body').textContent).toContain('page: Accueil')
expect(dom.querySelector('body').textContent).toContain('page: Accueil')
})
})

Expand Down Expand Up @@ -1045,3 +1045,35 @@ describe('vuex disabled', () => {
expect(getDom(await get('/fr')).querySelector('#current-locale').textContent).toBe('locale: fr')
})
})

describe('no_prefix + detectBrowserLanguage + alwaysRedirect', () => {
let nuxt

beforeAll(async () => {
const override = {
i18n: {
strategy: 'no_prefix',
detectBrowserLanguage: {
alwaysRedirect: true
}
}
}

nuxt = (await setup(loadConfig(__dirname, 'basic', override, { merge: true }))).nuxt
})

afterAll(async () => {
await nuxt.close()
})

test('fallbacks to default locale with invalid locale cookie', async () => {
const requestOptions = {
headers: {
Cookie: 'i18n_redirected=invalid'
}
}
const html = await get('/', requestOptions)
const dom = getDom(html)
expect(dom.querySelector('#current-locale').textContent).toBe('locale: en')
})
})

0 comments on commit 2ec72bc

Please sign in to comment.