Public: anyone can read this; the crew can change it.
An app's page served with the forge toolbar keeps a validator, so a signed-out revisit is a 304 instead of the whole page #9139
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9136-an-app-s-page-served-with-the-forge-tool → main 3 commits fork 2 threads · 2 unresolved
2 files
changed +181
−1
src/apps/proxy.ts +29 −1
| @@ -20,6 +20,7 @@ import type { Database } from "bun:sqlite"; | ||
| 20 | 20 | import { getIssue, type Issue } from "../core/issues.ts"; |
| 21 | 21 | import { getPull } from "../core/pulls.ts"; |
| 22 | 22 | import { mayView } from "../core/sealed.ts"; |
| 23 | import { holds, weakEtag } from "../http/etag.ts"; | |
| 23 | 24 | import { holdOpen } from "../http/caller.ts"; |
| 24 | 25 | import { appHostSuffix } from "./host.ts"; |
| 25 | 26 | import { REPORT_LINK_TITLE } from "../actions/report_words.ts"; |
| @@ -1148,8 +1149,12 @@ export async function proxyApp( | ||
| 1148 | 1149 | !target.nested && |
| 1149 | 1150 | status.toolbar !== false && |
| 1150 | 1151 | (request.headers.get("accept") ?? "").includes("text/html") |
| 1151 | ) | |
| 1152 | ) { | |
| 1152 | 1153 | headers.set("accept-encoding", "identity"); |
| 1154 | // The proxy compares after fetching: a tag it did not make would pair an | |
| 1155 | // old toolbar with a new page. | |
| 1156 | headers.delete("if-none-match"); | |
| 1157 | } | |
| 1153 | 1158 | |
| 1154 | 1159 | if (isWebSocketHandshake(request)) { |
| 1155 | 1160 | return proxyWebSocket( |
| @@ -1279,8 +1284,31 @@ export async function proxyApp( | ||
| 1279 | 1284 | out.set("content-encoding", encoding); |
| 1280 | 1285 | } |
| 1281 | 1286 | out.set("content-length", String(sent.length)); |
| 1287 | const upstreamTag = out.get("etag"); | |
| 1282 | 1288 | out.delete("etag"); |
| 1283 | 1289 | out.delete("content-md5"); |
| 1290 | // The same page for every viewer keeps a validator: the app's tag joined | |
| 1291 | // with the bytes of what was sent, so a new toolbar is a new tag. | |
| 1292 | const tag = | |
| 1293 | upstreamTag !== null && | |
| 1294 | response.status === 200 && | |
| 1295 | request.method === "GET" && | |
| 1296 | proof === undefined && | |
| 1297 | vouched?.who === undefined && | |
| 1298 | out.getSetCookie().length === 0 && | |
| 1299 | cspNonce(out.get("content-security-policy")) === undefined && | |
| 1300 | !/\b(no-store|private)\b/i.test(out.get("cache-control") ?? "") | |
| 1301 | ? weakEtag(`${upstreamTag}|${Bun.hash(page).toString(36)}`) | |
| 1302 | : undefined; | |
| 1303 | if (tag !== undefined) { | |
| 1304 | out.set("etag", tag); | |
| 1305 | if (holds(request, tag)) { | |
| 1306 | out.delete("content-length"); | |
| 1307 | out.delete("content-encoding"); | |
| 1308 | relayPhase(request, "rewrite", performance.now() - rewriting); | |
| 1309 | return withPhases(request, new Response(null, { status: 304, headers: out })); | |
| 1310 | } | |
| 1311 | } | |
| 1284 | 1312 | relayPhase(request, "rewrite", performance.now() - rewriting); |
| 1285 | 1313 | return withPhases( |
| 1286 | 1314 | request, |
test/apps-proxy-etag.test.ts added +152
| @@ -0,0 +1,152 @@ | ||
| 1 | /** A page served through the proxy with the toolbar keeps a validator (#9136). */ | |
| 2 | ||
| 3 | import { afterAll, beforeAll, expect, spyOn, test } from "bun:test"; | |
| 4 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 5 | import { tmpdir } from "node:os"; | |
| 6 | import { join } from "node:path"; | |
| 7 | import { createApp, type App } from "../src/app.ts"; | |
| 8 | import { newApp } from "../src/actions/apps.ts"; | |
| 9 | import { AppsRuntime, type EnvStatus } from "../src/apps/runtime.ts"; | |
| 10 | import { signGrant, type AppTarget } from "../src/apps/proxy.ts"; | |
| 11 | import { createUser } from "../src/core/users.ts"; | |
| 12 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 13 | import { quietLogs } from "./helpers/globals.ts"; | |
| 14 | ||
| 15 | quietLogs(); | |
| 16 | ||
| 17 | const DOMAIN = "apps.test"; | |
| 18 | ||
| 19 | let dataDir: string; | |
| 20 | let record: AppTarget["app"]; | |
| 21 | let app: App; | |
| 22 | let servers: Servers; | |
| 23 | let runtime: AppsRuntime; | |
| 24 | let backend: ReturnType<typeof Bun.serve>; | |
| 25 | let toolbar: boolean | undefined = undefined; | |
| 26 | let page = "<!doctype html><html><head><title>a</title></head><body>one</body></html>"; | |
| 27 | let etagNow = 'W/"one"'; | |
| 28 | let status: ReturnType<typeof spyOn>; | |
| 29 | ||
| 30 | beforeAll(async () => { | |
| 31 | backend = Bun.serve({ | |
| 32 | port: 0, | |
| 33 | fetch: (req) => { | |
| 34 | const path = new URL(req.url).pathname; | |
| 35 | if (path === "/missing") | |
| 36 | return new Response(page, { status: 404, headers: { "content-type": "text/html", etag: '"v"' } }); | |
| 37 | if (path === "/private") | |
| 38 | return new Response(page, { headers: { "content-type": "text/html", etag: '"v"', "cache-control": "private, no-store" } }); | |
| 39 | return new Response(page, { headers: { "content-type": "text/html; charset=utf-8", etag: etagNow } }); | |
| 40 | }, | |
| 41 | }); | |
| 42 | dataDir = await mkdtemp(join(tmpdir(), "forge-proxy-timing-")); | |
| 43 | app = createApp({ dataDir, port: 0, host: "forge.test" }); | |
| 44 | servers = await serve(app); | |
| 45 | runtime = new AppsRuntime(app, { | |
| 46 | domain: DOMAIN, | |
| 47 | scheme: "http", | |
| 48 | publicPort: servers.port, | |
| 49 | forgeUrl: servers.url, | |
| 50 | concurrency: 1, | |
| 51 | }); | |
| 52 | await runtime.start(); | |
| 53 | status = spyOn(AppsRuntime.prototype, "envStatus").mockImplementation( | |
| 54 | () => | |
| 55 | ({ | |
| 56 | env: "production", | |
| 57 | state: "running", | |
| 58 | port: backend.port, | |
| 59 | commitOid: "a".repeat(40), | |
| 60 | toolbar, | |
| 61 | }) as EnvStatus, | |
| 62 | ); | |
| 63 | const a = await createUser(app.db, { | |
| 64 | origin: "seed", | |
| 65 | trusted: true, | |
| 66 | username: "alice", | |
| 67 | email: "[email protected]", | |
| 68 | password: "a-good-passphrase", | |
| 69 | adminIfFirst: true, | |
| 70 | }); | |
| 71 | if (!a.ok) throw new Error(a.error); | |
| 72 | const made = await newApp(app, a.value, { | |
| 73 | name: "notes", | |
| 74 | description: "A place for words", | |
| 75 | visibility: "public", | |
| 76 | }); | |
| 77 | if (!made.ok) throw new Error(String(made.error)); | |
| 78 | record = made.value.app; | |
| 79 | }, 120_000); | |
| 80 | ||
| 81 | afterAll(async () => { | |
| 82 | status.mockRestore(); | |
| 83 | await runtime.stop(); | |
| 84 | servers.stop(); | |
| 85 | backend.stop(true); | |
| 86 | app.close(); | |
| 87 | await rm(dataDir, { recursive: true, force: true }); | |
| 88 | }); | |
| 89 | ||
| 90 | const HEADERS = { host: `notes.${DOMAIN}`, accept: "text/html" }; | |
| 91 | const get = (path: string, extra: Record<string, string> = {}) => | |
| 92 | fetch(`${servers.url}${path}`, { headers: { ...HEADERS, ...extra }, redirect: "manual" }); | |
| 93 | ||
| 94 | test("a tagged page is 200 with a weak etag, then 304 on a repeat", async () => { | |
| 95 | const first = await get("/"); | |
| 96 | expect(first.status).toBe(200); | |
| 97 | const tag = first.headers.get("etag")!; | |
| 98 | expect(tag).toStartWith("W/"); | |
| 99 | expect(await first.text()).toContain("one"); | |
| 100 | const again = await get("/", { "if-none-match": tag }); | |
| 101 | expect(again.status).toBe(304); | |
| 102 | expect(await again.text()).toBe(""); | |
| 103 | }); | |
| 104 | ||
| 105 | test("a changed upstream body or toolbar is 200 with a new tag", async () => { | |
| 106 | const tag = (await get("/")).headers.get("etag")!; | |
| 107 | page = page.replace("one", "two"); | |
| 108 | etagNow = 'W/"two"'; | |
| 109 | const changed = await get("/", { "if-none-match": tag }); | |
| 110 | expect(changed.status).toBe(200); | |
| 111 | const next = changed.headers.get("etag")!; | |
| 112 | expect(next).not.toBe(tag); | |
| 113 | expect((await get("/", { "if-none-match": next })).status).toBe(304); | |
| 114 | const opts = runtime.options as { forgeUrl: string }; | |
| 115 | const was = opts.forgeUrl; | |
| 116 | opts.forgeUrl = "http://other.test"; | |
| 117 | try { | |
| 118 | const moved = await get("/", { "if-none-match": next }); | |
| 119 | expect(moved.status).toBe(200); | |
| 120 | expect(moved.headers.get("etag")).not.toBe(next); | |
| 121 | } finally { | |
| 122 | opts.forgeUrl = was; | |
| 123 | } | |
| 124 | }); | |
| 125 | ||
| 126 | test("a private page and a non-200 carry no etag", async () => { | |
| 127 | expect((await get("/private")).headers.get("etag")).toBeNull(); | |
| 128 | const missing = await get("/missing"); | |
| 129 | expect(missing.status).toBe(404); | |
outdated
This is the only no-tag-for-proof coverage and it tests the upstream header, not the proof or signed-in path. Add a test that sends a real signed-in or proof request and asserts no etag. |
||
| 130 | expect(missing.headers.get("etag")).toBeNull(); | |
| 131 | }); | |
| 132 | ||
| 133 | test("a signed-in visitor's page carries no etag, with the proof or without it", async () => { | |
| 134 | const grant = signGrant(dataDir, { app: record, env: "production" } as AppTarget, 60_000, Date.now(), { | |
| 135 | id: "u1", | |
| 136 | name: "alice", | |
| 137 | role: "admin", | |
| 138 | }); | |
| 139 | const cookie = `forge_grant_${record.id.slice(-8)}=${grant}`; | |
| 140 | // A document load is handed the one-use proof. | |
| 141 | const withProof = await get("/", { cookie, "sec-fetch-dest": "document" }); | |
| 142 | expect(withProof.status).toBe(200); | |
| 143 | expect(withProof.headers.get("cache-control")).toBe("private, no-store"); | |
| 144 | expect(withProof.headers.get("etag")).toBeNull(); | |
| 145 | // A script's own fetch gets no proof, yet is still a signed-in page: still untagged. | |
| 146 | const bare = await get("/", { cookie, "sec-fetch-dest": "empty" }); | |
| 147 | expect(bare.status).toBe(200); | |
| 148 | expect(bare.headers.get("etag")).toBeNull(); | |
| 149 | // And a guest's tag never answers a signed-in revisit with 304. | |
| 150 | const guestTag = (await get("/")).headers.get("etag")!; | |
| 151 | expect((await get("/", { cookie, "if-none-match": guestTag })).status).toBe(200); | |
| 152 | }); |
new above
offline
Orphan comment above nothing, and blank lines doubled. Remove.