fix(lambda-at-edge): use 307 redirect for root -> locale URI instead … (#886)

Author: dphang

packages/libs/lambda-at-edge/src/default-handler.ts

@@ -26,18 +26,19 @@ import type { Readable } from "stream";
 import {
   createRedirectResponse,
   getDomainRedirectPath,
+  getLanguageRedirect,
   getRedirectPath
 } from "./routing/redirector";
 import {
   createExternalRewriteResponse,
-  getLanguageRewrite,
   getRewritePath,
   isExternalRewrite
 } from "./routing/rewriter";
 import { addHeadersToResponse } from "./headers/addHeaders";
 import { isValidPreviewRequest } from "./lib/isValidPreviewRequest";
 import { getUnauthenticatedResponse } from "./auth/authenticator";
 import { buildS3RetryStrategy } from "./s3/s3RetryStrategy";
+import { isLocaleIndexUri } from "./routing/locale-utils";
 
 const basePath = RoutesManifestJson.basePath;
 
@@ -374,16 +375,19 @@ const handleOriginRequest = async ({
 
   // Handle root language rewrite
   const languageHeader = request.headers["accept-language"];
-  const languageRewriteUri = getLanguageRewrite(
+  const languageRedirectUri = getLanguageRedirect(
     languageHeader ? languageHeader[0].value : undefined,
     uri,
-    routesManifest
+    routesManifest,
+    manifest
   );
 
-  if (languageRewriteUri) {
-    request.uri = languageRewriteUri;
-
-    uri = normaliseUri(request.uri);
+  if (languageRedirectUri) {
+    return createRedirectResponse(
+      languageRedirectUri,
+      request.querystring,
+      307
+    );
   }
 
   const isStaticPage = pages.html.nonDynamic[uri]; // plain page without any props
@@ -417,7 +421,7 @@ const handleOriginRequest = async ({
     } else if (isHTMLPage || hasFallback) {
       s3Origin.path = `${basePath}/static-pages/${manifest.buildId}`;
       let pageName;
-      if (languageRewriteUri) {
+      if (isLocaleIndexUri(uri, routesManifest)) {
         pageName = `${uri}/index`;
       } else {
         pageName = uri === "/" ? "/index" : uri;

packages/libs/lambda-at-edge/src/headers/addHeaders.ts

@@ -1,7 +1,7 @@
 import { CloudFrontResultResponse } from "aws-lambda";
 import { RoutesManifest } from "../types";
 import { matchPath } from "../routing/matcher";
-import { addDefaultLocaleToPath } from "../routing/common-utils";
+import { addDefaultLocaleToPath } from "../routing/locale-utils";
 
 export function addHeadersToResponse(
   path: string,

packages/libs/lambda-at-edge/src/routing/common-utils.ts renamed to packages/libs/lambda-at-edge/src/routing/locale-utils.ts

@@ -29,3 +29,18 @@ export function addDefaultLocaleToPath(
 
   return path;
 }
+
+export function isLocaleIndexUri(
+  normalisedUri: string,
+  routesManifest: RoutesManifest
+) {
+  if (routesManifest.i18n) {
+    for (const locale of routesManifest.i18n.locales) {
+      if (normalisedUri === `/${locale}`) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}

packages/libs/lambda-at-edge/src/routing/redirector.ts

@@ -9,7 +9,8 @@ import {
 import * as http from "http";
 import { CloudFrontRequest } from "aws-lambda";
 import { CloudFrontResultResponse } from "aws-lambda";
-import { addDefaultLocaleToPath } from "./common-utils";
+import { addDefaultLocaleToPath } from "./locale-utils";
+import Accept from "@hapi/accept";
 
 /**
  * Whether this is the default trailing slash redirect.
@@ -176,3 +177,41 @@ export function getDomainRedirectPath(
   }
   return null;
 }
+
+export function getLanguageRedirect(
+  acceptLanguageHeader: string | undefined,
+  normalisedUri: string,
+  routesManifest: RoutesManifest,
+  manifest: OriginRequestDefaultHandlerManifest
+): string | undefined {
+  // Only redirect root page and if accept language is defined
+  if (normalisedUri !== "/" || !acceptLanguageHeader) {
+    return undefined;
+  }
+
+  if (routesManifest.i18n) {
+    const locales = routesManifest.i18n.locales;
+    const defaultLocale = routesManifest.i18n.defaultLocale;
+    const basePath = routesManifest.basePath;
+
+    const preferredLanguage = Accept.language(acceptLanguageHeader);
+
+    let localeToUse = undefined;
+
+    // Find language in locale that matches preferred language
+    for (const locale of locales) {
+      if (preferredLanguage === locale) {
+        localeToUse = locale;
+        break;
+      }
+    }
+
+    if (!localeToUse || localeToUse === defaultLocale) {
+      return undefined;
+    } else {
+      return `${basePath}/${localeToUse}${manifest.trailingSlash ? "/" : ""}`;
+    }
+  }
+
+  return undefined;
+}

packages/libs/lambda-at-edge/src/routing/rewriter.ts

@@ -1,8 +1,7 @@
 import { compileDestination, matchPath } from "./matcher";
 import { RewriteData, RoutesManifest } from "../types";
 import { IncomingMessage, ServerResponse } from "http";
-import Accept from "@hapi/accept";
-import { addDefaultLocaleToPath } from "./common-utils";
+import { addDefaultLocaleToPath } from "./locale-utils";
 
 /**
  * Get the rewrite of the given path, if it exists. Otherwise return null.
@@ -129,40 +128,3 @@ export async function createExternalRewriteResponse(
   res.statusCode = fetchResponse.status;
   res.end(await fetchResponse.buffer());
 }
-
-export function getLanguageRewrite(
-  acceptLanguageHeader: string | undefined,
-  normalisedUri: string,
-  routesManifest: RoutesManifest
-): string | undefined {
-  // Only rewrites root page and if accept language is defined
-  if (normalisedUri !== "/" || !acceptLanguageHeader) {
-    return undefined;
-  }
-
-  if (routesManifest.i18n) {
-    const locales = routesManifest.i18n.locales;
-    const defaultLocale = routesManifest.i18n.defaultLocale;
-    const basePath = routesManifest.basePath;
-
-    const preferredLanguage = Accept.language(acceptLanguageHeader);
-
-    let localeToUse = undefined;
-
-    // Find language in locale that matches preferred language
-    for (const locale of locales) {
-      if (preferredLanguage === locale) {
-        localeToUse = locale;
-        break;
-      }
-    }
-
-    if (!localeToUse || localeToUse === defaultLocale) {
-      return undefined;
-    } else {
-      return `${basePath}/${localeToUse}`;
-    }
-  }
-
-  return undefined;
-}

packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest-with-trailing-slash.json

@@ -156,8 +156,10 @@
       "nonDynamic": {
         "/": "pages/index.html",
         "/terms": "pages/terms.html",
-        "/nl": "pages/index.html",
-        "/nl/terms": "pages/terms.html",
+        "/en": "pages/en/index.html",
+        "/en/terms": "pages/en/terms.html",
+        "/nl": "pages/nl/index.html",
+        "/nl/terms": "pages/nl/terms.html",
         "/fr": "pages/fr/index.html",
         "/fr/terms": "pages/fr/terms.html"
       },

packages/libs/lambda-at-edge/tests/default-handler/default-build-manifest.json

@@ -156,8 +156,10 @@
       "nonDynamic": {
         "/": "pages/index.html",
         "/terms": "pages/terms.html",
-        "/nl": "pages/index.html",
-        "/nl/terms": "pages/terms.html",
+        "/en": "pages/en/index.html",
+        "/en/terms": "pages/en/terms.html",
+        "/nl": "pages/nl/index.html",
+        "/nl/terms": "pages/nl/terms.html",
         "/fr": "pages/fr/index.html",
         "/fr/terms": "pages/fr/terms.html"
       },

packages/libs/lambda-at-edge/tests/default-handler/default-handler-with-basepath.test.ts

@@ -45,7 +45,8 @@ describe("Lambda@Edge", () => {
       expectedRedirect: string,
       statusCode: number,
       querystring?: string,
-      host?: string
+      host?: string,
+      requestHeaders?: { [p: string]: { key: string; value: string }[] }
     ) => Promise<void>;
     beforeEach(() => {
       jest.resetModules();
@@ -96,15 +97,17 @@ describe("Lambda@Edge", () => {
         expectedRedirect: string,
         statusCode: number,
         querystring?: string,
-        host?: string
+        host?: string,
+        requestHeaders?: { [p: string]: { key: string; value: string }[] }
       ): Promise<void> => {
         await runRedirectTestWithHandler(
           handler,
           path,
           expectedRedirect,
           statusCode,
           querystring,
-          host
+          host,
+          requestHeaders
         );
       };
     });
@@ -757,48 +760,70 @@ describe("Lambda@Edge", () => {
       );
     });
 
-    describe("Root Language Rewrite", () => {
+    describe("Root Locale Redirect", () => {
       it.each`
-        acceptLanguageHeader   | expectedPage
-        ${"nl"}                | ${"/nl/index.html"}
-        ${"en"}                | ${"/index.html"}
-        ${undefined}           | ${"/index.html"}
-        ${"fr,nl,en"}          | ${"/fr/index.html"}
-        ${"nl,fr"}             | ${"/nl/index.html"}
-        ${"fr,nl"}             | ${"/fr/index.html"}
-        ${"en,nl"}             | ${"/index.html"}
-        ${"en;q=1.0,nl;q=0.8"} | ${"/index.html"}
-        ${"en;q=0.5,nl;q=0.8"} | ${"/nl/index.html"}
+        acceptLanguageHeader   | expectedRedirect
+        ${"nl"}                | ${"/basepath/nl"}
+        ${"fr,nl,en"}          | ${"/basepath/fr"}
+        ${"nl,fr"}             | ${"/basepath/nl"}
+        ${"fr,nl"}             | ${"/basepath/fr"}
+        ${"en;q=0.5,nl;q=0.8"} | ${"/basepath/nl"}
       `(
-        "rewrites / with accept-language [$acceptLanguageHeader] to $expectedPage",
-        async ({ acceptLanguageHeader, expectedPage }) => {
-          const event = createCloudFrontEvent({
-            uri: trailingSlash ? "/basepath/" : "/basepath",
-            host: "mydistribution.cloudfront.net",
-            requestHeaders: {
+        "redirects path /basepath with accept-language [$acceptLanguageHeader] to $expectedRedirect",
+        async ({ acceptLanguageHeader, expectedRedirect }) => {
+          if (trailingSlash) {
+            expectedRedirect += "/";
+          }
+
+          await runRedirectTest(
+            trailingSlash ? "/basepath/" : "/basepath",
+            expectedRedirect,
+            307,
+            undefined,
+            undefined,
+            {
               "accept-language": [
                 { key: "Accept-Language", value: acceptLanguageHeader }
               ]
             }
-          });
-
-          const request = await handler(event);
-
-          expect(request.origin).toEqual({
-            s3: {
-              authMethod: "origin-access-identity",
-              domainName: "my-bucket.s3.amazonaws.com",
-              path: "/basepath/static-pages/build-id",
-              region: "us-east-1"
-            }
-          });
-          expect(request.uri).toEqual(expectedPage);
-          expect(request.headers.host[0].key).toEqual("host");
-          expect(request.headers.host[0].value).toEqual(
-            "my-bucket.s3.amazonaws.com"
           );
         }
       );
     });
+
+    describe("Locale Pages", () => {
+      it.each`
+        path              | expectedPage
+        ${"/basepath"}    | ${"/index.html"}
+        ${"/basepath/en"} | ${"/en/index.html"}
+        ${"/basepath/fr"} | ${"/fr/index.html"}
+        ${"/basepath/nl"} | ${"/nl/index.html"}
+      `("path $path renders $expectedPage", async ({ path, expectedPage }) => {
+        if (trailingSlash && !path.endsWith("/")) {
+          path += "/";
+        }
+
+        const event = createCloudFrontEvent({
+          uri: path,
+          host: "mydistribution.cloudfront.net"
+        });
+
+        const request = await handler(event);
+
+        expect(request.origin).toEqual({
+          s3: {
+            authMethod: "origin-access-identity",
+            domainName: "my-bucket.s3.amazonaws.com",
+            path: "/basepath/static-pages/build-id",
+            region: "us-east-1"
+          }
+        });
+        expect(request.uri).toEqual(expectedPage);
+        expect(request.headers.host[0].key).toEqual("host");
+        expect(request.headers.host[0].value).toEqual(
+          "my-bucket.s3.amazonaws.com"
+        );
+      });
+    });
   });
 });