From 98899c76dbb0a418982444ddc220a4a5e107c10b Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 9 Nov 2026 13:50:43 -0500 Subject: [PATCH 1/3] Redirect edition root URLs without a trailing slash A request for /v/{edition} (no trailing slash) served the edition's index.html in place with a 200 instead of redirecting to /v/{edition}/. The browser then resolved the page's relative asset links against /v/ rather than /v/{edition}/, so Sphinx CSS and JS failed to load and the page rendered unstyled. Reported against pipelines.lsst.io/v/weekly. The resolver's directory-index fallback only redirected when route.path was non-empty, a guard meant to stop the main edition's "/" from redirecting to "//". But the edition root has an empty route.path for both /v/weekly/ and /v/weekly, so the no-slash form slipped through. Decide from the request pathname instead: redirect whenever the index file is found and the pathname does not already end in "/". This also covers the path-prefix scheme's /docs/{project} root. Add resolver unit tests for the edition root with and without the slash (including query-string preservation) and integration tests for the subdomain and path-prefix schemes. --- cloudflare-worker/src/resolver.ts | 13 ++++-- cloudflare-worker/test/index.test.ts | 36 +++++++++++++++ cloudflare-worker/test/resolver.test.ts | 60 +++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 4 deletions(-) diff --git a/cloudflare-worker/src/resolver.ts b/cloudflare-worker/src/resolver.ts index ec4fd061..3c706e8b 100644 --- a/cloudflare-worker/src/resolver.ts +++ b/cloudflare-worker/src/resolver.ts @@ -7,7 +7,9 @@ * 1. Look up `{project}/{edition}` in KV to get the build mapping. * 2. Construct the R2 object key as `{r2_prefix}{file_path}`. * 3. Try exact path, then `{path}/index.html` for directory index - * resolution. + * resolution. When the index file is found but the request pathname + * lacks a trailing slash (e.g. `/v/weekly` or `/api/core`), return a + * 301 to the slash-terminated URL so relative asset links resolve. * 4. Return the R2 object with `Content-Type` inferred from the file * extension and `Cache-Control` derived from the KV value's * `cache_profile` field: `CACHE_CONTROL_LONG` for `"long"`, @@ -252,9 +254,12 @@ async function resolveEdition( : `${r2Key}/index.html`; object = await r2.get(indexKey); if (object !== null) { - // Redirect to trailing slash so relative links resolve correctly - if (route.path !== "" && !route.path.endsWith("/")) { - const url = new URL(request.url); + // Redirect to trailing slash so relative links resolve correctly. + // Decide from the request pathname rather than `route.path`: the + // edition root has an empty `route.path` whether the request was + // `/v/weekly/` or `/v/weekly`, and only the latter needs the redirect. + const url = new URL(request.url); + if (!url.pathname.endsWith("/")) { url.pathname = `${url.pathname}/`; return Response.redirect(url.toString(), 301); } diff --git a/cloudflare-worker/test/index.test.ts b/cloudflare-worker/test/index.test.ts index 43e7c726..707b37b2 100644 --- a/cloudflare-worker/test/index.test.ts +++ b/cloudflare-worker/test/index.test.ts @@ -132,6 +132,18 @@ describe("Worker integration — subdomain routing", () => { ); }); + it("redirects /v/{edition} without trailing slash to /v/{edition}/", async () => { + const response = await SELF.fetch( + "https://pipelines.lsst.io/v/v1.0", + { redirect: "manual" }, + ); + + expect(response.status).toBe(301); + expect(response.headers.get("Location")).toBe( + "https://pipelines.lsst.io/v/v1.0/", + ); + }); + it("infers correct Content-Type for CSS", async () => { const response = await SELF.fetch( "https://pipelines.lsst.io/style.css", @@ -429,6 +441,30 @@ describe("Worker integration — path-prefix routing", () => { expect(await response.text()).toBe("root"); }); + it("redirects /docs/{project} without trailing slash", async () => { + const response = await fetchPathPrefix( + "https://docs.example.com/docs/pipelines", + { redirect: "manual" }, + ); + + expect(response.status).toBe(301); + expect(response.headers.get("Location")).toBe( + "https://docs.example.com/docs/pipelines/", + ); + }); + + it("redirects /docs/{project}/v/{edition} without trailing slash", async () => { + const response = await fetchPathPrefix( + "https://docs.example.com/docs/pipelines/v/main", + { redirect: "manual" }, + ); + + expect(response.status).toBe(301); + expect(response.headers.get("Location")).toBe( + "https://docs.example.com/docs/pipelines/v/main/", + ); + }); + it("serves a page under __main", async () => { const response = await fetchPathPrefix( "https://docs.example.com/docs/pipelines/getting-started.html", diff --git a/cloudflare-worker/test/resolver.test.ts b/cloudflare-worker/test/resolver.test.ts index 2d58d3f9..2e57dd99 100644 --- a/cloudflare-worker/test/resolver.test.ts +++ b/cloudflare-worker/test/resolver.test.ts @@ -353,6 +353,66 @@ describe("resolve — edition routes", () => { ); }); + it("redirects edition root without trailing slash", async () => { + // `/v/weekly` parses to an edition route with an empty path, exactly + // like `/v/weekly/`; only the request pathname tells them apart. + const rootRoute: Route = { + kind: "edition", + project: "pipelines", + edition: "weekly", + path: "", + }; + const kv = createMockKV({ + "pipelines/weekly": JSON.stringify({ + build_id: "b123", + r2_prefix: "pipelines/__builds/b123/", + }), + }); + const r2 = createMockR2({ + "pipelines/__builds/b123/index.html": { + body: streamFromString("weekly"), + size: 19, + }, + }); + const dashboardStore = createMockDashboardStore(); + const request = new Request("https://pipelines.lsst.io/v/weekly?x=1"); + + const response = await resolve(rootRoute, request, kv, r2, dashboardStore, ctx); + + expect(response.status).toBe(301); + expect(response.headers.get("Location")).toBe( + "https://pipelines.lsst.io/v/weekly/?x=1", + ); + }); + + it("serves the edition root index.html when the request has a trailing slash", async () => { + const rootRoute: Route = { + kind: "edition", + project: "pipelines", + edition: "weekly", + path: "", + }; + const kv = createMockKV({ + "pipelines/weekly": JSON.stringify({ + build_id: "b123", + r2_prefix: "pipelines/__builds/b123/", + }), + }); + const r2 = createMockR2({ + "pipelines/__builds/b123/index.html": { + body: streamFromString("weekly"), + size: 19, + }, + }); + const dashboardStore = createMockDashboardStore(); + const request = new Request("https://pipelines.lsst.io/v/weekly/"); + + const response = await resolve(rootRoute, request, kv, r2, dashboardStore, ctx); + + expect(response.status).toBe(200); + expect(await response.text()).toBe("weekly"); + }); + it("serves index.html when path has trailing slash", async () => { const directoryRoute: Route = { kind: "edition", From 91dd4b4f35c8ec55f0aae1d736ce3941e911672f Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 9 Nov 2026 15:14:40 -0500 Subject: [PATCH 2/3] Reuse resolveRedirect for the directory-index trailing-slash 301 The directory-index fallback built its own URL and Response.redirect, duplicating the resolveRedirect helper that the /v redirect route already uses. Route both canonicalising redirects through the helper so they cannot drift apart. --- cloudflare-worker/src/resolver.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cloudflare-worker/src/resolver.ts b/cloudflare-worker/src/resolver.ts index 3c706e8b..e13a4a3b 100644 --- a/cloudflare-worker/src/resolver.ts +++ b/cloudflare-worker/src/resolver.ts @@ -258,10 +258,9 @@ async function resolveEdition( // Decide from the request pathname rather than `route.path`: the // edition root has an empty `route.path` whether the request was // `/v/weekly/` or `/v/weekly`, and only the latter needs the redirect. - const url = new URL(request.url); - if (!url.pathname.endsWith("/")) { - url.pathname = `${url.pathname}/`; - return Response.redirect(url.toString(), 301); + const { pathname } = new URL(request.url); + if (!pathname.endsWith("/")) { + return resolveRedirect(`${pathname}/`, request); } resolvedPath = indexKey.slice(prefix.length); } From 6d3a6a70e37f09502b53064905dfbe8f51c129b0 Mon Sep 17 00:00:00 2001 From: Jonathan Sick Date: Mon, 9 Nov 2026 15:18:07 -0500 Subject: [PATCH 3/3] Send an explicit Cache-Control on canonicalising redirects Response.redirect() emits a bare 301 with only a Location header. The Cloudflare edge cannot cache a response without a cache directive, so every no-slash request reached the Worker and paid KV + R2 I/O, while browsers treat a header-less 301 as heuristically cacheable forever. Build the 301 by hand in resolveRedirect and attach CACHE_CONTROL_LONG (5 min browser, 1 day edge). The target is a pure function of the request URL's shape, never of edition content, so the long profile is safe regardless of the edition's own cache_profile. Assert the header in the redirect unit and integration tests. --- cloudflare-worker/src/resolver.ts | 21 +++++++++++++++++++-- cloudflare-worker/test/index.test.ts | 6 ++++++ cloudflare-worker/test/resolver.test.ts | 5 ++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/cloudflare-worker/src/resolver.ts b/cloudflare-worker/src/resolver.ts index e13a4a3b..13203c70 100644 --- a/cloudflare-worker/src/resolver.ts +++ b/cloudflare-worker/src/resolver.ts @@ -41,7 +41,10 @@ * responses. * * - `redirect` routes return a 301 with `Location` set to `route.to` on the - * request's origin, preserving the original query string. + * request's origin, preserving the original query string. Every 301 this + * module emits (redirect routes and the directory-index trailing-slash + * canonicalisation) carries `CACHE_CONTROL_LONG`, since the target is a + * pure function of the request URL. * * 404 handling: every 404-producing branch in this module routes through * `notFoundResponse(project, dashboardStore, ctx)`, which serves the @@ -163,10 +166,24 @@ export async function resolve( } } +/** + * Build a 301 to `to` on the request's origin, preserving the query string. + * + * Carries an explicit `Cache-Control` so the edge can cache the redirect + * and browsers don't fall back to heuristic (effectively permanent) caching + * of a header-less 301. The long profile is safe here: the target depends + * only on the request URL's shape, never on edition content. + */ function resolveRedirect(to: string, request: Request): Response { const url = new URL(request.url); url.pathname = to; - return Response.redirect(url.toString(), 301); + return new Response(null, { + status: 301, + headers: { + "Location": url.toString(), + "Cache-Control": CACHE_CONTROL_LONG, + }, + }); } /** diff --git a/cloudflare-worker/test/index.test.ts b/cloudflare-worker/test/index.test.ts index 707b37b2..0cec6dcc 100644 --- a/cloudflare-worker/test/index.test.ts +++ b/cloudflare-worker/test/index.test.ts @@ -142,6 +142,9 @@ describe("Worker integration — subdomain routing", () => { expect(response.headers.get("Location")).toBe( "https://pipelines.lsst.io/v/v1.0/", ); + expect(response.headers.get("Cache-Control")).toBe( + "public, max-age=300, s-maxage=86400", + ); }); it("infers correct Content-Type for CSS", async () => { @@ -463,6 +466,9 @@ describe("Worker integration — path-prefix routing", () => { expect(response.headers.get("Location")).toBe( "https://docs.example.com/docs/pipelines/v/main/", ); + expect(response.headers.get("Cache-Control")).toBe( + "public, max-age=300, s-maxage=86400", + ); }); it("serves a page under __main", async () => { diff --git a/cloudflare-worker/test/resolver.test.ts b/cloudflare-worker/test/resolver.test.ts index 2e57dd99..12c7a7f5 100644 --- a/cloudflare-worker/test/resolver.test.ts +++ b/cloudflare-worker/test/resolver.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { resolve, notFoundCacheKey } from "../src/resolver"; +import { resolve, notFoundCacheKey, CACHE_CONTROL_LONG } from "../src/resolver"; import type { DashboardStore } from "../src/dashboardStore"; import type { Route } from "../src/router"; @@ -351,6 +351,7 @@ describe("resolve — edition routes", () => { expect(response.headers.get("Location")).toBe( "https://example.com/pipelines/api/core/", ); + expect(response.headers.get("Cache-Control")).toBe(CACHE_CONTROL_LONG); }); it("redirects edition root without trailing slash", async () => { @@ -383,6 +384,7 @@ describe("resolve — edition routes", () => { expect(response.headers.get("Location")).toBe( "https://pipelines.lsst.io/v/weekly/?x=1", ); + expect(response.headers.get("Cache-Control")).toBe(CACHE_CONTROL_LONG); }); it("serves the edition root index.html when the request has a trailing slash", async () => { @@ -1021,6 +1023,7 @@ describe("resolve — redirect routes", () => { expect(new URL(response.headers.get("Location") ?? "").pathname).toBe( "/v/", ); + expect(response.headers.get("Cache-Control")).toBe(CACHE_CONTROL_LONG); // Edition / dashboard paths must not be touched for redirect dispatch. expect(kv.get).not.toHaveBeenCalled(); expect(r2.get).not.toHaveBeenCalled();