Public: anyone can read this; the crew can change it.

GET /api/v1/repos/:owner/:name/upstream carries an ETag and answers a repeat with 304 until the outside upstream or its last check changes #9267

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/9245-get-api-v1-repos-owner-name-upstream-car → main 2 commits fork

  • Merged. scout/9245-get-api-v1-repos-owner-name-upstream-car was merged into main as bc0221c.

Review

Sign in to review this change.
2 files changed +127 −1
src/http/api/upstream.ts +8 −1
@@ -37,7 +37,14 @@ export function upstreamRoutes(router: Router<Handler>): void {
3737 return getFollow(ctx.app.db, repo.id) ?? null;
3838 });
3939 if (read === undefined) return notFound();
40 return timePhase(ctx.request, "render", () => json({ following: read === null ? null : followJson(read) }));
40 // The tag is of the exact bytes sent, and only after the access check above: a refusal carries none.
41 return timePhase(ctx.request, "render", () => {
42 const text = JSON.stringify({ following: read === null ? null : followJson(read) });
43 const etag = weakEtag(text);
44 const headers = { etag, "cache-control": "private, no-cache", vary: "authorization, cookie" };
45 if (holds(ctx.request, etag)) return new Response(null, { status: 304, headers });
46 return new Response(text, { headers: { "content-type": "application/json", ...headers } });
47 });
4148 });
4249
4350 router.add("PUT", "/api/v1/repos/:owner/:name/upstream", async (ctx) => {
test/upstream-etag.test.ts added +119
@@ -0,0 +1,119 @@
1 /** GET …/upstream carries a weak ETag and answers a repeat with 304 until the upstream or its last check moves (#9245). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createRepo } from "../src/actions/repos.ts";
7 import { createApp, type App } from "../src/app.ts";
8 import { createToken } from "../src/core/tokens.ts";
9 import { createUser } from "../src/core/users.ts";
10 import { noteCheck, putFollow } from "../src/core/upstream_follow.ts";
11 import { serve, type Servers } from "../src/http/server.ts";
12 import { promote } from "./helpers/members.ts";
13
14 let dataDir = "";
15 let app: App, servers: Servers;
16 let alice: any, repo: any;
17 let aliceBearer = "", bobBearer = "";
18 const URL_ = () => `${servers.url}/api/v1/repos/alice/app/upstream`;
19 const get = (headers: Record<string, string> = {}, bearer = aliceBearer) =>
20 fetch(URL_(), { headers: bearer ? { authorization: `Bearer ${bearer}`, ...headers } : headers });
21 const tagOf = async () => {
22 const r = await get();
23 await r.text();
24 return r.headers.get("etag")!;
25 };
26 async function changed(before: string, what: string): Promise<string> {
27 const res = await get({ "if-none-match": before });
28 expect(res.status, what).toBe(200);
29 await res.text();
30 const tag = res.headers.get("etag")!;
31 expect(tag, what).not.toBe(before);
32 return tag;
33 }
34 const mk = async (username: string) => {
35 const r = await createUser(app.db, { origin: "seed", trusted: true, username, email: `${username}@f.dev`, password: "a-good-passphrase", adminIfFirst: username === "alice" });
36 if (!r.ok) throw new Error("user");
37 promote(app.db, r.value);
38 return r.value;
39 };
40 const follow = (url: string, branch = "main", enabled = true) =>
41 putFollow(app.db, { repoId: repo.id, url, branch, enabled, byId: alice.id, originId: "" });
42
43 beforeAll(async () => {
44 dataDir = await mkdtemp(join(tmpdir(), "forge-upstreametag-"));
45 app = createApp({ dataDir, port: 0 });
46 servers = await serve(app);
47 alice = await mk("alice");
48 const bob = await mk("bob");
49 const made = await createRepo(app, alice, { name: "app", visibility: "public" });
50 if (!made.ok) throw new Error("repo");
51 repo = made.value;
52 aliceBearer = createToken(app.db, alice.id, "a", "ro").token;
53 bobBearer = createToken(app.db, bob.id, "b", "ro").token;
54 });
55 afterAll(async () => {
56 servers.stop();
57 app.close();
58 await rm(dataDir, { recursive: true, force: true });
59 });
60
61 test("200 carries a weak tag, private no-cache and vary; a repeat is a bodiless 304 with the floor headers", async () => {
62 follow("https://example.com/a.git");
63 const res = await get();
64 expect(res.status).toBe(200);
65 const tag = res.headers.get("etag")!;
66 expect(tag).toMatch(/^W\//);
67 expect(res.headers.get("cache-control")).toBe("private, no-cache");
68 expect(res.headers.get("vary")).toContain("authorization, cookie");
69 expect(res.headers.get("content-type")).toContain("application/json");
70 expect(((await res.json()) as any).following.url).toBe("https://example.com/a.git");
71 const again = await get({ "if-none-match": tag });
72 expect(again.status).toBe(304);
73 expect(again.headers.get("etag")).toBe(tag);
74 expect(again.headers.get("cache-control")).toBe("private, no-cache");
75 expect(again.headers.get("vary")).toContain("authorization, cookie");
76 expect(again.headers.get("x-content-type-options")).toBe("nosniff");
77 expect(again.headers.get("content-security-policy")).not.toBeNull();
78 expect(await again.text()).toBe("");
79 });
80
81 test("the upstream changed, a check recorded and the upstream removed each change the tag", async () => {
82 let tag = await tagOf();
83 follow("https://example.com/b.git");
84 tag = await changed(tag, "url changed");
85 noteCheck(app.db, repo.id, { at: new Date().toISOString(), tip: "abc1234", behind: 3, error: "" });
86 tag = await changed(tag, "check recorded");
87 noteCheck(app.db, repo.id, { at: new Date(Date.now() + 1000).toISOString(), tip: "abc1234", behind: 3, error: "" });
88 tag = await changed(tag, "check repeated at a new time");
89 follow("https://example.com/b.git", "main", false);
90 tag = await changed(tag, "disabled");
91 app.db.query("DELETE FROM repo_upstream WHERE repo_id = ?1").run(repo.id);
92 const res = await get({ "if-none-match": tag });
93 expect(res.status).toBe(200);
94 expect(await res.json()).toEqual({ following: null });
95 expect(res.headers.get("etag")).not.toBe(tag);
96 });
97
98 test("a non-admin, an anonymous caller and a missing repository answer as before with no ETag, even for *", async () => {
99 follow("https://example.com/c.git");
100 for (const [what, res] of [
101 ["non-admin", await get({ "if-none-match": "*" }, bobBearer)],
102 ["anonymous", await get({ "if-none-match": "*" }, "")],
103 ["missing", await fetch(`${servers.url}/api/v1/repos/alice/nope/upstream`, { headers: { authorization: `Bearer ${aliceBearer}`, "if-none-match": "*" } })],
104 ] as const) {
105 expect(res.status, what).not.toBe(304);
106 expect(res.status, what).toBeGreaterThanOrEqual(400);
107 expect(res.headers.get("etag"), what).toBeNull();
108 await res.text();
109 }
110 const owner = await get({ "if-none-match": "*" });
111 expect(owner.status).toBe(304);
112 });
113
114 test("two viewers shown different bodies never share a tag", async () => {
115 const admin = await tagOf();
116 const stranger = await get({ "if-none-match": admin }, bobBearer);
117 expect(stranger.status).toBeGreaterThanOrEqual(400);
118 expect(stranger.headers.get("etag")).toBeNull();
119 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs