Add application redirects + vary headers

config/default-search.js

@@ -19,5 +19,5 @@ module.exports = {
     },
   },
 
-  redirectLangPrefix: false,
+  enablePrefixMiddleware: false,
 };

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',
 };

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": {

src/amo/containers/DetailPage.js

@@ -0,0 +1,11 @@
+import React from 'react';
+
+export default class DetailPage extends React.Component {
+  render() {
+    return (
+      <div>
+        <h1>Detail Page</h1>
+      </div>
+    );
+  }
+}

src/amo/css/App.scss

@@ -4,5 +4,5 @@
 
 html,
 body {
-  background: red;
+  background: #fff;
 }

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 (
-  <Route path="/(:lang/)" component={App}>
+  <Route path="/:lang/:application" component={App}>
     <IndexRoute component={Home} />
+    <Route path="addon/:slug/" component={DetailPage} />
   </Route>
 );

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);
 });
 

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 };
 }

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();
+}

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 = (

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);
+}