Skip to content

Commit 41a8692

Browse files
fix: audit pass 7 — Link header parser + cache Vary key
### Bug 50 — Link header parser (RFC 8288 §3) The previous parser split on raw commas, which broke as soon as a URL contained one (e.g. \`/items?ids=1,2,3\`). It also missed: - space-separated rel values (\`rel="next foo"\`) - malformed entries (returned a partial mapping) - rel after a quoted value with embedded semicolons Replaced the regex-split approach with a state machine that walks the header treating \`<...>\` as the URL boundary and consuming params up to the next top-level comma. Five new tests in test/link-header.test.ts pin: simple rel=next, URL with commas, multi-link, space-separated rel, malformed input. ### Bug — cache Vary stored only the latest variant \`vary\` headers were recorded on the cache entry but the entry was keyed only by URL+method. Sequential requests with different Accept-Language overwrote each other; en-tr-en hit the network three times. Now per-variant entries are stored under a derived key (\`baseKey | header=value & ...\`) and the base entry remembers Vary so beforeRequest knows to look up the variant. en-tr-en hits the network twice (en2 served from cache). Four new tests in test/cache-rfc.test.ts pin: \`Cache-Control: no-store\`, \`max-age\` overriding local TTL, Vary header per-variant caching, and \`Vary: *\` forcing revalidation. 126/126 tests pass; lint, typecheck, build, bundle budget all clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e6b89bf commit 41a8692

4 files changed

Lines changed: 375 additions & 15 deletions

File tree

src/cache/index.ts

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,19 @@ export function withCache(misina: Misina, opts: CacheOptions = {}): Misina {
7272
hooks: {
7373
beforeRequest: async (ctx) => {
7474
if (!methods.includes(ctx.options.method)) return
75-
const key = keyOf(ctx)
76-
const entry = await store.get(key)
75+
const baseKey = keyOf(ctx)
76+
// Try the base key first; if that entry has Vary, build a per-variant
77+
// key from the request headers and look that up too.
78+
const baseEntry = await store.get(baseKey)
79+
const variantKey = baseEntry?.vary
80+
? variantKeyFor(baseKey, baseEntry.vary, ctx.request.headers)
81+
: undefined
82+
const entry = variantKey ? await store.get(variantKey) : baseEntry
7783
if (!entry) return
7884

79-
// Vary check: stored vary headers must match the current request.
8085
if (entry.vary && !varyMatches(entry.vary, ctx.request.headers)) return
8186

8287
if (entry.expires > Date.now()) {
83-
// Fresh — short-circuit fetch
8488
return entry.response.clone()
8589
}
8690

@@ -94,12 +98,12 @@ export function withCache(misina: Misina, opts: CacheOptions = {}): Misina {
9498
afterResponse: async (ctx) => {
9599
if (!ctx.response) return
96100
if (!methods.includes(ctx.options.method)) return
97-
const key = keyOf(ctx)
101+
const baseKey = keyOf(ctx)
98102

99103
if (ctx.response.status === 304) {
100-
const entry = await store.get(key)
104+
const entry = await store.get(baseKey)
101105
if (entry) {
102-
await store.set(key, { ...entry, expires: Date.now() + ttl })
106+
await store.set(baseKey, { ...entry, expires: Date.now() + ttl })
103107
return entry.response.clone()
104108
}
105109
}
@@ -120,18 +124,40 @@ export function withCache(misina: Misina, opts: CacheOptions = {}): Misina {
120124
const cloned = ctx.response.clone()
121125
const vary = recordVaryHeaders(ctx.response, ctx.request)
122126

123-
await store.set(key, {
127+
const entry: CacheEntry = {
124128
response: cloned,
125129
expires: Date.now() + entryTtl,
126130
etag: ctx.response.headers.get("etag") ?? undefined,
127131
lastModified: ctx.response.headers.get("last-modified") ?? undefined,
128132
vary,
129-
})
133+
}
134+
135+
if (vary && !("*" in vary)) {
136+
// Store the per-variant entry under a derived key, but also keep the
137+
// base entry's `vary` field so subsequent requests know to look up
138+
// a variant.
139+
const variantKey = variantKeyFor(baseKey, vary, ctx.request.headers)
140+
await store.set(variantKey, entry)
141+
await store.set(baseKey, { ...entry, response: ctx.response.clone() })
142+
} else {
143+
await store.set(baseKey, entry)
144+
}
130145
},
131146
},
132147
})
133148
}
134149

150+
function variantKeyFor(
151+
baseKey: string,
152+
vary: Record<string, string | null>,
153+
headers: Headers,
154+
): string {
155+
const parts = Object.keys(vary)
156+
.sort()
157+
.map((name) => `${name}=${headers.get(name) ?? ""}`)
158+
return `${baseKey} | ${parts.join(" & ")}`
159+
}
160+
135161
function parseMaxAge(cacheControl: string): number | null {
136162
const match = /(?:^|,)\s*max-age\s*=\s*(\d+)/i.exec(cacheControl)
137163
if (!match || !match[1]) return null

src/paginate/index.ts

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,76 @@ function defaultNext<R>(
118118
return { url: next }
119119
}
120120

121+
/**
122+
* Parse a `Link` header (RFC 8288 §3) into a `{ rel: url }` map. Splitting on
123+
* raw commas is wrong because URLs can contain commas (e.g.
124+
* `/items?ids=1,2,3`). We instead walk the header, treating `<...>` as the
125+
* URL and consuming params up to the next top-level comma.
126+
*/
121127
function parseLinkHeader(value: string): Record<string, string> {
122128
const out: Record<string, string> = {}
123-
for (const part of value.split(",")) {
124-
const match = /<([^>]+)>;\s*rel="?([^",]+)"?/.exec(part.trim())
125-
if (match) {
126-
const url = match[1]
127-
const rel = match[2]
128-
if (url && rel) out[rel] = url
129+
let i = 0
130+
while (i < value.length) {
131+
// Skip whitespace and leading commas
132+
while (i < value.length && (value[i] === "," || value[i] === " " || value[i] === "\t")) i++
133+
if (i >= value.length) break
134+
if (value[i] !== "<") break // malformed
135+
const close = value.indexOf(">", i + 1)
136+
if (close === -1) break
137+
const url = value.slice(i + 1, close)
138+
i = close + 1
139+
140+
// Walk parameters until the next top-level comma.
141+
let rel: string | undefined
142+
while (i < value.length && value[i] !== ",") {
143+
// Skip ; and whitespace
144+
while (i < value.length && (value[i] === ";" || value[i] === " " || value[i] === "\t")) i++
145+
if (i >= value.length || value[i] === ",") break
146+
// Read param name
147+
const eq = value.indexOf("=", i)
148+
const semi = value.indexOf(";", i)
149+
const comma = value.indexOf(",", i)
150+
const end = nextStop(eq, semi, comma, value.length)
151+
const name = value.slice(i, end).trim().toLowerCase()
152+
i = end
153+
if (value[i] === "=") {
154+
i++
155+
let pval: string
156+
if (value[i] === '"') {
157+
const q = value.indexOf('"', i + 1)
158+
if (q === -1) {
159+
pval = value.slice(i + 1)
160+
i = value.length
161+
} else {
162+
pval = value.slice(i + 1, q)
163+
i = q + 1
164+
}
165+
} else {
166+
const stopSemi = value.indexOf(";", i)
167+
const stopComma = value.indexOf(",", i)
168+
const stop = nextStop(stopSemi, stopComma, -1, value.length)
169+
pval = value.slice(i, stop).trim()
170+
i = stop
171+
}
172+
if (name === "rel" && !rel) rel = pval
173+
}
174+
}
175+
176+
if (rel) {
177+
// A rel can be a space-separated list — store each so callers can ask
178+
// for any one of them.
179+
for (const r of rel.split(/\s+/)) {
180+
if (r && !(r in out)) out[r] = url
181+
}
129182
}
130183
}
131184
return out
132185
}
186+
187+
function nextStop(...candidates: number[]): number {
188+
let best = Infinity
189+
for (const c of candidates) {
190+
if (c >= 0 && c < best) best = c
191+
}
192+
return best === Infinity ? (candidates[candidates.length - 1] ?? 0) : best
193+
}

test/cache-rfc.test.ts

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { describe, expect, it } from "vitest"
2+
import { createMisina } from "../src/index.ts"
3+
import { withCache, memoryStore } from "../src/cache/index.ts"
4+
5+
function jsonResponse(body: unknown, init: ResponseInit = {}): Response {
6+
return new Response(JSON.stringify(body), {
7+
headers: { "content-type": "application/json" },
8+
...init,
9+
})
10+
}
11+
12+
describe("withCache — RFC 9111 compliance", () => {
13+
it("Cache-Control: no-store is honored (response not stored)", async () => {
14+
let calls = 0
15+
const driver = {
16+
name: "ns",
17+
request: async () => {
18+
calls++
19+
return jsonResponse(
20+
{ count: calls },
21+
{
22+
headers: { "content-type": "application/json", "cache-control": "no-store" },
23+
},
24+
)
25+
},
26+
}
27+
28+
const store = memoryStore()
29+
const m = withCache(createMisina({ driver, retry: 0 }), { store, ttl: 60_000 })
30+
31+
await m.get("https://api.test/")
32+
await m.get("https://api.test/")
33+
34+
// No store, so both calls hit network.
35+
expect(calls).toBe(2)
36+
})
37+
38+
it("Cache-Control: max-age overrides local ttl", async () => {
39+
let calls = 0
40+
const driver = {
41+
name: "served",
42+
request: async () => {
43+
calls++
44+
return jsonResponse(
45+
{ x: 1 },
46+
{
47+
headers: {
48+
"content-type": "application/json",
49+
// 1 second max-age — well under the local ttl of 60s
50+
"cache-control": "max-age=1",
51+
},
52+
},
53+
)
54+
},
55+
}
56+
57+
const store = memoryStore()
58+
const m = withCache(createMisina({ driver, retry: 0 }), { store, ttl: 60_000 })
59+
60+
await m.get("https://api.test/")
61+
expect(calls).toBe(1)
62+
63+
// Wait past the server-controlled max-age but well within local ttl.
64+
await new Promise((r) => setTimeout(r, 1100))
65+
66+
await m.get("https://api.test/")
67+
expect(calls).toBe(2)
68+
})
69+
70+
it("Vary header — different request headers bust the cache", async () => {
71+
let calls = 0
72+
const driver = {
73+
name: "vary",
74+
request: async (req: Request) => {
75+
calls++
76+
return jsonResponse(
77+
{ lang: req.headers.get("accept-language") },
78+
{
79+
headers: {
80+
"content-type": "application/json",
81+
vary: "Accept-Language",
82+
},
83+
},
84+
)
85+
},
86+
}
87+
88+
const store = memoryStore()
89+
const m = withCache(createMisina({ driver, retry: 0 }), { store, ttl: 60_000 })
90+
91+
const en = await m.get<{ lang: string }>("https://api.test/", {
92+
headers: { "accept-language": "en" },
93+
})
94+
expect(en.data.lang).toBe("en")
95+
96+
// Same URL, different Accept-Language — must not reuse the en cache.
97+
const tr = await m.get<{ lang: string }>("https://api.test/", {
98+
headers: { "accept-language": "tr" },
99+
})
100+
expect(tr.data.lang).toBe("tr")
101+
expect(calls).toBe(2)
102+
103+
// But en again hits the cache.
104+
const en2 = await m.get<{ lang: string }>("https://api.test/", {
105+
headers: { "accept-language": "en" },
106+
})
107+
expect(en2.data.lang).toBe("en")
108+
expect(calls).toBe(2)
109+
})
110+
111+
it("Vary: * forces revalidation every time", async () => {
112+
let calls = 0
113+
const driver = {
114+
name: "vary-star",
115+
request: async () => {
116+
calls++
117+
return jsonResponse(
118+
{ x: 1 },
119+
{
120+
headers: {
121+
"content-type": "application/json",
122+
vary: "*",
123+
},
124+
},
125+
)
126+
},
127+
}
128+
129+
const store = memoryStore()
130+
const m = withCache(createMisina({ driver, retry: 0 }), { store, ttl: 60_000 })
131+
132+
await m.get("https://api.test/")
133+
await m.get("https://api.test/")
134+
135+
expect(calls).toBe(2)
136+
})
137+
})

0 commit comments

Comments
 (0)