static: Fix last-modified headers, move index handler to static

`serverless-esbuild`, which we use to build the project, doesn't preserve file
modification times. We make use of these times to generate the `last-modified`
headers for static files, which we bundle in the Lambda `.zip`s.  I [submitted a
PR upstream][pr] to fix this. Switch to using a fork containing this fix.

The `index` handler was implementing very similar logic to the `static` handler,
but slightly differently. We move the `index` handler to the `static` module and
make it use the same `fileData` as the `static` handler. This allows us to
remove the `index` handler entirely.

Add tests for both handlers. This uses a test file stored in the repo. But note
that `git` doesn't preserve file modification times, so we have to work these
out dynamically in the tests.

Fix a nit too: handlers don't have to be async. They can simply return their
result directly, no need to wrap it in a `Promise.resolve`.

[pr]: floydspace/serverless-esbuild#539

app/src/handlers/static/fileinfo.test.ts

@@ -0,0 +1,27 @@
+import { describe, expect, it } from "@jest/globals";
+import { createHash } from "crypto";
+import path from "path";
+import { fileURLToPath } from "url";
+
+import { precomputeFileData } from "./fileinfo";
+
+describe("precomputeFileData", () => {
+  it("returns correct file data", async () => {
+    const __dirname = path.dirname(fileURLToPath(import.meta.url));
+    const staticFilesDir = path.join(__dirname, "..", "..", "testdata");
+    const fileData = await precomputeFileData(staticFilesDir);
+
+    const text = "test\n";
+    const sha256 = createHash("sha256").update(text).digest("hex");
+
+    expect(fileData).toEqual({
+      "test.txt": {
+        buffer: Buffer.from(text),
+        contentType: "text/plain; charset=utf-8",
+        etag: `"${sha256}"`,
+        lastModified: expect.anything(),
+        size: 5,
+      },
+    });
+  });
+});

app/src/handlers/static/fileinfo.ts

@@ -0,0 +1,59 @@
+import { createHash } from "crypto";
+import { readFile, readdir, stat } from "fs/promises";
+import { contentType } from "mime-types";
+import * as path from "path";
+import { fileURLToPath } from "url";
+
+export interface staticFileInfo {
+  buffer: Buffer;
+  etag: string;
+  lastModified: Date;
+  contentType: string;
+  size: number;
+}
+
+/**
+ * Precompute file data for a directory of static files. This is useful to avoid
+ * reading files on every request.
+ *
+ * @param dir Directory to read files from
+ * @returns Object containing file data
+ */
+export async function precomputeFileData(dir: string) {
+  const fileInfo: { [path: string]: staticFileInfo } = {};
+
+  const files = await readdir(dir, { withFileTypes: true });
+
+  await Promise.all(
+    files.map(async (dirent) => {
+      const dir = dirent.parentPath;
+      const filePath = path.join(dir, dirent.name);
+      const fileBuffer = await readFile(filePath);
+      const hashSum = createHash("sha256").update(fileBuffer).digest("hex");
+
+      const stats = await stat(filePath);
+      const lastModified = stats.mtime;
+      // HTTP timestamps can't have milliseconds
+      lastModified.setMilliseconds(0);
+
+      const fileContentType =
+        contentType(path.extname(dirent.name)) || "application/octet-stream";
+
+      fileInfo[dirent.name] = {
+        buffer: fileBuffer,
+        contentType: fileContentType,
+        etag: `"${hashSum}"`,
+        lastModified: lastModified,
+        size: stats.size,
+      };
+    }),
+  );
+
+  return fileInfo;
+}
+
+// Read all files from `/static` and precompute their metadata.
+const __dirname = path.dirname(fileURLToPath(import.meta.url));
+const staticFilesDir = path.resolve(__dirname, "../../../static");
+
+export const staticFileData = await precomputeFileData(staticFilesDir);

app/src/handlers/static/index.ts

@@ -1,128 +1,6 @@
-import type {
-  APIGatewayProxyEventV2,
-  APIGatewayProxyResultV2,
-} from "aws-lambda";
-import { createHash } from "crypto";
-import { readFile, readdir, stat } from "fs/promises";
-import { StatusCodes } from "http-status-codes";
-import { contentType } from "mime-types";
-import * as path from "path";
-import { fileURLToPath } from "url";
+import { staticFileData } from "./fileinfo";
+import { staticHandlerFactory } from "./static";
 
-import { handlerFactory } from "@/lib/handler-factory";
-import { LoggerContext } from "@/lib/logger";
+export const staticHandler = staticHandlerFactory(staticFileData);
 
-const { NOT_FOUND, NOT_MODIFIED, OK } = StatusCodes;
-
-const __dirname = path.dirname(fileURLToPath(import.meta.url));
-const staticFilesDir = path.resolve(__dirname, "../../../static");
-
-// Read all files from `/static` and precompute their metadata.
-const fileData = await (async () => {
-  const fileInfo: {
-    [key: string]: {
-      buffer: Buffer;
-      etag: string;
-      lastModified: Date;
-      contentType: string;
-      size: number;
-    };
-  } = {};
-
-  const files = await readdir(staticFilesDir);
-
-  await Promise.all(
-    files.map(async (file) => {
-      const filePath = path.join(staticFilesDir, file);
-      const fileBuffer = await readFile(filePath);
-      const hashSum = createHash("sha256");
-      hashSum.update(fileBuffer);
-      const hex = hashSum.digest("hex");
-
-      const stats = await stat(filePath);
-      const lastModified = stats.mtime;
-
-      const fileContentType =
-        contentType(path.extname(file)) || "application/octet-stream";
-
-      fileInfo[file] = {
-        buffer: fileBuffer,
-        contentType: fileContentType,
-        etag: hex,
-        lastModified: lastModified,
-        size: stats.size,
-      };
-    }),
-  );
-
-  return fileInfo;
-})();
-
-const notFoundFiles = new Set(["favicon.ico"]);
-
-function staticFileHandler(
-  event: APIGatewayProxyEventV2,
-  { logger }: LoggerContext,
-): Promise<APIGatewayProxyResultV2> {
-  const p = path.relative("/", event.requestContext.http.path);
-  const fd = fileData[p];
-
-  const log = logger.createChild({
-    persistentLogAttributes: {
-      handler: "static",
-      path: p,
-    },
-  });
-
-  if (!fd) {
-    return Promise.resolve({
-      statusCode: NOT_FOUND,
-      body: `File ${p} was not found.\n`,
-      ...(notFoundFiles.has(p)
-        ? {
-            headers: {
-              "cache-control": "public, max-age=3600",
-            },
-          }
-        : {}),
-    });
-  }
-
-  const { buffer, contentType, etag, lastModified, size } = fd;
-
-  const ifNoneMatch = event.headers["If-None-Match"];
-  const ifModifiedSince = event.headers["If-Modified-Since"];
-
-  const cacheHeaders = {
-    "cache-control": "public, s-maxage=60",
-    ETag: `"${etag}"`,
-    "last-modified": lastModified.toUTCString(),
-  };
-
-  if (
-    ifNoneMatch === etag ||
-    (ifModifiedSince && new Date(ifModifiedSince) >= lastModified)
-  ) {
-    log.debug("Not modified, sending 304");
-
-    return Promise.resolve({
-      statusCode: NOT_MODIFIED,
-      headers: cacheHeaders,
-    });
-  }
-
-  log.debug("Sending file");
-
-  return Promise.resolve({
-    statusCode: OK,
-    headers: {
-      "content-length": size,
-      "content-type": contentType,
-      ...cacheHeaders,
-    },
-    body: buffer.toString("base64"),
-    isBase64Encoded: true,
-  });
-}
-
-export const staticHandler = handlerFactory(staticFileHandler);
+export { indexHandler } from "./indexhandler";

app/src/handlers/static/indexhandler.test.ts

@@ -0,0 +1,137 @@
+import { describe, expect, it } from "@jest/globals";
+import { Event } from "@middy/http-header-normalizer";
+import { APIGatewayProxyEventV2 } from "aws-lambda";
+import { StatusCodes } from "http-status-codes";
+import { mock } from "jest-mock-extended";
+
+import { GeoCodeContext } from "@/lib/geocode";
+import { GeoLocateContext } from "@/lib/geolocate";
+import { LoggerContext } from "@/lib/logger";
+
+import { indexHandler } from ".";
+
+const mockContext = mock<GeoCodeContext & GeoLocateContext & LoggerContext>();
+
+describe("index (/) handler", () => {
+  it.each<{
+    description: string;
+    acceptHeader: string;
+    path: string;
+    queryString?: string;
+    expectedStatus: StatusCodes;
+    expectedHeaders?: { [key: string]: string };
+  }>([
+    {
+      description: "redirects to /:unknown if client doesn't accept HTML",
+      acceptHeader: "text/plain",
+      path: "/",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+      expectedHeaders: {
+        "cache-control": "public, max-age=3600",
+        location: "/:unknown",
+      },
+    },
+    {
+      description: "preserves query string when redirecting",
+      acceptHeader: "text/plain",
+      path: "/",
+      queryString: "foo=bar",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+      expectedHeaders: {
+        "cache-control": "public, max-age=3600",
+        location: "/:unknown?foo=bar",
+      },
+    },
+    {
+      description: "redirects when accept header is */*",
+      acceptHeader: "*/*",
+      path: "/",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+    },
+    {
+      description: "returns index.html if client explicitly accepts HTML",
+      acceptHeader: "text/html",
+      path: "/",
+      expectedStatus: StatusCodes.OK,
+      expectedHeaders: {
+        "cache-control": "public, s-maxage=60",
+      },
+    },
+    {
+      description: "preserves the prefix when redirecting",
+      acceptHeader: "text/plain",
+      path: "/foo",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+      expectedHeaders: {
+        location: "/foo/:unknown",
+      },
+    },
+    {
+      description: "preserves the prefix when redirecting, with query string",
+      acceptHeader: "text/plain",
+      path: "/foo",
+      queryString: "bar=baz",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+      expectedHeaders: {
+        location: "/foo/:unknown?bar=baz",
+      },
+    },
+    {
+      description: "preserves the prefix when redirecting (trailing slash)",
+      acceptHeader: "text/plain",
+      path: "/foo/",
+      queryString: "bar=baz",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+      expectedHeaders: {
+        location: "/foo/:unknown?bar=baz",
+      },
+    },
+    {
+      description: "handles a complex accept header",
+      acceptHeader:
+        "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7",
+      path: "/",
+      expectedStatus: StatusCodes.OK,
+    },
+    {
+      description: "redirects if the client prefers text/plain over text/html",
+      acceptHeader: "text/plain, text/html",
+      path: "/",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+    },
+    {
+      description:
+        "redirects if the client prefers text/plain over text/html, using quality values",
+      acceptHeader: "text/html;q=0.5, text/plain;q=0.8",
+      path: "/",
+      expectedStatus: StatusCodes.TEMPORARY_REDIRECT,
+    },
+  ])(
+    "$description",
+    async ({
+      acceptHeader,
+      path,
+      queryString,
+      expectedStatus,
+      expectedHeaders,
+    }) => {
+      const event = mock<APIGatewayProxyEventV2 & Event>({
+        rawPath: path,
+        rawQueryString: queryString ?? "",
+        headers: {
+          accept: acceptHeader,
+        },
+      });
+
+      await expect(indexHandler(event, mockContext)).resolves.toEqual(
+        expect.objectContaining({
+          headers: expect.objectContaining(expectedHeaders ?? {}),
+          ...(expectedStatus === StatusCodes.OK && {
+            body: expect.anything(),
+          }),
+          statusCode: expectedStatus,
+        }),
+      );
+    },
+  );
+});