diff --git a/cloudflare-worker/src/resolver.ts b/cloudflare-worker/src/resolver.ts index ec4fd061..13203c70 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"`, @@ -39,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 @@ -161,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, + }, + }); } /** @@ -252,11 +271,13 @@ 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); - url.pathname = `${url.pathname}/`; - return Response.redirect(url.toString(), 301); + // 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 { pathname } = new URL(request.url); + if (!pathname.endsWith("/")) { + return resolveRedirect(`${pathname}/`, request); } resolvedPath = indexKey.slice(prefix.length); } diff --git a/cloudflare-worker/test/index.test.ts b/cloudflare-worker/test/index.test.ts index 43e7c726..0cec6dcc 100644 --- a/cloudflare-worker/test/index.test.ts +++ b/cloudflare-worker/test/index.test.ts @@ -132,6 +132,21 @@ 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/", + ); + expect(response.headers.get("Cache-Control")).toBe( + "public, max-age=300, s-maxage=86400", + ); + }); + it("infers correct Content-Type for CSS", async () => { const response = await SELF.fetch( "https://pipelines.lsst.io/style.css", @@ -429,6 +444,33 @@ 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/", + ); + expect(response.headers.get("Cache-Control")).toBe( + "public, max-age=300, s-maxage=86400", + ); + }); + 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..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,68 @@ 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 () => { + // `/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", + ); + 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 () => { + 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 () => { @@ -961,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();