Merge pull request #554 from trails-cool/fix-journal-malformed-url-crash

fix(journal): don't crash the process on a malformed request URL

apps/journal/serve-static.test.ts

@@ -0,0 +1,35 @@
+import { describe, it, expect, vi } from "vitest";
+import type { IncomingMessage, ServerResponse } from "node:http";
+import { serveStatic } from "./serve-static.ts";
+
+function mockReq(url: string, method = "GET"): IncomingMessage {
+  return { url, method, headers: { host: "trails.cool" } } as unknown as IncomingMessage;
+}
+
+function mockRes(): ServerResponse {
+  return { setHeader: vi.fn() } as unknown as ServerResponse;
+}
+
+describe("serveStatic", () => {
+  // Regression: a malformed path makes `new URL` throw ERR_INVALID_URL.
+  // Because serveStatic runs synchronously inside the HTTP server
+  // callback, an unhandled throw is process-fatal — a single `GET //`
+  // crash-looped the journal in prod. It must fall through (return false)
+  // so the request reaches React Router and 404s instead.
+  it.each(["//", "///", "/\\", "//foo\\bar"])(
+    "returns false without throwing for malformed path %j",
+    (path) => {
+      const req = mockReq(path);
+      expect(() => serveStatic(req, mockRes())).not.toThrow();
+      expect(serveStatic(req, mockRes())).toBe(false);
+    },
+  );
+
+  it("falls through for non-GET/HEAD methods", () => {
+    expect(serveStatic(mockReq("/assets/app.js", "POST"), mockRes())).toBe(false);
+  });
+
+  it("falls through for a well-formed path with no matching file", () => {
+    expect(serveStatic(mockReq("/does/not/exist.js"), mockRes())).toBe(false);
+  });
+});

apps/journal/serve-static.ts

@@ -0,0 +1,55 @@
+import type { IncomingMessage, ServerResponse } from "node:http";
+import { createReadStream, statSync } from "node:fs";
+import { join, extname, resolve } from "node:path";
+
+const CLIENT_DIR = resolve(import.meta.dirname, "build", "client");
+
+const MIME: Record<string, string> = {
+  ".js": "application/javascript",
+  ".css": "text/css",
+  ".html": "text/html",
+  ".json": "application/json",
+  ".png": "image/png",
+  ".jpg": "image/jpeg",
+  ".svg": "image/svg+xml",
+  ".ico": "image/x-icon",
+  ".woff": "font/woff",
+  ".woff2": "font/woff2",
+};
+
+/**
+ * Serve a static asset from the client build. Returns true if the request
+ * was handled, false if it should fall through to the app request handler.
+ *
+ * Runs synchronously inside the HTTP server callback, so it must never
+ * throw: a thrown error here is an uncaught exception that crashes the
+ * whole process. Malformed request paths (e.g. `//`, `///`, `/\`) make
+ * `new URL` throw `ERR_INVALID_URL`; we catch that and fall through so
+ * React Router 404s the request instead of taking the server down.
+ */
+export function serveStatic(req: IncomingMessage, res: ServerResponse): boolean {
+  if (req.method !== "GET" && req.method !== "HEAD") return false;
+
+  let url: URL;
+  try {
+    url = new URL(req.url ?? "/", `http://${req.headers.host}`);
+  } catch {
+    return false;
+  }
+  const filePath = resolve(join(CLIENT_DIR, url.pathname));
+
+  if (!filePath.startsWith(CLIENT_DIR)) return false;
+
+  try {
+    if (!statSync(filePath).isFile()) return false;
+  } catch {
+    return false;
+  }
+
+  res.setHeader("Content-Type", MIME[extname(filePath)] ?? "application/octet-stream");
+  if (url.pathname.startsWith("/assets/")) {
+    res.setHeader("Cache-Control", "public, immutable, max-age=31536000");
+  }
+  createReadStream(filePath).pipe(res);
+  return true;
+}

apps/journal/server.ts

@@ -2,8 +2,7 @@ import * as Sentry from "@sentry/node";
 import { nodeSentryConfig, drop404s } from "@trails-cool/sentry-config";
 import { createRequestListener } from "@react-router/node";
 import { createServer, type IncomingMessage, type ServerResponse } from "node:http";
-import { createReadStream, statSync } from "node:fs";
-import { join, extname, resolve } from "node:path";
+import { serveStatic } from "./serve-static.ts";
 import { logger, requestContext } from "./app/lib/logger.server.ts";
 import { randomUUID } from "node:crypto";
 import { httpRequestDuration, normalizeRoute, registry } from "./app/lib/metrics.server.ts";
@@ -26,42 +25,6 @@ if (process.env.SENTRY_DISABLED !== "true" && sentryDsn) {
 }
 
 const port = Number(process.env.PORT ?? 3000);
-const CLIENT_DIR = resolve(import.meta.dirname, "build", "client");
-
-const MIME: Record<string, string> = {
-  ".js": "application/javascript",
-  ".css": "text/css",
-  ".html": "text/html",
-  ".json": "application/json",
-  ".png": "image/png",
-  ".jpg": "image/jpeg",
-  ".svg": "image/svg+xml",
-  ".ico": "image/x-icon",
-  ".woff": "font/woff",
-  ".woff2": "font/woff2",
-};
-
-function serveStatic(req: IncomingMessage, res: ServerResponse): boolean {
-  if (req.method !== "GET" && req.method !== "HEAD") return false;
-
-  const url = new URL(req.url ?? "/", `http://${req.headers.host}`);
-  const filePath = resolve(join(CLIENT_DIR, url.pathname));
-
-  if (!filePath.startsWith(CLIENT_DIR)) return false;
-
-  try {
-    if (!statSync(filePath).isFile()) return false;
-  } catch {
-    return false;
-  }
-
-  res.setHeader("Content-Type", MIME[extname(filePath)] ?? "application/octet-stream");
-  if (url.pathname.startsWith("/assets/")) {
-    res.setHeader("Cache-Control", "public, immutable, max-age=31536000");
-  }
-  createReadStream(filePath).pipe(res);
-  return true;
-}
 
 const listener = createRequestListener({
   build: () => import("./build/server/index.js" as string) as never,

apps/journal/vitest.config.ts

@@ -3,6 +3,13 @@ import shared from "../../vitest.shared.ts";
 import { resolve } from "node:path";
 
 export default mergeConfig(shared, defineConfig({
+  test: {
+    // mergeConfig concatenates arrays, so this adds to the shared
+    // include globs rather than replacing them. Picks up co-located
+    // tests for root-level server infra (server.ts, serve-static.ts)
+    // that live outside app/ and src/.
+    include: ["*.test.{ts,tsx}"],
+  },
   resolve: {
     alias: {
       "~": resolve(import.meta.dirname, "app"),