Bun.serve: serve HEAD requests with the GET handler in per-method route objects - #32822
Conversation
A route value of the form `{ GET: handler }` only registers the declared
method in the router, so a HEAD request to that path falls through to the
next matching route (returning a different representation than GET) or
404s when nothing else matches. Every other route form already derives
HEAD from GET.
When a per-method route object has a GET handler and no HEAD entry,
register the GET handler under HEAD as well. The existing HEAD response
path then strips the body and reports the Content-Length a GET would have
sent (RFC 9110 section 9.3.2). An explicit HEAD handler still takes
precedence, and other methods still fall through to later routes.
|
Warning Review limit reached
More reviews will be available in 27 minutes and 11 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThe PR changes server routing so HEAD is only registered where supported, derives implicit HEAD routes from GET in route-object parsing, and adds tests for explicit, implicit, and missing-GET HEAD behavior. ChangesImplicit HEAD routing
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 7:26 AM PT - Jun 27th, 2026
❌ @autofix-ci[bot], your commit 8af6df7 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 32822That installs a local version of the PR into your bun-32822 --bun |
There was a problem hiding this comment.
I didn't find any issues — the logic and tests look correct — but this is a user-visible routing behavior change in Bun.serve, so it's worth a maintainer confirming the semantics (e.g. that the GET handler should observe req.method === "HEAD" rather than having the method rewritten).
Extended reasoning...
Overview
This PR modifies ServerConfig::from_js in src/runtime/server/ServerConfig.rs so that when a per-method route object ({ GET: handler, ... }) defines a callable GET handler but no HEAD entry, an implicit HEAD user route is registered that shares the GET callback. It also adds a new describe block with five tests in test/js/bun/http/bun-serve-routes.test.ts covering fall-through, 404, observed method, explicit-HEAD precedence, and no-GET cases.
The implementation is small (~30 lines net): it hoists with_async_context_if_needed into a local callback (JSValue is Copy), tracks derived_head_route and has_head_route while iterating the fixed METHODS array (GET precedes HEAD in iteration order, so ordering is correct), and pushes the derived builder after the loop only if no explicit HEAD was found. has_head_route is set for both callable HEAD handlers and static-Response HEAD entries, so explicit HEAD always wins. If the derived route goes unused, its Strong is dropped normally. Static (non-callable) GET entries are intentionally not handled here because apply_static_route already registers a HEAD handler unconditionally.
Security risks
None identified. The change only adds a route registration that reuses an already-trusted user-provided handler; it doesn't touch parsing, auth, TLS, or request body handling. If anything it improves correctness for intermediaries that probe with HEAD.
Level of scrutiny
Medium. The diff is mechanically simple and well-tested, but it changes user-visible dispatch semantics in Bun.serve, a production-critical API. Previously HEAD /m on { GET: ... } fell through to the next route or 404'd; now it invokes the GET handler with req.method === "HEAD". That's RFC-9110-correct and consistent with every other Bun.serve route form, but it is a behavior change a maintainer should sign off on — particularly the choice to surface "HEAD" to the handler (matching plain-function routes) rather than transparently rewriting to "GET".
Other factors
No CODEOWNERS entries cover these paths. No prior human review comments. The bug-hunting system found nothing. The PR description is thorough and the test suite results reported there are consistent with the change being correct. I'm deferring solely because this is a semantic change to core routing, not because of any concern with the implementation.
|
On the Rewriting the method to The "the GET handler observes the real request method" test locks the choice in. |
|
CI status: the diff is green on every lane that actually runs. Across every build of this PR (65334, 65395, 65402, 65416), the only failing job is The same job fails the same way on unrelated branches, and everything else (280+ jobs per build) passes. This is the macOS agent pool, not this change; retrying that one job from Buildkite once the agents can download artifacts again is all that is needed. |
apply_static_route registered a HEAD handler for every static route entry regardless of the methods it was declared for. In a per-method route object that mixes a GET handler with a static Response under a different key, that HEAD handler is registered after the user routes and replaces them in the router, so HEAD returned the other method's representation instead of GET's. A static Response under a single non-GET key also answered HEAD on its own. Register the static HEAD handler only when the entry serves any method, GET, or HEAD itself, in both the HTTP/1 and HTTP/3 registration paths.
…dler apply_static_route registers its HEAD handler after the user routes, and uWS keeps the last registration for a method and path, so a per-method route object with a static GET Response and a callable HEAD handler served HEAD from the static GET entry instead of the declared handler. Skip the static entry's HEAD registration when a HEAD handler route already exists for the path.
A
routesvalue of the per-method object form never answers HEAD unless aHEADkey is spelled out. A HEAD request to that path is served by whatever matches next, so HEAD and GET on the same URL return different representations, or it 404s when nothing else matches.Repro
RFC 9110 section 9.3.2 requires HEAD to return the same header fields a GET of the same target would. Every other
Bun.serveroute form already derives HEAD from GET: plain function routes (registered for any method), staticResponse/Bun.fileroutes (which register a dedicated HEAD handler), and thefetchfallback. Only the per-method object form is missing it, and an intermediary that caches based on a HEAD probe gets a different answer than GET.Cause
Three sites, same class. Note that
HttpRouter::addremoves an existing handler for the same method, pattern, and priority before inserting, so the last registration for a method and path wins, andset_routesregisters static routes after user routes.ServerConfig::from_jsturns each key of a per-method route object into a user route registered for that one method only. Nothing registers the path under HEAD, so the router never matches it.apply_static_routeregistered a HEAD handler for every static entry regardless of its declared methods, so a staticResponseunder a non-GET key both answers HEAD on its own and captures HEAD away from a sibling GET handler:Responseunder the GET key silently displaced an explicit callable HEAD handler on the same path:Fix
method == HEAD, so the existing HEAD rendering path strips the body and reports the Content-Length GET would have produced; the handler observesreq.method === "HEAD", the same as a plain function route does today.apply_static_route(and its HTTP/3 twin) register the implicit HEAD handler only for an entry that serves any method, GET, or HEAD, and never for a path that already has an explicit HEAD handler route. A staticResponseunder a non-GET, non-HEAD key no longer answers or captures HEAD, and a static GETResponseno longer displaces a declared HEAD handler.With that, an explicit
HEADkey, handler or staticResponse, always takes precedence, and other methods are unchanged: they still fall through to later routes, which is how per-method routes compose with a"/*"catch-all.Verification
New
describe("implicit HEAD for per-method route objects")intest/js/bun/http/bun-serve-routes.test.tswith nine tests. Six fail on the unmodified build:"/*"catch-all instead of the GET handlerreq.methodand Content-Length the GET handler observes{ GET: handler, POST: new Response(...) }answers HEAD with the POST response's framing{ POST: new Response(...) }answers HEAD at all{ GET: new Response(...), HEAD: handler }drops the explicit HEAD handlerThe other three are positive controls: an explicit HEAD handler over a GET handler, an explicit static HEAD
Response, and a route object with no GET handler.bun-serve-routes.test.ts(50),bun-serve-static.test.ts(34),serve-http3.test.ts(45),bun-serve-file.test.ts(66),serve-if-none-match.test.ts(17), andbun-serve-html-manifest.test.ts(4) all pass.serve.test.tsandbun-server.test.tshave the same failures as the unmodified build (environment dependent: IPv6, privileged ports).Related: #32800 fixes HEAD response framing (what goes on the wire for a HEAD response). This fixes HEAD route dispatch (which handler a HEAD request reaches). The two change disjoint files. #32823 fixes the route-parameter percent-decoder, reported together with this but independent.