Skip to content

Commit 53d6fae

Browse files
committed
fix(api): handle whitespace around q-parameter in Accept negotiation
Round 2 review (#82): CodeRabbit flagged that the round-1 q parsing used `param.startsWith("q=")` which only matches the compact form. Per RFC 7230 §3.2.6, optional whitespace is permitted around the `=` in `parameter = token "=" ( token / quoted-string )`, so a strict client may send `Accept: text/event-stream ; q = 0`. The prior fix treated such forms as "no q parameter present" and returned true, incorrectly opting the client into streaming. Replaced startsWith check with an indexOf("=") + slice + trim pass: parse name and value separately so whitespace around the delimiter no longer hides q. Empty `q=` value now correctly resolves to false (Number("") = 0, fails q > 0). Verified before/after: - text/event-stream ; q = 0 → was true, now false - text/event-stream ; q = 0.5 → was true (default-q path), now true (real q parsed) - text/event-stream;q=0 → still false (round-1 case) - text/event-stream;q= → was true (empty slice), now false Tests: api-helpers.test.ts +3 cases covering whitespace q=0, whitespace q=0.5, and empty q value. 12 → 15 cases.
1 parent f30fe7a commit 53d6fae

2 files changed

Lines changed: 25 additions & 5 deletions

File tree

backend/src/utils/api-helpers.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ describe("acceptsEventStream", () => {
4646
expect(acceptsEventStream(h("text/event-stream;q=0.0"))).toBe(false);
4747
});
4848

49+
test("q = 0 with whitespace around = rejects SSE", () => {
50+
expect(acceptsEventStream(h("text/event-stream ; q = 0"))).toBe(false);
51+
});
52+
53+
test("q = 0.5 with whitespace around = is accepted", () => {
54+
expect(acceptsEventStream(h("text/event-stream ; q = 0.5"))).toBe(true);
55+
});
56+
57+
test("malformed empty q value is treated as not acceptable", () => {
58+
expect(acceptsEventStream(h("text/event-stream;q="))).toBe(false);
59+
});
60+
4961
test("q=0 in a weighted list rejects SSE", () => {
5062
expect(
5163
acceptsEventStream(h("text/event-stream;q=0, application/json")),

backend/src/utils/api-helpers.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,20 @@ export function acceptsEventStream(headers: Headers): boolean {
6464
if (mediaType !== "text/event-stream") {
6565
return false;
6666
}
67-
const qParam = params.find((p) => p.startsWith("q="));
68-
if (qParam === undefined) {
69-
return true;
67+
for (const param of params) {
68+
const eq = param.indexOf("=");
69+
if (eq === -1) {
70+
continue;
71+
}
72+
const name = param.slice(0, eq).trim();
73+
if (name !== "q") {
74+
continue;
75+
}
76+
const value = param.slice(eq + 1).trim();
77+
const q = Number(value);
78+
return Number.isFinite(q) && q > 0;
7079
}
71-
const q = Number(qParam.slice(2));
72-
return Number.isFinite(q) && q > 0;
80+
return true;
7381
});
7482
}
7583

0 commit comments

Comments
 (0)