diff --git a/config/default-search.js b/config/default-search.js index e72f8b6ff06..2f5077bdce7 100644 --- a/config/default-search.js +++ b/config/default-search.js @@ -19,5 +19,5 @@ module.exports = { }, }, - redirectLangPrefix: false, + enablePrefixMiddleware: false, }; diff --git a/config/default.js b/config/default.js index dc3c77eb276..6318e480b12 100644 --- a/config/default.js +++ b/config/default.js @@ -140,7 +140,8 @@ module.exports = { }, rtlLangs: ['ar', 'dbr', 'fa', 'he'], defaultLang: 'en-US', - redirectLangPrefix: true, + + enablePrefixMiddleware: true, localeDir: path.resolve(path.join(__dirname, '../locale')), @@ -150,4 +151,14 @@ module.exports = { trackingId: null, enablePostCssLoader: true, + + // The list of valid client application names. These are derived from UA strings when + // not supplied in the URL. + validClientApplications: [ + 'firefox', + 'android', + ], + + // The default app used in the URL. + defaultClientApp: 'firefox', }; diff --git a/package.json b/package.json index 789ce5832c3..286b2ad454b 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "eslint": "eslint .", "stylelint": "stylelint --syntax scss **/*.scss", "lint": "npm run eslint && npm run stylelint", - "servertest": "ADDONS_FRONTEND_BUILD_ALL=1 npm run build && better-npm-run servertest && better-npm-run servertest:disco && better-npm-run servertest:search", + "servertest": "ADDONS_FRONTEND_BUILD_ALL=1 npm run build && better-npm-run servertest && better-npm-run servertest:amo && better-npm-run servertest:disco && better-npm-run servertest:search", "start": "npm run version-check && NODE_PATH='./:./src' node bin/server.js", "test": "better-npm-run test", "unittest": "better-npm-run unittest", @@ -70,6 +70,15 @@ "TRACKING_ENABLED": "false" } }, + "servertest:amo": { + "command": "mocha --compilers js:babel-register --timeout 10000 tests/server/amo", + "env": { + "NODE_PATH": "./:./src", + "NODE_ENV": "production", + "NODE_APP_INSTANCE": "amo", + "TRACKING_ENABLED": "false" + } + }, "servertest:disco": { "command": "mocha --compilers js:babel-register --timeout 10000 tests/server/disco", "env": { diff --git a/src/amo/containers/DetailPage.js b/src/amo/containers/DetailPage.js new file mode 100644 index 00000000000..81dc7356e8d --- /dev/null +++ b/src/amo/containers/DetailPage.js @@ -0,0 +1,11 @@ +import React from 'react'; + +export default class DetailPage extends React.Component { + render() { + return ( +
+

Detail Page

+
+ ); + } +} diff --git a/src/amo/css/App.scss b/src/amo/css/App.scss index 3d05f597a38..18117c796a1 100644 --- a/src/amo/css/App.scss +++ b/src/amo/css/App.scss @@ -4,5 +4,5 @@ html, body { - background: red; + background: #fff; } diff --git a/src/amo/routes.js b/src/amo/routes.js index 7ae242e1318..08672bb923e 100644 --- a/src/amo/routes.js +++ b/src/amo/routes.js @@ -3,9 +3,11 @@ import { IndexRoute, Route } from 'react-router'; import App from './containers/App'; import Home from './containers/Home'; +import DetailPage from './containers/DetailPage'; export default ( - + + ); diff --git a/src/core/client/logger.js b/src/core/client/logger.js index ac7aec39af7..aa88838281d 100644 --- a/src/core/client/logger.js +++ b/src/core/client/logger.js @@ -34,7 +34,7 @@ export function bindConsoleMethod(consoleMethodName, { _consoleObj = window.cons } const log = {}; -['log', 'info', 'error', 'warn'].forEach((logMethodName) => { +['log', 'debug', 'info', 'error', 'warn'].forEach((logMethodName) => { log[logMethodName] = bindConsoleMethod(logMethodName); }); diff --git a/src/core/i18n/utils.js b/src/core/i18n/utils.js index 9ea002bd81b..0c9f6ffb9e6 100644 --- a/src/core/i18n/utils.js +++ b/src/core/i18n/utils.js @@ -1,16 +1,17 @@ -/* eslint-disable no-console */ - import config from 'config'; +import log from 'core/logger'; const defaultLang = config.get('defaultLang'); const langs = config.get('langs'); const langMap = config.get('langMap'); -const validLangs = langs.concat(Object.keys(langMap)); +// The full list of supported langs including those that +// will be mapped by sanitizeLanguage. +const supportedLangs = langs.concat(Object.keys(langMap)); const rtlLangs = config.get('rtlLangs'); -export function localeToLang(locale, log_ = console) { - let lang = ''; +export function localeToLang(locale, log_ = log) { + let lang; if (locale && locale.split) { const parts = locale.split('_'); if (parts.length === 1) { @@ -30,8 +31,8 @@ export function localeToLang(locale, log_ = console) { return lang; } -export function langToLocale(language, log_ = console) { - let locale = ''; +export function langToLocale(language, log_ = log) { + let locale; if (language && language.split) { const parts = language.split('-'); if (parts.length === 1) { @@ -59,14 +60,18 @@ export function normalizeLocale(locale) { return langToLocale(localeToLang(locale)); } -export function isValidLang(lang, { _validLangs = validLangs } = {}) { - return _validLangs.includes(normalizeLang(lang)); +export function isSupportedLang(lang, { _supportedLangs = supportedLangs } = {}) { + return _supportedLangs.includes(lang); +} + +export function isValidLang(lang, { _langs = langs } = {}) { + return _langs.includes(lang); } export function sanitizeLanguage(langOrLocale) { let language = normalizeLang(langOrLocale); // Only look in the un-mapped lang list. - if (!langs.includes(language)) { + if (!isValidLang(language)) { language = langMap.hasOwnProperty(language) ? langMap[language] : defaultLang; } return language; @@ -123,41 +128,45 @@ export function parseAcceptLanguage(header) { /* * Given an accept-language header and a list of currently - * supported languages, returns the best match with no normalization. + * supported languages, returns the best match normalized. + * + * Note: this doesn't map languages e.g. pt -> pt-PT. Use sanitizeLanguage for that. * */ -export function getLangFromHeader(acceptLanguage, { _validLangs } = {}) { +export function getLangFromHeader(acceptLanguage, { _supportedLangs } = {}) { let userLang; if (acceptLanguage) { const langList = parseAcceptLanguage(acceptLanguage); for (const langPref of langList) { - if (isValidLang(langPref.lang, { _validLangs })) { + if (isSupportedLang(normalizeLang(langPref.lang), { _supportedLangs })) { userLang = langPref.lang; break; // Match locale, even if region isn't supported - } else if (isValidLang(langPref.lang.split('-')[0], { _validLangs })) { + } else if (isSupportedLang(normalizeLang(langPref.lang.split('-')[0]), { _supportedLangs })) { userLang = langPref.lang.split('-')[0]; break; } } } - return userLang; + return normalizeLang(userLang); } /* - * Looks up the language from the router renderProps. - * When that fails fall-back to accept-language. + * Check validity of language: + * - If invalid, fall-back to accept-language. + * - Return object with lang and isLangFromHeader hint. * */ -export function getFilteredUserLanguage({ renderProps, acceptLanguage } = {}) { - let userLang; - // Get the lang from the url param by default if it exists. - if (renderProps && renderProps.params && renderProps.params.lang) { - userLang = renderProps.params.lang; - } - // If we don't have a valid userLang set yet try accept-language. - if (!isValidLang(userLang) && acceptLanguage) { +export function getLanguage({ lang, acceptLanguage } = {}) { + let userLang = lang; + let isLangFromHeader = false; + // If we don't have a supported userLang yet try accept-language. + if (!isSupportedLang(normalizeLang(userLang)) && acceptLanguage) { userLang = getLangFromHeader(acceptLanguage); + isLangFromHeader = true; } - return sanitizeLanguage(userLang); + // sanitizeLanguage will perform the following: + // - mapping e.g. pt -> pt-PT. + // - normalization e.g: en-us -> en-US. + return { lang: sanitizeLanguage(userLang), isLangFromHeader }; } diff --git a/src/core/middleware.js b/src/core/middleware.js new file mode 100644 index 00000000000..30f1b7c465d --- /dev/null +++ b/src/core/middleware.js @@ -0,0 +1,82 @@ +import { getClientApp, isValidClientApp } from 'core/utils'; +import { getLanguage, isValidLang } from 'core/i18n/utils'; +import config from 'config'; +import log from 'core/logger'; + + +export function prefixMiddleWare(req, res, next, { _config = config } = {}) { + // Split on slashes after removing the leading slash. + const URLParts = req.originalUrl.replace(/^\//, '').split('/'); + log.debug(URLParts); + + // Get lang and app parts from the URL. At this stage they may be incorrect + // or missing. + const [langFromURL, appFromURL] = URLParts; + + // Get language from URL or fall-back to detecting it from accept-language header. + const acceptLanguage = req.headers['accept-language']; + const { lang, isLangFromHeader } = getLanguage({ lang: langFromURL, acceptLanguage }); + + const hasValidLang = isValidLang(langFromURL); + const hasValidClientApp = isValidClientApp(appFromURL, { _config }); + + let prependedLang = false; + + if ((hasValidLang && langFromURL !== lang) || + (!hasValidLang && hasValidClientApp)) { + // Replace the first part of the URL if: + // * It's valid and we've mapped it e.g: pt -> pt-PT. + // * The lang is invalid but we have a valid application + // e.g. /bogus/firefox/. + log.info(`Replacing lang in URL ${URLParts[0]} -> ${lang}`); + URLParts[0] = lang; + } else if (!hasValidLang) { + // If lang wasn't valid or was missing prepend one. + log.info(`Prepending lang to URL: ${lang}`); + URLParts.splice(0, 0, lang); + prependedLang = true; + } + + let isApplicationFromHeader = false; + + if (!hasValidClientApp && prependedLang && + isValidClientApp(URLParts[1], { _config })) { + // We skip prepending an app if we'd previously prepended a lang and the + // 2nd part of the URL is now a valid app. + log.info('Application in URL is valid following prepending a lang.'); + } else if (!hasValidClientApp) { + // If the app supplied is not valid we need to prepend one. + const application = getClientApp(req.headers['user-agent']); + log.info(`Prepending application to URL: ${application}`); + URLParts.splice(1, 0, application); + isApplicationFromHeader = true; + } + + // Redirect to the new URL. + // For safety we'll deny a redirect to a URL starting with '//' since + // that will be treated as a protocol-free URL. + const newURL = `/${URLParts.join('/')}`; + if (newURL !== req.originalUrl && !newURL.startsWith('//')) { + // Collect vary headers to apply to the redirect + // so we can make it cacheable. + // TODO: Make the redirects cacheable by adding expires headers. + const varyHeaders = []; + if (isLangFromHeader) { + varyHeaders.push('accept-language'); + } + if (isApplicationFromHeader) { + varyHeaders.push('user-agent'); + } + res.set('vary', varyHeaders); + return res.redirect(302, newURL); + } + + // Add the data to res.locals to be utilised later. + /* eslint-disable no-param-reassign */ + const [newLang, newApp] = URLParts; + res.locals.lang = newLang; + res.locals.clientApp = newApp; + /* eslint-enable no-param-reassign */ + + return next(); +} diff --git a/src/core/server/base.js b/src/core/server/base.js index dba233a8903..7da6b4d5116 100644 --- a/src/core/server/base.js +++ b/src/core/server/base.js @@ -11,6 +11,7 @@ import ReactDOM from 'react-dom/server'; import { Provider } from 'react-redux'; import { match } from 'react-router'; import { ReduxAsyncConnect, loadOnServer } from 'redux-async-connect'; +import { prefixMiddleWare } from 'core/middleware'; import WebpackIsomorphicTools from 'webpack-isomorphic-tools'; import WebpackIsomorphicToolsConfig from 'webpack-isomorphic-tools-config'; @@ -20,7 +21,7 @@ import config from 'config'; import { convertBoolean } from 'core/utils'; import { setLang, setJWT } from 'core/actions'; import log from 'core/logger'; -import { getDirection, getFilteredUserLanguage, langToLocale } from 'core/i18n/utils'; +import { getDirection, isValidLang, langToLocale } from 'core/i18n/utils'; import I18nProvider from 'core/i18n/Provider'; import Jed from 'jed'; @@ -91,9 +92,14 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { res.redirect(302, '/en-US/firefox/discovery/pane/48.0/Darwin/normal')); } + // Handle application and lang redirections. + if (config.get('enablePrefixMiddleware')) { + app.use(prefixMiddleWare); + } + app.use((req, res) => { if (isDevelopment) { - log.info('Running in Development Mode'); + log.info('Clearing require cache for webpack isomorphic tools. [Development Mode]'); // clear require() cache if in development mode webpackIsomorphicTools.refresh(); @@ -121,28 +127,9 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { fs.readFileSync(path.join(config.get('basePath'), 'dist/sri.json')) ) : {}; - const acceptLanguage = req.headers['accept-language']; - // Get language from URL or fall-back to accept-language. - const lang = getFilteredUserLanguage({ renderProps, acceptLanguage }); - - if (renderProps.params) { - const origLang = renderProps.params.lang; - if (lang !== origLang) { - // If a lang was provided but the lang looked up is different - // redirect to the looked-up lang (or default). - // eslint-disable-next-line no-unused-vars - const [_, firstPart, ...rest] = req.originalUrl.split('/'); - if (origLang === decodeURIComponent(firstPart)) { - // The '' provides a leading / - return res.redirect(302, ['', lang, ...rest].join('/')); - } else if (!origLang && config.get('redirectLangPrefix')) { - // If there was no lang param. Redirect to the same URL with - // a lang prepended. - return res.redirect(302, `/${lang}${req.originalUrl}`); - } - } - } - + // Check the lang supplied by res.locals.lang for validity + // or fall-back to the default. + const lang = isValidLang(res.locals.lang) ? res.locals.lang : config.get('defaultLang'); const dir = getDirection(lang); const locale = langToLocale(lang); store.dispatch(setLang(lang)); @@ -181,8 +168,8 @@ function baseServer(routes, createStore, { appInstanceName = appName } = {}) { jedData = require(`json!../../locale/${locale}/${appInstanceName}.json`); } } catch (e) { - log.info(dedent`Locale not found or required for locale: "${locale}". - Falling back to default lang: "${config.get('defaultLang')}"`); + log.info(`Locale JSON not found or required for locale: "${locale}"`); + log.info(`Falling back to default lang: "${config.get('defaultLang')}".`); } const i18n = new Jed(jedData); const InitialComponent = ( diff --git a/src/core/utils.js b/src/core/utils.js index db72001bfeb..f5f03c76a76 100644 --- a/src/core/utils.js +++ b/src/core/utils.js @@ -1,4 +1,5 @@ import camelCase from 'camelcase'; +import config from 'config'; export function gettext(str) { return str; @@ -38,3 +39,12 @@ export function convertBoolean(value) { return false; } } + +export function getClientApp() { + // TODO: Look at user-agent header passed in. + return 'firefox'; +} + +export function isValidClientApp(value, { _config = config } = {}) { + return _config.get('validClientApplications').includes(value); +} diff --git a/src/disco/routes.js b/src/disco/routes.js index 321c1060362..e79757d476f 100644 --- a/src/disco/routes.js +++ b/src/disco/routes.js @@ -7,7 +7,7 @@ import DiscoPane from './containers/DiscoPane'; export default ( diff --git a/tests/client/amo/containers/TestDetail.js b/tests/client/amo/containers/TestDetail.js new file mode 100644 index 00000000000..a97fa551060 --- /dev/null +++ b/tests/client/amo/containers/TestDetail.js @@ -0,0 +1,16 @@ +import React from 'react'; +import { findDOMNode } from 'react-dom'; +import { + renderIntoDocument, +} from 'react-addons-test-utils'; + +import DetailPage from 'amo/containers/DetailPage'; + + +describe('DetailPage', () => { + it('renders a heading', () => { + const root = renderIntoDocument(); + const rootNode = findDOMNode(root); + assert.include(rootNode.querySelector('h1').textContent, 'Detail Page'); + }); +}); diff --git a/tests/client/core/i18n/test_utils.js b/tests/client/core/i18n/test_utils.js index 334051fba2c..18e2cff2789 100644 --- a/tests/client/core/i18n/test_utils.js +++ b/tests/client/core/i18n/test_utils.js @@ -1,6 +1,8 @@ import * as utils from 'core/i18n/utils'; import config from 'config'; +const defaultLang = config.get('defaultLang'); + describe('i18n utils', () => { describe('normalizeLang()', () => { @@ -24,6 +26,11 @@ describe('i18n utils', () => { it('should handle a language with 3 parts', () => { assert.equal(utils.normalizeLang('ja-JP-mac'), 'ja-Mac'); }); + + it('should return undefined for no match', () => { + assert.equal(utils.normalizeLang(1), undefined); + assert.equal(utils.normalizeLang(''), undefined); + }); }); describe('normalizeLocale()', () => { @@ -58,6 +65,11 @@ describe('i18n utils', () => { utils.langToLocale('whatevs-this-is-really-odd', fakeLog); assert.ok(fakeLog.error.called); }); + + it('should return undefined for invalid input', () => { + assert.equal(utils.langToLocale(''), undefined); + assert.equal(utils.langToLocale(1), undefined); + }); }); describe('localeToLang()', () => { @@ -80,6 +92,11 @@ describe('i18n utils', () => { utils.localeToLang('what_the_heck_is_this', fakeLog); assert.ok(fakeLog.error.called); }); + + it('should return undefined for invalid input', () => { + assert.equal(utils.localeToLang(''), undefined); + assert.equal(utils.localeToLang(1), undefined); + }); }); describe('sanitizeLanguage()', () => { @@ -92,11 +109,11 @@ describe('i18n utils', () => { }); it('should return the default if lookup not present', () => { - assert.equal(utils.sanitizeLanguage('awooga'), 'en-US'); + assert.equal(utils.sanitizeLanguage('awooga'), defaultLang); }); it('should return the default if bad type', () => { - assert.equal(utils.sanitizeLanguage(1), 'en-US'); + assert.equal(utils.sanitizeLanguage(1), defaultLang); }); it('should return a lang if handed a locale', () => { @@ -104,7 +121,7 @@ describe('i18n utils', () => { }); it('should return the default if handed undefined', () => { - assert.equal(utils.sanitizeLanguage(undefined), 'en-US'); + assert.equal(utils.sanitizeLanguage(undefined), defaultLang); }); it('should return the default if handed an empty string', () => { @@ -135,6 +152,14 @@ describe('i18n utils', () => { }); describe('isValidLang()', () => { + it('should see en-us as an invalid lang', () => { + assert.equal(utils.isValidLang('en-us'), false); + }); + + it('should see en_US as an invalid lang', () => { + assert.equal(utils.isValidLang('en_US'), false); + }); + it('should see en-US as a valid lang', () => { assert.equal(utils.isValidLang('en-US'), true); }); @@ -147,85 +172,87 @@ describe('i18n utils', () => { assert.equal(utils.isValidLang('awooga'), false); }); - it('should see pt as a valid lang', () => { - assert.equal(utils.isValidLang('pt'), true); + it('should see pt as an invalid lang since it requires mapping', () => { + assert.equal(utils.isValidLang('pt'), false); + }); + }); + + describe('isSupportedLang()', () => { + it('should see en-us as an unsupported lang', () => { + assert.equal(utils.isSupportedLang('en-us'), false); + }); + + it('should see en_US as an unsupported lang', () => { + assert.equal(utils.isSupportedLang('en_US'), false); + }); + + it('should see en-US as a supported lang', () => { + assert.equal(utils.isSupportedLang('en-US'), true); + }); + + it('should see incorrect type as unsupported lang', () => { + assert.equal(utils.isSupportedLang(1), false); + }); + + it('should see bogus value as an unsupported lang', () => { + assert.equal(utils.isSupportedLang('awooga'), false); + }); + + it('should see pt as a supported lang (requires mapping)', () => { + assert.equal(utils.isSupportedLang('pt'), true); }); }); - describe('getFilteredUserLanguage()', () => { + describe('getLanguage()', () => { it('should return default lang if called without args', () => { - assert.equal(utils.getFilteredUserLanguage(), config.get('defaultLang')); + const result = utils.getLanguage(); + assert.equal(result.lang, defaultLang); + assert.equal(result.isLangFromHeader, false); }); it('should return default lang if no lang is provided', () => { - const fakeRenderProps = {}; - const result = utils.getFilteredUserLanguage({ renderProps: fakeRenderProps }); - assert.equal(result, config.get('defaultLang')); + const result = utils.getLanguage({ lang: '' }); + assert.equal(result.lang, defaultLang); + assert.equal(result.isLangFromHeader, false); }); it('should return default lang if bad lang is provided', () => { - const fakeRenderProps = { - params: { - lang: 'bogus', - }, - }; - const result = utils.getFilteredUserLanguage({ renderProps: fakeRenderProps }); - assert.equal(result, config.get('defaultLang')); + const result = utils.getLanguage({ lang: 'bogus' }); + assert.equal(result.lang, defaultLang); + assert.equal(result.isLangFromHeader, false); }); it('should return default lang if bad lang type provided', () => { - const fakeRenderProps = { - params: { - lang: 1, - }, - }; - const result = utils.getFilteredUserLanguage({ renderProps: fakeRenderProps }); - assert.equal(result, config.get('defaultLang')); + const result = utils.getLanguage({ lang: 1 }); + assert.equal(result.lang, defaultLang); + assert.equal(result.isLangFromHeader, false); }); it('should return lang if provided via the URL', () => { - const fakeRenderProps = { - params: { - lang: 'fr', - }, - }; - assert.equal(utils.getFilteredUserLanguage({ renderProps: fakeRenderProps }), 'fr'); + const result = utils.getLanguage({ lang: 'fr' }); + assert.equal(result.lang, 'fr'); + assert.equal(result.isLangFromHeader, false); }); it('should fall-back to accept-language', () => { - const fakeRenderProps = { - params: { - lang: 'bogus', - }, - }; const acceptLanguage = 'pt-br;q=0.5,en-us;q=0.3,en;q=0.2'; - const result = utils.getFilteredUserLanguage( - { renderProps: fakeRenderProps, acceptLanguage }); - assert.equal(result, 'pt-BR'); + const result = utils.getLanguage({ lang: 'bogus', acceptLanguage }); + assert.equal(result.lang, 'pt-BR'); + assert.equal(result.isLangFromHeader, true); }); it('should map lang from accept-language too', () => { - const fakeRenderProps = { - params: { - lang: 'wat', - }, - }; const acceptLanguage = 'pt;q=0.5,en-us;q=0.3,en;q=0.2'; - const result = utils.getFilteredUserLanguage( - { renderProps: fakeRenderProps, acceptLanguage }); - assert.equal(result, 'pt-PT'); + const result = utils.getLanguage({ lang: 'wat', acceptLanguage }); + assert.equal(result.lang, 'pt-PT'); + assert.equal(result.isLangFromHeader, true); }); it('should fallback when nothing matches', () => { - const fakeRenderProps = { - params: { - lang: 'wat', - }, - }; const acceptLanguage = 'awooga;q=0.5'; - const result = utils.getFilteredUserLanguage( - { renderProps: fakeRenderProps, acceptLanguage }); - assert.equal(result, 'en-US'); + const result = utils.getLanguage({ lang: 'wat', acceptLanguage }); + assert.equal(result.lang, defaultLang); + assert.equal(result.isLangFromHeader, true); }); }); @@ -283,22 +310,22 @@ describe('i18n utils', () => { it('should find an exact language match for Punjabi', () => { const acceptLanguage = 'pa,sv;q=0.8,fi;q=0.7,it-ch;q=0.5,en-us;q=0.3,en;q=0.2'; const supportedLangs = ['af', 'en-US', 'pa']; - const result = utils.getLangFromHeader(acceptLanguage, { _validLangs: supportedLangs }); + const result = utils.getLangFromHeader(acceptLanguage, { _supportedLangs: supportedLangs }); assert.equal(result, 'pa'); }); it('should find an exact language match for Punjabi India', () => { const acceptLanguage = 'pa-in,sv;q=0.8,fi;q=0.7,it-ch;q=0.5,en-us;q=0.3,en;q=0.2'; const supportedLangs = ['af', 'en-US', 'pa']; - const result = utils.getLangFromHeader(acceptLanguage, { _validLangs: supportedLangs }); + const result = utils.getLangFromHeader(acceptLanguage, { _supportedLangs: supportedLangs }); assert.equal(result, 'pa'); }); it('should not extend into region unless exact match is found', () => { const acceptLanguage = 'pa,sv;q=0.8,fi;q=0.7,it-ch;q=0.5,en-us;q=0.3,en;q=0.2'; const supportedLangs = ['af', 'en-US', 'pa-IN']; - const result = utils.getLangFromHeader(acceptLanguage, { _validLangs: supportedLangs }); - assert.equal(result, 'en-us'); + const result = utils.getLangFromHeader(acceptLanguage, { _supportedLangs: supportedLangs }); + assert.equal(result, 'en-US'); }); it('should not match Finnish to Filipino (Philiippines)', () => { @@ -310,7 +337,7 @@ describe('i18n utils', () => { q=0.31,id;q=0.29,ru-RU;q=0.26,ru;q=0.23,de-DE;q=0.2,de; q=0.17,ko-KR;q=0.14,ko;q=0.11,es-ES;q=0.09,es;q=0.06,en-AP;q=0.0`; const supportedLangs = ['en-US', 'fi']; - const result = utils.getLangFromHeader(acceptLanguage, { _validLangs: supportedLangs }); + const result = utils.getLangFromHeader(acceptLanguage, { _supportedLangs: supportedLangs }); assert.equal(result, 'en-US'); }); @@ -323,7 +350,7 @@ describe('i18n utils', () => { q=0.31,id;q=0.29,ru-RU;q=0.26,ru;q=0.23,de-DE;q=0.2,de; q=0.17,ko-KR;q=0.14,ko;q=0.11,es-ES;q=0.09,es;q=0.06,en-AP;q=0.0`; const supportedLangs = ['en-US', 'fi', 'fil-PH']; - const result = utils.getLangFromHeader(acceptLanguage, { _validLangs: supportedLangs }); + const result = utils.getLangFromHeader(acceptLanguage, { _supportedLangs: supportedLangs }); assert.equal(result, 'fil-PH'); }); @@ -336,14 +363,14 @@ describe('i18n utils', () => { q=0.31,id;q=0.29,ru-RU;q=0.26,ru;q=0.23,de-DE;q=0.2,de; q=0.17,ko-KR;q=0.14,ko;q=0.11,es-ES;q=0.09,es;q=0.06,en-AP;q=0.0`; const supportedLangs = ['en-US', 'fi', 'fil']; - const result = utils.getLangFromHeader(acceptLanguage, { _validLangs: supportedLangs }); + const result = utils.getLangFromHeader(acceptLanguage, { _supportedLangs: supportedLangs }); assert.equal(result, 'fil'); }); it('should return undefined language for no match', () => { const acceptLanguage = 'whatever'; const supportedLangs = ['af', 'en-US', 'pa']; - const result = utils.getLangFromHeader(acceptLanguage, { _validLangs: supportedLangs }); + const result = utils.getLangFromHeader(acceptLanguage, { _supportedLangs: supportedLangs }); assert.equal(result, undefined); }); diff --git a/tests/client/core/test_middleware.js b/tests/client/core/test_middleware.js new file mode 100644 index 00000000000..f3cad7e1b5c --- /dev/null +++ b/tests/client/core/test_middleware.js @@ -0,0 +1,121 @@ +import { prefixMiddleWare } from 'core/middleware'; + +describe('Prefix Middleware', () => { + let fakeRes; + let fakeNext; + let fakeConfig; + + beforeEach(() => { + fakeNext = sinon.stub(); + fakeRes = { + locals: {}, + redirect: sinon.stub(), + set: sinon.stub(), + }; + fakeConfig = new Map(); + fakeConfig.set('validClientApplications', ['firefox', 'android']); + }); + + it('should call res.redirect if changing the case', () => { + const fakeReq = { + originalUrl: '/en-us/firefox', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/en-US/firefox']); + assert.notOk(fakeNext.called); + }); + + it('should call res.redirect if handed a locale insted of a lang', () => { + const fakeReq = { + originalUrl: '/en_US/firefox', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/en-US/firefox']); + assert.notOk(fakeNext.called); + }); + + it('should add an application when missing', () => { + const fakeReq = { + originalUrl: '/en-US/whatever/', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/en-US/firefox/whatever/']); + }); + + it('should prepend a lang when missing but leave a valid app intact', () => { + const fakeReq = { + originalUrl: '/firefox/whatever', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/en-US/firefox/whatever']); + assert.deepEqual(fakeRes.set.firstCall.args, ['vary', []]); + }); + + it('should fallback to and vary on accept-language headers', () => { + const fakeReq = { + originalUrl: '/firefox/whatever', + headers: { + 'accept-language': 'pt-br;q=0.5,en-us;q=0.3,en;q=0.2', + }, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/pt-BR/firefox/whatever']); + assert.sameMembers(fakeRes.set.firstCall.args[1], ['accept-language']); + }); + + it('should map aliased langs', () => { + const fakeReq = { + originalUrl: '/pt/firefox/whatever', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/pt-PT/firefox/whatever']); + }); + + it('should vary on accept-language and user-agent', () => { + const fakeReq = { + originalUrl: '/whatever', + headers: { + 'accept-language': 'pt-br;q=0.5,en-us;q=0.3,en;q=0.2', + }, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/pt-BR/firefox/whatever']); + assert.sameMembers(fakeRes.set.firstCall.args[1], ['accept-language', 'user-agent']); + }); + + it('should populate res.locals for a sane request', () => { + const fakeReq = { + originalUrl: '/en-US/firefox/', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.equal(fakeRes.locals.lang, 'en-US'); + assert.equal(fakeRes.locals.clientApp, 'firefox'); + assert.notOk(fakeRes.redirect.called); + }); + + it('should not populate res.locals for a redirection', () => { + const fakeReq = { + originalUrl: '/foo/bar', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.equal(fakeRes.locals.lang, undefined); + assert.equal(fakeRes.locals.clientApp, undefined); + assert.ok(fakeRes.redirect.called); + }); + + it('should not mangle a query string for a redirect', () => { + const fakeReq = { + originalUrl: '/foo/bar?test=1&bar=2', + headers: {}, + }; + prefixMiddleWare(fakeReq, fakeRes, fakeNext, { _config: fakeConfig }); + assert.deepEqual(fakeRes.redirect.firstCall.args, [302, '/en-US/firefox/foo/bar?test=1&bar=2']); + }); +}); diff --git a/tests/client/core/test_tracking.js b/tests/client/core/test_tracking.js index 8479ca73b8b..f580127aa02 100644 --- a/tests/client/core/test_tracking.js +++ b/tests/client/core/test_tracking.js @@ -23,7 +23,6 @@ describe('Tracking', () => { info: sinon.stub(), }, }); - // eslint-disable-next-line no-underscore-dangle assert.ok(tracking._log.info.calledWith(sinon.match(/OFF/), 'Tracking init')); }); @@ -35,7 +34,6 @@ describe('Tracking', () => { info: sinon.stub(), }, }); - // eslint-disable-next-line no-underscore-dangle assert.ok(tracking._log.info.secondCall.calledWith(sinon.match(/OFF/), 'Missing tracking id')); }); diff --git a/tests/client/core/test_utils.js b/tests/client/core/test_utils.js index e32be89e267..f18d339a3b3 100644 --- a/tests/client/core/test_utils.js +++ b/tests/client/core/test_utils.js @@ -1,4 +1,10 @@ -import { camelCaseProps, convertBoolean, getClientConfig } from 'core/utils'; +import { + camelCaseProps, + convertBoolean, + getClientApp, + getClientConfig, + isValidClientApp, +} from 'core/utils'; describe('camelCaseProps', () => { const input = { @@ -72,3 +78,28 @@ describe('convertBoolean', () => { assert.equal(convertBoolean('whatevs'), false); }); }); + + +describe('getClientApplication', () => { + it('should return firefox', () => { + assert.equal(getClientApp(), 'firefox'); + }); +}); + + +describe('isValidClientApp', () => { + const _config = new Map(); + _config.set('validClientApplications', ['firefox', 'android']); + + it('should be valid if passed "firefox"', () => { + assert.equal(isValidClientApp('firefox', { _config }), true); + }); + + it('should be valid if passed "android"', () => { + assert.equal(isValidClientApp('android', { _config }), true); + }); + + it('should be invalid if passed "whatever"', () => { + assert.equal(isValidClientApp('whatever', { _config }), false); + }); +}); diff --git a/tests/server/amo/TestViews.js b/tests/server/amo/TestViews.js new file mode 100644 index 00000000000..b0059e49543 --- /dev/null +++ b/tests/server/amo/TestViews.js @@ -0,0 +1,52 @@ +/* eslint-disable no-loop-func */ + +import request from 'supertest-as-promised'; +import { assert } from 'chai'; + +import { runServer } from 'core/server/base'; +import Policy from 'csp-parse'; + +import { checkSRI } from '../helpers'; + +const defaultURL = '/en-US/firefox/'; + +describe('AMO GET Requests', () => { + let app; + + before(() => runServer({ listen: false, app: 'amo' }) + .then((server) => { + app = server; + })); + + after(() => { + webpackIsomorphicTools.undo(); + }); + + it('should have a CSP policy on the amo app homepage', () => request(app) + .get(defaultURL) + .expect(200) + .then((res) => { + const policy = new Policy(res.header['content-security-policy']); + assert.notInclude(policy.get('script-src'), "'self'"); + assert.include(policy.get('script-src'), 'https://addons.cdn.mozilla.net'); + assert.notInclude(policy.get('connect-src'), "'self'"); + assert.include(policy.get('connect-src'), 'https://addons.mozilla.org'); + })); + + it('should be using SRI for script and style on the amo app homepage', () => request(app) + .get(defaultURL) + .expect(200) + .then((res) => checkSRI(res))); + + it('should be a 200 for requests to the homepage directly', () => request(app) + .get('/en-US/firefox/') + .expect(200)); + + it('should redirect an invalid locale', () => request(app) + .get('/whatever/firefox/') + .expect(302) + .then((res) => { + assert.equal(res.header.location, + '/en-US/firefox/'); + })); +}); diff --git a/tests/server/disco/TestViews.js b/tests/server/disco/TestViews.js index 6a93eaa0145..b53078b695a 100644 --- a/tests/server/disco/TestViews.js +++ b/tests/server/disco/TestViews.js @@ -22,7 +22,7 @@ describe('Discovery Pane GET requests', () => { webpackIsomorphicTools.undo(); }); - it('should have a CSP policy for / on the disco app', () => request(app) + it('should have a CSP policy on the disco app', () => request(app) .get(defaultURL) .expect(200) .then((res) => { @@ -33,17 +33,17 @@ describe('Discovery Pane GET requests', () => { assert.include(policy.get('connect-src'), 'https://addons.mozilla.org'); })); - it('should be using SRI for script and style in /', () => request(app) + it('should be using SRI for script and style', () => request(app) .get(defaultURL) .expect(200) .then((res) => checkSRI(res))); - it('should be a 404 for requests to /', () => request(app) - .get('/') + it('should be a 404 for requests to /en-US/firefox', () => request(app) + .get('/en-US/firefox') .expect(404)); - it('should be a 404 for requests to /en-US/', () => request(app) - .get('/en-US/') + it('should be a 404 for requests to /en-US/firefox/', () => request(app) + .get('/en-US/firefox/') .expect(404)); it('should redirect an invalid locale', () => request(app) @@ -54,54 +54,6 @@ describe('Discovery Pane GET requests', () => { '/en-US/firefox/discovery/pane/48.0/Darwin/normal'); })); - it('should redirect an invalid locale which will be encoded', () => request(app) - .get('/