fix: inconsistent use of accept-header header

commit 6d80189
Merge: 4746946 a7f04a3
Author: Rickard Falk <[email protected]>
Date:   Wed May 8 08:44:25 2024 +0200

    Merge pull request #57 from KTH/fix/language-error

    Fix language error, add tests for it and refactor the init method

commit a7f04a3
Author: Pipeline <[email protected]>
Date:   Thu May 2 12:09:48 2024 +0000

    9.3.1-0

commit cf61720
Author: Rickard Falk <[email protected]>
Date:   Thu May 2 14:03:39 2024 +0200

    refactor language tests

commit da2a120
Author: Rickard Falk <[email protected]>
Date:   Thu May 2 14:03:10 2024 +0200

    fix: set res.locals.locale.language

commit 6be3590
Author: Rickard Falk <[email protected]>
Date:   Tue Apr 30 10:24:51 2024 +0200

    add tests for res.locals.locale.language

commit 9078fe5
Author: Rickard Falk <[email protected]>
Date:   Tue Apr 30 08:43:53 2024 +0200

    fix: accept-language header contains preferred languages, not a single language

commit d46d132
Author: Ellen <[email protected]>
Date:   Mon Apr 29 15:42:22 2024 +0200

    Refactor init method for language

commit 8f96edc
Author: Ellen <[email protected]>
Date:   Mon Apr 29 15:41:57 2024 +0200

    Add some more tests

commit e08ce20
Author: mthege <[email protected]>
Date:   Mon Apr 29 14:55:38 2024 +0200

    feat: tests for language.js

commit 3d5dd4d
Author: Rickard Falk <[email protected]>
Date:   Mon Apr 29 13:34:03 2024 +0200

    fix: use chosenLocale for cookie language

Author: falric

lib/language.js

@@ -7,42 +7,41 @@
 const locale = require('locale')
 
 const cookieName = 'language'
+const cookieOptions = { secure: true, sameSite: 'strict', httpOnly: true }
 const validLanguages = ['sv', 'en']
 const defaultLanguage = validLanguages[0]
 const supportedLocales = new locale.Locales(validLanguages, defaultLanguage)
 
+const validLanguage = lang => {
+  if (validLanguages.includes(lang)) {
+    return lang
+  }
+}
+
+const useAcceptLanguageOrDefault = preferredLanguages => {
+  const preferredLocales = new locale.Locales(preferredLanguages, defaultLanguage)
+  const bestLocale = preferredLocales.best(supportedLocales)
+  return bestLocale.language
+}
+
 /**
  * Initialize locale and language for the current user session (respects cookie: language).
  */
 function _init(req, res, newLang) {
-  let lang = newLang || req.cookies[cookieName]
+  const lang =
+    validLanguage(newLang) ||
+    validLanguage(req.cookies[cookieName]) ||
+    useAcceptLanguageOrDefault(req.headers['accept-language'])
 
-  // Only allow lang to be one of the valid languages
-  if (lang && !validLanguages.includes(lang)) lang = defaultLanguage
-
-  let chosenLocale
-
-  if (lang) {
-    const locales = new locale.Locales(lang, defaultLanguage)
-    chosenLocale = locales.best(supportedLocales)
-  } else {
-    const locales = new locale.Locales(req.headers['accept-language'], defaultLanguage)
-    chosenLocale = locales.best(supportedLocales)
+  if (req.cookies[cookieName] !== lang) {
+    res.cookie(cookieName, lang, cookieOptions)
   }
 
-  // If we got an explicit language we set the language cookie
-  if (newLang) {
-    res.cookie(cookieName, chosenLocale.language, { secure: true, sameSite: 'strict', httpOnly: true })
-  } else if (!req.cookies[cookieName]) {
-    // Make sure language cookie is set so subsequent requests are guaranteed to use the same language
-    res.cookie(cookieName, lang || 'sv', { secure: true, sameSite: 'strict', httpOnly: true })
-  }
-
-  res.locals.locale = chosenLocale
+  const locales = new locale.Locales(lang)
+  const bestLocale = locales.best(supportedLocales)
+  res.locals.locale = bestLocale
   // Backwards compatibility only in case someone accessed the prop directly:
-  res.locals.language = chosenLocale.language
-
-  return chosenLocale
+  res.locals.language = bestLocale.language
 }
 
 /**
@@ -77,7 +76,6 @@ function _cookieLanguage(req) {
 
 module.exports = {
   getLanguage: _getLanguage,
-  init: _init,
   validLanguages,
   defaultLanguage,
   languageHandler: _languageHandler,

lib/language.test.js

@@ -0,0 +1,113 @@
+// const locale = require('locale')
+const { languageHandler } = require('./language')
+
+describe('languageHandler', () => {
+  const cookieOptions = { httpOnly: true, sameSite: 'strict', secure: true }
+  const cookieName = 'language'
+
+  it('should save language if requested and valid', () => {
+    const requestedLanguage = 'sv'
+    const req = { query: { l: requestedLanguage }, cookies: {}, headers: {} }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).toHaveBeenCalledWith(cookieName, requestedLanguage, cookieOptions)
+    expect(res.locals.locale.language).toEqual(requestedLanguage)
+  })
+
+  it('should not save language in cookie if language has not changed', () => {
+    const presetLanguage = 'sv'
+    const req = { cookies: { [cookieName]: presetLanguage }, query: {}, headers: {} }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).not.toHaveBeenCalled()
+  })
+
+  it('should not save invalid language', () => {
+    const invalidLanguage = 'fr'
+    const defaultLanguage = 'sv'
+    const req = { query: { l: invalidLanguage }, cookies: {}, headers: {} }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).toHaveBeenCalledWith(cookieName, defaultLanguage, cookieOptions)
+    expect(res.locals.locale.language).toEqual(defaultLanguage)
+  })
+
+  it('should use default language as fallback', () => {
+    const defaultLanguage = 'sv'
+    const req = { query: {}, cookies: {}, headers: {} }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).toHaveBeenCalledWith(cookieName, defaultLanguage, cookieOptions)
+    expect(res.locals.locale.language).toEqual(defaultLanguage)
+  })
+
+  it('should update language if requested ', () => {
+    const requestedLanguage = 'en'
+    const presetLanguage = 'sv'
+    const req = {
+      query: { l: requestedLanguage },
+      cookies: { [cookieName]: presetLanguage },
+      headers: {},
+    }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).toHaveBeenCalledWith(cookieName, requestedLanguage, cookieOptions)
+    expect(res.locals.locale.language).toEqual(requestedLanguage)
+  })
+
+  it('should use accept-language if requested language and cookie are unset', () => {
+    const acceptLanguage = 'en,sv;q=0.9'
+    const expectedLanguage = 'en'
+    const req = { query: {}, cookies: {}, headers: { 'accept-language': acceptLanguage } }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).toHaveBeenCalledWith(cookieName, expectedLanguage, cookieOptions)
+    expect(res.locals.locale.language).toEqual(expectedLanguage)
+  })
+
+  it('should use accept-language if requested language is invalid and cookie is unset', () => {
+    const requestedLanguage = 'fr'
+    const acceptLanguage = 'en,sv;q=0.9'
+    const expectedLanguage = 'en'
+    const req = {
+      query: { l: requestedLanguage },
+      cookies: {},
+      headers: { 'accept-language': acceptLanguage },
+    }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).toHaveBeenCalledWith(cookieName, expectedLanguage, cookieOptions)
+    expect(res.locals.locale.language).toEqual(expectedLanguage)
+  })
+
+  it('should use accept-language if cookie language is invalid', () => {
+    const cookieLanguage = 'fr'
+    const acceptLanguage = 'en,sv;q=0.9'
+    const expectedLanguage = 'en'
+    const req = {
+      query: {},
+      cookies: { [cookieName]: cookieLanguage },
+      headers: { 'accept-language': acceptLanguage },
+    }
+    const res = { locals: {}, cookie: jest.fn() }
+    const next = jest.fn()
+
+    languageHandler(req, res, next)
+    expect(res.cookie).toHaveBeenCalledWith(cookieName, expectedLanguage, cookieOptions)
+    expect(res.locals.locale.language).toEqual(expectedLanguage)
+  })
+})

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@kth/kth-node-web-common",
-  "version": "9.3.0",
+  "version": "9.3.1-0",
   "description": "Common components for node-web projects",
   "scripts": {
     "test": "jest",