Skip to content

Bun.serve: treat empty header values as absent so auto Content-Type/Date are not duplicated - #35336

Open
robobun wants to merge 7 commits into
mainfrom
farm/c48b394b/serve-empty-header-dup
Open

Bun.serve: treat empty header values as absent so auto Content-Type/Date are not duplicated#35336
robobun wants to merge 7 commits into
mainfrom
farm/c48b394b/serve-empty-header-dup

Also skip empty uncommon headers so dynamic and static/file paths agree

3d70b96
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 24, 2026 in 31m 36s

Code review found 3 potential issues

Found 5 candidates, confirmed 3. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 3
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/jsc/FetchHeaders.rs:146-150 put_default change makes Response.json diverge from Fetch spec for empty init content-type
🟡 Nit src/jsc/bindings/NodeHTTP.cpp:855-857 Set-Cookie loop not given isEmpty() skip → dynamic and static paths still diverge
🟡 Nit src/runtime/server/StaticRoute.rs:244-247 Remaining key-presence auto-header gates not converted to value-non-empty (etag, Content-Disposition, Content-Range)

Annotations

Check warning on line 150 in src/jsc/FetchHeaders.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

put_default change makes Response.json diverge from Fetch spec for empty init content-type

Unlike the other changes in this PR (which are scoped to the wire serializer), `put_default` is called at **Response construction time** by `Response.json` (Response.rs:1015), so switching it from `fast_has` to `fast_get(...).is_some()` mutates the JS-observable `Headers`: `Response.json({a:1}, {headers:{'content-type':''}}).headers.get('content-type')` now returns `'application/json;charset=utf-8'` instead of `''` as the Fetch spec (and Node/Chrome/Firefox) require. Not blocking — nobody realis

Check warning on line 857 in src/jsc/bindings/NodeHTTP.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

Set-Cookie loop not given isEmpty() skip → dynamic and static paths still diverge

The `getSetCookieHeaders()` loop at the top of `writeFetchHeadersToUWSResponse` (line 780) and `writeFetchHeadersToH3Response` (line 1536) is a third sibling that didn't get the `isEmpty()` skip, so `{headers:{'set-cookie':''}}` on the dynamic path still emits an empty `set-cookie: ` line while the static/file path (whose `copyTo` snapshot includes Set-Cookie entries and now hits the `value.length == 0` skip) drops it — the exact dynamic/static divergence 3d70b967 was closing. Set-Cookie has no 

Check warning on line 247 in src/runtime/server/StaticRoute.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Remaining key-presence auto-header gates not converted to value-non-empty (etag, Content-Disposition, Content-Range)

A few sibling presence checks that gate other auto-headers weren't converted from key-presence to value-non-empty, so with the new empty-value serializer skip an empty user value now suppresses the auto-header *and* is dropped from the wire, leaving the response with neither: `headers.get(b"etag").is_none()` at `StaticRoute.rs:240` and `:98` (auto-ETag), and `fast_has(ContentDisposition/ContentRange)` at `RequestContext.rs:3633-3634` / `:1834` (auto `filename=…` and auto `Content-Range` on the d