Bun.serve: HTTP/3 (QUIC) support via h3: true#29768
Merged
Merged
Conversation
Vendors lsquic + ls-qpack, rewrites the usockets QUIC layer with per-context
state, and adds an Http3{App,Response,Request} that mirrors the HTTP/1.1 API
through a new `uws_h3_*` C ABI and `uws.H3` Zig bindings. `h3: true` (with
`tls`) listens on the same UDP port; `h1: false` makes it H3-only.
…handler
NewRequestContext gains a comptime `http3: bool` parameter and each TLS
server type gets an H3RequestContext instantiation alongside the regular
one. onH3Request/onH3UserRouteRequest are now thin wrappers over the same
generic dispatch path, so ReadableStream bodies, Bun.file, cookies, range
requests and the promise paths all work over QUIC with no transport-
specific Zig.
- streams.zig: HTTPServerWritable(ssl, http3) + H3ResponseSink (jssink
codegen extended). Sink finalize no longer clears onAborted/onData it
never owned.
- AnyRequestContext: hand-rolled switches collapsed into one dispatch()
helper over the type map; H3 contexts return null from getRequest()
since URL/headers are populated eagerly.
- FetchHeaders__toUWSResponse / CookieMap__write take an int kind
(0=TCP, 1=SSL, 2=H3); new createFromH3 builds headers from
Http3Request.
- ZigGlobalObject: 8 new Bun__HTTPRequestContext{,Debug}H3 promise
handlers replace Bun__H3Handler__*.
- Http3ResponseData keeps a separate writableUserData slot — QUIC
reports backpressure on tryEnd where corked TCP never does, so the
sink and the request context can't share one userData.
- Http3Context fires onAborted on stream-close (post-FIN cleanup) so the
RequestContext drops its *Response before lsquic frees the stream.
- quic.c: packets_out batches sendmmsg(64) on Linux.
- test: fetch-h3.ts curl wrapper + serve-protocols.test.ts runs the same
assertions for http/1.1 and http/3; serve-http3.test.ts covers route
params and Bun.file.
Adds uws.AnyRequest (h1/h3 union with inline-else dispatch for header, method, url, setYield, dateForHeader) so StaticRoute/FileRoute/Range take an explicit transport-agnostic request instead of *uws.Request. applyStaticRouteH3 registers the same route entries on h3_app for the .static and .file cases (.html stays H1-only — dev-server entangled). Tests cover GET/304-ETag for static routes and full + Range/206 for Bun.file routes over HTTP/3.
…ial tests
The double-await test (Promise.resolve() + Bun.sleep(0) before responding)
exposed a heap-use-after-free: Http3Response::tryEnd → internalEnd →
markDone → us_quic_stream_kick → lsquic_engine_process_conns can fire
on_close and free `this` synchronously when the client has already FIN'd.
The next read of hasResponded() (and the post-tryEnd clearOnWritable in
the C shim) hit freed memory under ASan.
Fix: short-circuit `{ok, ok || hasResponded()}` — when ok is true,
markDone has already cleared HTTP_RESPONSE_PENDING. Drop the redundant
clearOnWritableAndAborted in uws_h3_res_try_end; markDone nulls those.
Adds 14 adversarial tests to serve-http3.test.ts: 64 concurrent on one
conn, 7KB+50×100B headers, 8MB POST echo, slow-read backpressure,
204→200 pipelining, HEAD content-length, lying content-length, RST
mid-response, 8×96KB and 3×300KB per-stream-unique transformed echoes,
Response(subprocess.stdout), Response(req.body) passthrough,
Response(Bun.file().stream()), and req.{url,method,headers,params}
across micro/macro/double await boundaries.
- server.reload(): clear h3_app routes alongside the H1 router so stale UserRoute/StaticRoute pointers don't survive in the H3 HttpRouter. - quic.c: defer free(ls) until us_quic_socket_context_free; the engine, timer, and live conns still hold ls as peer_ctx after stop(). Cooldown + flush before closing the UDP socket; guard ls->udp == NULL in packets_out. - FileResponseStream: canSendfile() returns false for .H3 (no socket fd). Large Bun.file() responses now take the reader path instead of sendfile(invalid_fd) → EBADF. - req.remoteAddress over H3: format via ares_inet_ntop and unwrap IN6_IS_ADDR_V4MAPPED in us_quic_socket_remote_address; the shim was returning raw in_addr bytes and h3.zig left .ip.len undefined. - H3 request bodies without Content-Length: arm onData unconditionally for H3 and skip onStartBuffering's CL==0 && !TE → .Null shortcut — RFC 9114 makes CL optional and forbids TE; body end is the stream FIN. - Windows H3ResponseSink: instantiate the generic on every platform so H3ResponseSink__* are exported (UWSResponse falls back to the SSL type on Windows where the sink is unreachable). - loop.c UDP dispatch: clear WRITABLE before on_drain instead of after, so a callback that re-arms it (QUIC packets_out hitting EAGAIN) keeps the re-arm. Removes the 1 s send stall under sustained backpressure. Tests: reload-then-hit-old-route, stop-then-probe, 2 MB file md5, requestIP, curl -T - body without CL.
- Security limits in lsquic settings: es_max_header_list_size=16K, explicit es_init_max_streams_bidi=100, idleTimeout plumbed to es_idle_to via the App.create signature. - Graceful stop: us_quic_socket_context_shutdown sets closing, lsquic_engine_cooldown (GOAWAY every conn, drop mini conns), flush; on_new_conn rejects during drain. server.stopListening calls it for the non-abrupt path so in-flight H3 requests complete. - Auto Alt-Svc: H1/H2 responses on an h3-enabled server emit alt-svc: h3=":<port>"; ma=86400 (cached on the server, skipped if the user already set one). - Transfer-Encoding rejected with 400 per RFC 9114 §4.2 (defense in depth — compliant clients strip it). - SNI: us_quic_socket_context_add_server_name builds a per-hostname SSL_CTX and ea_lookup_cert linear-scans on the SNI string; mirrored from the H1 sni: config. - HTMLBundle.Route over H3: onAnyRequest takes uws.AnyRequest; the DevServer HMR path stays H1-only. - server.upgrade() over H3 returns false cleanly (upgrade_context is never set on H3RequestContext). - Unix-socket address: H3 listener is skipped with a warning (QUIC over AF_UNIX is non-standard and Alt-Svc can't advertise it). - us_quic_stream_send_informational added for future 1xx support (Bun.serve has no Expect: handling on any transport yet). Tests: graceful stop polls server-side in-flight count instead of sleeping (cooldown drops still-handshaking conns), req.signal abort on client RST, Alt-Svc on TCP fetch, upgrade=false.
Collaborator
|
Updated 10:08 AM PT - Apr 27th, 2026
❌ @Jarred-Sumner, your commit 3addffd has 2 failures in
Add 🧪 To try this PR locally: bunx bun-pr 29768That installs a local version of the PR into your bun-29768 --bun |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds HTTP/3 (QUIC) support across the stack: new Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/api/NativePromiseContext.zig (1)
32-46:⚠️ Potential issue | 🟠 MajorAdd alignment assertions for the new packed H3 context pointers.
Line 32 introduces new tags that are packed into low pointer bits, but the compile-time alignment guard list (Line 117 onward) was not extended for these new pointer types. Please add matching asserts to keep packing safety explicit.
Suggested patch
comptime { // Low 3 bits hold the tag; verify both capacity and alignment // slack so adding a tag or a packed field can't silently break // the packing. bun.assert(`@typeInfo`(Tag).@"enum".fields.len <= tag_mask + 1); bun.assert(`@alignOf`(server.HTTPServer.RequestContext) > tag_mask); bun.assert(`@alignOf`(server.HTTPSServer.RequestContext) > tag_mask); bun.assert(`@alignOf`(server.DebugHTTPServer.RequestContext) > tag_mask); bun.assert(`@alignOf`(server.DebugHTTPSServer.RequestContext) > tag_mask); bun.assert(`@alignOf`(bun.webcore.Body.ValueBufferer) > tag_mask); + if (!bun.Environment.isWindows) { + bun.assert(`@alignOf`(server.HTTPSServer.H3RequestContext) > tag_mask); + bun.assert(`@alignOf`(server.DebugHTTPSServer.H3RequestContext) > tag_mask); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/api/NativePromiseContext.zig` around lines 32 - 46, The new tags HTTPSServerH3RequestContext and DebugHTTPSServerH3RequestContext are packed into low pointer bits but the compile-time alignment assertions for packed pointer safety were not updated; add alignment asserts for server.HTTPSServer.H3RequestContext and server.DebugHTTPSServer.H3RequestContext in the same compile-time guard block used for NativePromiseContext.Tag (the existing alignment/assert list around the compile-time guards) so their alignment is verified (same style as the other `@alignOf/`@compileTimeAssert checks) to ensure the pointer-tagging safety is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bun-usockets/src/quic.c`:
- Around line 471-478: The static int once check around
lsquic_global_init/lsquic_logger_init is racy; replace the ad-hoc flag with a
thread-safe one-time init (e.g., use pthread_once with a dedicated initializer
function or an atomic_flag/C11 call_once) so the initialization code including
lsquic_global_init, lsquic_logger_init(&us_quic_logger, ...), and
lsquic_set_log_level("debug") runs exactly once across threads; implement a
static init function (e.g., quic_global_init) containing the existing init block
and invoke it via pthread_once(&once, quic_global_init) or call_once equivalent
instead of the current `static int once` check.
- Around line 52-53: us_quic_socket_context_free currently only iterates
closed_listeners so live listeners created by us_quic_socket_context_listen
remain untracked, leaving UDP fds with ls->ctx pointing at freed memory; fix by
tracking active listeners on the context (e.g. add a live_listeners list or
unify into a single listeners list), ensure us_quic_socket_context_listen
inserts the new struct us_quic_listen_socket_s into that list and that
listener-close/remove routines unlink from it, and update
us_quic_socket_context_free to iterate and properly close or detach all
listeners (set ls->ctx = NULL before freeing/closing the UDP fd) to prevent fd
leaks and use-after-free on UDP callbacks.
In `@packages/bun-uws/src/Http3Request.h`:
- Around line 51-56: getMethod() currently lowercases by blindly OR-ing bytes
and writes into fixed-size char array methodLower which corrupts non A-Z tokens
and truncates longer custom methods; change the storage to a std::string member
(e.g., methodLower as std::string) and implement safe ASCII-lowercasing only for
'A'..'Z' characters while preserving others and resizing methodLower to
method.size() so no truncation occurs; update getMethod() and the other similar
occurrence (the other getMethod-like call at the second occurrence) to produce
and return a string_view pointing at the std::string's data after proper
resizing and lowercasing.
In `@packages/bun-uws/src/Http3Response.h`:
- Around line 38-41: The numeric header formatting truncates max uint64_t
because the temporary buffer is only 20 bytes; in Http3Response::writeHeader
(and the analogous HttpResponse::writeHeader) increase the buffer to at least 21
bytes (20 digits + NUL) or, better, replace snprintf with a non-truncating
conversion (e.g., std::to_chars or std::to_string) to produce the full decimal
representation before calling writeHeader(key, std::string_view{...}). Ensure
the conversion yields the complete string for values up to 18446744073709551615.
In `@patches/lsquic/versions-to-string.patch`:
- Around line 100-163: The vers_2_h3_alnps table contains entries with
LSQVER_I002 that advertise an empty ALPN token (""), which lsquic_alpn2ver()
cannot map back and empty ALPNs are invalid per RFC7301; update the table so
every entry that currently pairs LSQVER_I002 with "" instead advertises "h3"
(the same ALPN used for LSQVER_I001) or remove LSQVER_I002 from H3 ALPN entries,
ensuring reverse lookup in lsquic_alpn2ver() succeeds for the LSQVER_I002 mask.
In `@src/bun.js/api/server.zig`:
- Around line 1329-1333: The code only treats h3_listener specially for
server.port/stopListening; update the server to treat h3_listener as a
first-class listener everywhere by adding a small helper (e.g.,
getActiveListener/getActivePort or normalizeListener) that returns the effective
listener or bound port prioritizing this.h3_listener when present, falling back
to this.listener and then this.config.address.tcp.port; then use that helper
from stopFromJS(), disposeFromJS(), getAllClosedPromise(), deinitIfWeCan(),
getAddress(), getURLAsString(), and any code paths that check listener presence
(including stopListening/server.port) so H3-only servers report the correct
bound UDP port and are stopped/disposed correctly.
In `@src/bun.js/api/server/AnyRequestContext.zig`:
- Around line 122-129: The onAbort implementation in AnyRequestContext currently
unsafely `@ptrCast`'s every uws.AnyResponse arm to *T.Resp, risking silent UB if
variants are mismatched; update AnyRequestContext.onAbort (the dispatched
function f) to perform a variant-safe switch on the incoming uws.AnyResponse
arms, only cast the matching arm to *T.Resp and call ctx.onAbort, and explicitly
handle non-matching arms with a fail-fast failure (e.g., a panic/assert or a
tagged-union mismatch error) so a context/response type mismatch is detected
immediately rather than producing undefined behavior.
In `@src/bun.js/api/server/HTMLBundle.zig`:
- Around line 148-154: The .h3 branch currently ends the response with
resp.endWithoutBody(true), causing H3 HTML-bundle requests to return blank
responses; instead route H3 through the same bundle-render path as H1 by
invoking bun.handleOom(dev.respondForHTMLBundle(this, h3, resp)) (or equivalent)
so the HTML bundle render runs for H3 as it does for H1; update the switch on
req (the .h3 arm) to call dev.respondForHTMLBundle with the h3 request value and
pass the result into bun.handleOom rather than terminating the response.
In `@src/bun.js/api/server/RequestContext.zig`:
- Around line 2283-2288: doWriteHeaders currently strips Transfer-Encoding only
on the normal metadata path but doRenderHeadResponse still emits user-supplied
or synthesized transfer-encoding for HEAD responses over H3; update
RequestContext::doRenderHeadResponse to also remove any Transfer-Encoding (e.g.,
call headers.fastRemove(.TransferEncoding)) before writing the response when
handling a HEAD method or when resp_kind/version indicates HTTP/3, and stop
synthesizing "transfer-encoding: chunked" for locked/empty bodies in that
HEAD+H3 code path so no RFC9114-invalid header is sent.
In `@src/bun.js/api/server/ServerConfig.zig`:
- Around line 248-266: applyStaticRouteH3 fails to set entry.server so routes
registered for H3 keep server == null; update applyStaticRouteH3 to initialize
entry.server before registering handlers (e.g., add a server parameter or
retrieve the server from the app if available), then set entry.server = server
just prior to calling app.head / app.any / app.method so HTMLBundle.Route sees a
non-null server and timeout/pending-request tracking works correctly; make this
change in the applyStaticRouteH3 function and ensure the same entry.server
assignment pattern used in applyStaticRoute is mirrored here.
In `@src/bun.js/bindings/CookieMap.cpp`:
- Around line 30-41: The switch in CookieMap.cpp unsafely casts arg2 on the
default branch; instead of reinterpreting unknown kinds as
uWS::HttpResponse<false> you must explicitly handle invalid kinds by returning
or logging an error; update the switch over kind in the function that calls
CookieMap__writeFetchHeadersToUWSResponse so case 1 and (when LIBUS_USE_QUIC)
case 2 call CookieMap__writeFetchHeadersToUWSResponse with the correct cast
(uWS::HttpResponse<true>* or uWS::Http3Response*), and change the default to
either call an error handler/abort/return without casting arg2 (or assert on
unexpected kind) to avoid wrong-type dereference of arg2. Ensure references to
kind, arg2, and CookieMap__writeFetchHeadersToUWSResponse are used so the
reviewer can locate the fix.
In `@src/bun.js/bindings/NodeHTTP.cpp`:
- Around line 1062-1079: The HTTP/3 path in writeFetchHeadersToH3Response() is
currently emitting connection-specific headers that are forbidden by RFC 9114;
update the loops over internalHeaders.commonHeaders() and
internalHeaders.uncommonHeaders() to skip any header whose name is "Connection",
"Keep-Alive", "Proxy-Connection", or "Upgrade" before calling writeOne(), and
retain the existing logic for Content-Length and Date; reference the
functions/structures internalHeaders.commonHeaders(),
internalHeaders.uncommonHeaders(), writeOne(), doWriteHeaders(), and
writeFetchHeadersToH3Response() so the check is applied on both common and
uncommon header iterations (optionally log or assert if such a forbidden header
is encountered).
In `@src/deps/libuwsockets_h3.cpp`:
- Around line 215-250: The getters (uws_h3_req_get_url, uws_h3_req_get_method,
uws_h3_req_get_header, uws_h3_req_get_query, uws_h3_req_get_parameter) and the
for-each callback currently write v.data() (or name.data()/value.data()) even
when the std::string_view is empty, which can yield a nullptr; normalize empty
views by writing a non-null pointer (e.g. a static empty string literal like ""
or a static const char empty[] = "") when v.empty() (or
name.empty()/value.empty()) so *dest and the callback arguments are never NULL
while preserving length 0; update each function and the lambda in
uws_h3_req_for_each_header to perform this check and assign the empty-string
pointer when needed.
In `@src/deps/uws/h3.zig`:
- Around line 37-43: The dateForHeader function can trap when `@intFromFloat` is
called on negative milliseconds; after obtaining ms via s.parseDate (in
Request.dateForHeader), add a guard that ms is non-negative (e.g., if
std.math.isNan(ms) or !std.math.isFinite(ms) or ms < 0 then return null) before
calling `@intFromFloat` so negative (pre-1970) parsed dates do not crash the
server.
In `@test/js/bun/http/fetch-h3.ts`:
- Around line 97-117: The code should synchronously reject if the provided
AbortSignal is already aborted before launching curl: add a check for
init.signal?.aborted immediately before the Bun.spawn(...) call in fetchH3
(i.e., before the proc is created) and throw new DOMException("aborted",
"AbortError") to mirror fetch() behavior; keep the existing addEventListener
cleanup for future aborts but ensure you do not call Bun.spawn when the signal
is already aborted.
In `@test/js/bun/http/serve-http3.test.ts`:
- Around line 789-825: withCustomServer currently drains proc.stderr fully
(first to find PORT and then via a background loop), stealing bytes that callers
expect to read (e.g., RELOADED). Fix by performing only a temporary, bounded
read for the startup handshake inside withCustomServer (like withServer does):
iterate proc.stderr just until you match /PORT=(\d+)/, then stop and do NOT
start any background drain; release/close the temporary reader so the stderr
stream remains available for the caller to consume later. Keep references to the
function name withCustomServer and the proc.stderr handling so reviewers can
locate the change.
---
Outside diff comments:
In `@src/bun.js/api/NativePromiseContext.zig`:
- Around line 32-46: The new tags HTTPSServerH3RequestContext and
DebugHTTPSServerH3RequestContext are packed into low pointer bits but the
compile-time alignment assertions for packed pointer safety were not updated;
add alignment asserts for server.HTTPSServer.H3RequestContext and
server.DebugHTTPSServer.H3RequestContext in the same compile-time guard block
used for NativePromiseContext.Tag (the existing alignment/assert list around the
compile-time guards) so their alignment is verified (same style as the other
`@alignOf/`@compileTimeAssert checks) to ensure the pointer-tagging safety is
explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 36a6bf62-d0b1-47cf-9b2b-6364e88357fd
📒 Files selected for processing (49)
packages/bun-types/serve.d.tspackages/bun-usockets/src/loop.cpackages/bun-usockets/src/quic.cpackages/bun-usockets/src/quic.hpackages/bun-uws/src/Http3App.hpackages/bun-uws/src/Http3Context.hpackages/bun-uws/src/Http3ContextData.hpackages/bun-uws/src/Http3Request.hpackages/bun-uws/src/Http3Response.hpackages/bun-uws/src/Http3ResponseData.hpatches/lsquic/allow-no-sni.patchpatches/lsquic/versions-to-string.patchscripts/build/deps/index.tsscripts/build/deps/lsqpack.tsscripts/build/deps/lsquic.tssrc/bake/DevServer.zigsrc/bun.js/api/NativePromiseContext.zigsrc/bun.js/api/server.zigsrc/bun.js/api/server/AnyRequestContext.zigsrc/bun.js/api/server/FileResponseStream.zigsrc/bun.js/api/server/FileRoute.zigsrc/bun.js/api/server/HTMLBundle.zigsrc/bun.js/api/server/NodeHTTPResponse.zigsrc/bun.js/api/server/RangeRequest.zigsrc/bun.js/api/server/RequestContext.zigsrc/bun.js/api/server/ServerConfig.zigsrc/bun.js/api/server/StaticRoute.zigsrc/bun.js/bindings/CookieMap.cppsrc/bun.js/bindings/FetchHeaders.zigsrc/bun.js/bindings/NativePromiseContext.hsrc/bun.js/bindings/NodeHTTP.cppsrc/bun.js/bindings/ServerRouteList.cppsrc/bun.js/bindings/Sink.hsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/ZigGlobalObject.hsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/headers.hsrc/bun.js/webcore.zigsrc/bun.js/webcore/CookieMap.zigsrc/bun.js/webcore/streams.zigsrc/codegen/generate-jssink.tssrc/deps/libuwsockets_h3.cppsrc/deps/uws.zigsrc/deps/uws/Request.zigsrc/deps/uws/Response.zigsrc/deps/uws/h3.zigtest/js/bun/http/fetch-h3.tstest/js/bun/http/serve-http3.test.tstest/js/bun/http/serve-protocols.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/bun.js/api/server/RequestContext.zig (1)
2283-2288:⚠️ Potential issue | 🟠 MajorHEAD responses can still emit
Transfer-Encodingover H3.These removals only affect the normal metadata path.
doRenderHeadResponse()still forwards a user-suppliedTransfer-Encodingheader at Lines 1364-1369 and still synthesizestransfer-encoding: chunkedfor.Lockedbodies at Lines 1425-1428, so HEAD over HTTP/3 can remain RFC 9114-invalid.Suggested fix
fn doRenderHeadResponse(pair: *HeaderResponsePair) void { var this = pair.this; var response = pair.response; @@ if (response.getFetchHeaders()) |headers| { + if (comptime http3) headers.fastRemove(.TransferEncoding); // first respect the headers if (headers.fastGet(.TransferEncoding)) |transfer_encoding| { const transfer_encoding_str = transfer_encoding.toSlice(server.allocator); defer transfer_encoding_str.deinit(); this.renderMetadata(); @@ }, .Locked => { this.renderMetadata(); - resp.writeHeader("transfer-encoding", "chunked"); + if (comptime !http3) { + resp.writeHeader("transfer-encoding", "chunked"); + } this.endWithoutBody(this.shouldCloseConnection()); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/api/server/RequestContext.zig` around lines 2283 - 2288, doRenderHeadResponse currently forwards user-supplied Transfer-Encoding and synthesizes transfer-encoding: chunked for Locked bodies, which allows HEAD responses to violate RFC 9114; update doRenderHeadResponse to sanitize headers the same way doWriteHeaders does by removing Transfer-Encoding (and Content-Length if appropriate) before forwarding, and change the Locked-body synthesis logic (the code path that creates "transfer-encoding: chunked" for Locked bodies) to skip generating chunked for HEAD responses or when rendering a HEAD response; reference doRenderHeadResponse, doWriteHeaders, and the Locked body synthesis code path to locate and apply these header-removal and conditional-synthesis changes.test/js/bun/http/serve-http3.test.ts (1)
804-816:⚠️ Potential issue | 🟠 MajorStop double-consuming
proc.stderrinwithCustomServer.This helper reads
proc.stderrto findPORT=..., then starts a second background drain on the same stream. Callers like Lines 857-860 also iterateproc.stderrforRELOADED, so those bytes can be stolen and the lifecycle tests can hang nondeterministically.Suggested fix
let port = 0; - let buf = ""; - for await (const chunk of proc.stderr) { - buf += new TextDecoder().decode(chunk); - const m = buf.match(/PORT=(\d+)/); - if (m) { - port = Number(m[1]); - break; - } - if (buf.length > 8192) break; - } - (async () => { - for await (const _ of proc.stderr) { - } - })(); + let buf = ""; + const stderr = proc.stderr.getReader(); + while (true) { + const { value, done } = await stderr.read(); + if (done) break; + buf += new TextDecoder().decode(value); + const m = buf.match(/PORT=(\d+)/); + if (m) { + port = Number(m[1]); + break; + } + if (buf.length > 8192) break; + } + stderr.releaseLock(); expect(port).toBeGreaterThan(0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/js/bun/http/serve-http3.test.ts` around lines 804 - 816, withCustomServer currently consumes proc.stderr twice: first to find "PORT=..." in the for-await loop, then it starts a background IIFE that also iterates proc.stderr, which steals bytes needed by callers (e.g., tests that wait for "RELOADED"). Remove the background drain (the async IIFE that iterates proc.stderr) and ensure withCustomServer only reads stderr until it finds PORT and then stops; if you need to preserve subsequent stderr for tests, either buffer and expose the remaining data or let callers read proc.stderr directly instead of spawning a second iterator.src/deps/uws/h3.zig (1)
37-43:⚠️ Potential issue | 🟠 MajorGuard negative
parseDate()results before@intFromFloat.A finite pre-1970 timestamp still reaches
@intFromFloat(ms). In Zig that conversion tou64traps for negative values, so a malformed or old date header can crash this path.Proposed fix
pub fn dateForHeader(this: *Request, name: []const u8) bun.JSError!?u64 { const value = this.header(name) orelse return null; var s = bun.String.init(value); defer s.deref(); const ms = try s.parseDate(bun.jsc.VirtualMachine.get().global); - if (!std.math.isNan(ms) and std.math.isFinite(ms)) return `@intFromFloat`(ms); + if (!std.math.isNan(ms) and std.math.isFinite(ms) and ms >= 0) { + return `@intFromFloat`(ms); + } return null; }Does Zig `@intFromFloat` trap when converting a negative finite `f64` to `u64` in safe modes?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/deps/uws/h3.zig` around lines 37 - 43, In dateForHeader, guard against negative parseDate results before calling `@intFromFloat`: after computing const ms = try s.parseDate(...), keep the existing NaN/Finite checks and also ensure ms >= 0 (or ms < 0 => return null) so a pre-1970 or malformed date doesn't cause `@intFromFloat`(ms) to trap; adjust the control flow around the ms variable and the final return to return null for negative values.src/deps/libuwsockets_h3.cpp (1)
248-290:⚠️ Potential issue | 🔴 CriticalNever return nullable pointers through the Zig request-string ABI.
These getters and the header iterator forward
std::string_view::data()directly. For empty views that can be null, butsrc/deps/uws/h3.zigdeclares these as non-null[*]const u8and slices them immediately. An empty URL/query/header/parameter can therefore cross the FFI boundary as a null many-pointer and trigger UB on the Zig side.Proposed fix
+static inline const char* ffi_data(std::string_view s) +{ + return s.empty() ? "" : s.data(); +} + size_t uws_h3_req_get_url(uws_h3_req_t* req, const char** dest) { std::string_view u = ((Http3Request*)req)->getFullUrl(); - *dest = u.data(); + *dest = ffi_data(u); return u.length(); } size_t uws_h3_req_get_method(uws_h3_req_t* req, const char** dest) { std::string_view m = ((Http3Request*)req)->getMethod(); - *dest = m.data(); + *dest = ffi_data(m); return m.length(); } size_t uws_h3_req_get_header(uws_h3_req_t* req, const char* lower, size_t lower_len, const char** dest) { std::string_view v = ((Http3Request*)req)->getHeader(sv(lower, lower_len)); - *dest = v.data(); + *dest = ffi_data(v); return v.length(); } void uws_h3_req_for_each_header(uws_h3_req_t* req, void (*cb)(const char*, size_t, const char*, size_t, void*), void* user_data) { ((Http3Request*)req)->forEachHeader([cb, user_data](std::string_view name, std::string_view value) { - cb(name.data(), name.length(), value.data(), value.length(), user_data); + cb(ffi_data(name), name.length(), ffi_data(value), value.length(), user_data); }); } size_t uws_h3_req_get_query(uws_h3_req_t* req, const char* key, size_t key_len, const char** dest) { std::string_view v = key ? ((Http3Request*)req)->getQuery(sv(key, key_len)) : ((Http3Request*)req)->getQuery(); - *dest = v.data(); + *dest = ffi_data(v); return v.length(); } size_t uws_h3_req_get_parameter(uws_h3_req_t* req, unsigned short index, const char** dest) { std::string_view v = ((Http3Request*)req)->getParameter(index); - *dest = v.data(); + *dest = ffi_data(v); return v.length(); }In C++17, can `std::string_view::data()` be null for an empty or default-constructed `std::string_view`? In Zig, are many-pointers like `[*]const u8` allowed to be null, and is slicing a null many-pointer with length 0 valid?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/deps/libuwsockets_h3.cpp` around lines 248 - 290, The C++ getters and header iterator (uws_h3_req_get_url, uws_h3_req_get_method, uws_h3_req_get_header, uws_h3_req_get_query, uws_h3_req_get_parameter and uws_h3_req_for_each_header) currently forward std::string_view::data() which may be null for empty views; ensure no nullable pointer crosses the Zig ABI by returning a pointer to a static empty buffer (e.g. a single '\0' static char[]) whenever string_view.data() is null or string_view.length() == 0, and use that same non-null empty pointer in the header iterator callback for both name and value when empty. Ensure you only change pointer assignment logic (set *dest or pass pointer in cb) and preserve lengths returned/forwarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/js/bun/http/serve-protocols.test.ts`:
- Around line 32-33: The tests claim "One server fixture per protocol" but each
test.concurrent block still calls withServer(...) causing a fresh server (and
QUIC handshake) per assertion; update the test suite to create the server once
per protocol by moving the withServer(...) invocation into a per-protocol
setup/teardown (e.g., wrap the H3 protocol row's tests in a describe/serial
block that calls withServer(...) once and reuses the returned fixture for its
test.concurrent cases), or alternatively change the comment to reflect the
current behavior; locate and modify references to withServer(...) and the
test.concurrent(...) blocks in the H3 row (and similarly adjust the blocks
covering lines 122-201) so the server lifecycle is either hoisted or the comment
is trimmed to match implementation.
---
Duplicate comments:
In `@src/bun.js/api/server/RequestContext.zig`:
- Around line 2283-2288: doRenderHeadResponse currently forwards user-supplied
Transfer-Encoding and synthesizes transfer-encoding: chunked for Locked bodies,
which allows HEAD responses to violate RFC 9114; update doRenderHeadResponse to
sanitize headers the same way doWriteHeaders does by removing Transfer-Encoding
(and Content-Length if appropriate) before forwarding, and change the
Locked-body synthesis logic (the code path that creates "transfer-encoding:
chunked" for Locked bodies) to skip generating chunked for HEAD responses or
when rendering a HEAD response; reference doRenderHeadResponse, doWriteHeaders,
and the Locked body synthesis code path to locate and apply these header-removal
and conditional-synthesis changes.
In `@src/deps/libuwsockets_h3.cpp`:
- Around line 248-290: The C++ getters and header iterator (uws_h3_req_get_url,
uws_h3_req_get_method, uws_h3_req_get_header, uws_h3_req_get_query,
uws_h3_req_get_parameter and uws_h3_req_for_each_header) currently forward
std::string_view::data() which may be null for empty views; ensure no nullable
pointer crosses the Zig ABI by returning a pointer to a static empty buffer
(e.g. a single '\0' static char[]) whenever string_view.data() is null or
string_view.length() == 0, and use that same non-null empty pointer in the
header iterator callback for both name and value when empty. Ensure you only
change pointer assignment logic (set *dest or pass pointer in cb) and preserve
lengths returned/forwarded.
In `@src/deps/uws/h3.zig`:
- Around line 37-43: In dateForHeader, guard against negative parseDate results
before calling `@intFromFloat`: after computing const ms = try s.parseDate(...),
keep the existing NaN/Finite checks and also ensure ms >= 0 (or ms < 0 => return
null) so a pre-1970 or malformed date doesn't cause `@intFromFloat`(ms) to trap;
adjust the control flow around the ms variable and the final return to return
null for negative values.
In `@test/js/bun/http/serve-http3.test.ts`:
- Around line 804-816: withCustomServer currently consumes proc.stderr twice:
first to find "PORT=..." in the for-await loop, then it starts a background IIFE
that also iterates proc.stderr, which steals bytes needed by callers (e.g.,
tests that wait for "RELOADED"). Remove the background drain (the async IIFE
that iterates proc.stderr) and ensure withCustomServer only reads stderr until
it finds PORT and then stops; if you need to preserve subsequent stderr for
tests, either buffer and expose the remaining data or let callers read
proc.stderr directly instead of spawning a second iterator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43bcde2f-1b91-4bd7-acdb-ecb8127fa8ec
📒 Files selected for processing (7)
LICENSE.mdsrc/bun.js/api/server/RequestContext.zigsrc/deps/libuwsockets_h3.cppsrc/deps/uws/Request.zigsrc/deps/uws/h3.zigtest/js/bun/http/serve-http3.test.tstest/js/bun/http/serve-protocols.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- quic.c: track live listeners on the context and close them in us_quic_socket_context_free so the graceful-drain path can't leave a UDP fd whose ls->ctx points at freed memory. lsquic_global_init guarded by pthread_once instead of an unsynchronized static int. - Http3Request: keep the leading '?' in the stored query so getDecodedQueryValue (which drops one byte) sees the delimiter instead of the first param's first character. - Http3Response: 24-byte buffer for the uint64 writeHeader overload. - CookieMap__write: explicit case 0 for TCP, default → ASSERT_NOT_REACHED instead of casting an unknown kind. - RequestContext: HEAD over H3 no longer emits transfer-encoding: chunked; doWriteHeaders strips Connection/Keep-Alive/Upgrade for H3 per RFC 9114 §4.2. - dateForHeader (h3.zig and uws/Request.zig): guard ms >= 0 before @intFromFloat(u64) so a pre-epoch If-Modified-Since doesn't trap. - applyStaticRouteH3 takes the AnyServer and sets entry.server so static/file routes mirrored only to H3 still see their server. - HTMLBundle.Route over H3 with DevServer present responds 503 with a message instead of an empty body. - serve-protocols.test.ts: correct the misleading "shared fixture" comment.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/api/server/RequestContext.zig (1)
2270-2280: 🧹 Nitpick | 🔵 TrivialMinor: Fallback status reason phrase is non-descriptive.
The fallback format
"{d} HM"produces status lines like"999 HM"for unknown codes. While technically valid (HTTP doesn't mandate specific reason phrases), a more descriptive fallback like"Unknown"would be clearer for debugging. This only affects HTTP/1.x since HTTP/2+ don't transmit reason phrases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/api/server/RequestContext.zig` around lines 2270 - 2280, The fallback reason phrase in doWriteStatus (RequestContext) currently formats as "{d} HM" producing non-descriptive phrases like "999 HM"; change the fallback passed to resp.writeStatus to include a clearer phrase such as "{d} Unknown" (so unknown codes become "999 Unknown") or otherwise append "Unknown" instead of "HM". Update the std.fmt.bufPrint call in doWriteStatus to format the status code with "Unknown" and leave the existing HTTPStatusText lookup and flags logic unchanged.
♻️ Duplicate comments (3)
src/bun.js/api/server.zig (1)
1329-1333:⚠️ Potential issue | 🔴 Critical
h3_listenerstill needs to be the effective listener everywhere.
getPort()now checks UDP, but the rest of the lifecycle still keys offthis.listeneronly. With{ h1: false, h3: true },stopFromJS()/disposeFromJS()never callstop(),getAllClosedPromise()resolves immediately,deinitIfWeCan()treats the server as already closed, andgetAddress()/getURLAsString()still report the configured TCP port instead of the bound UDP port. Please centralize “active listener / bound port” lookup and reuse it across those paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/api/server.zig` around lines 1329 - 1333, The code uses this.listener directly in many lifecycle paths while getPort() was updated to consider UDP/h3_listener, causing mismatches; add a single helper (e.g., getEffectiveListener or getActiveListener) that returns the currently bound listener (checking this.listener, this.h3_listener/UDP variant, and falling back to this.config.address.tcp) and then replace direct reads of this.listener and direct port/config lookups in stopFromJS(), disposeFromJS(), getAllClosedPromise(), deinitIfWeCan(), getAddress(), getURLAsString(), and any other lifecycle code with calls to that helper so the bound port and stop() invocation are always based on the actual active listener.src/bun.js/api/server/RequestContext.zig (1)
1362-1371:⚠️ Potential issue | 🟠 MajorUser-supplied
Transfer-Encodingstill written for HTTP/3 HEAD responses.This code path writes a user-supplied
Transfer-Encodingheader directly at line 1368, bypassing the stripping indoWriteHeaders. While line 1427 correctly guards the synthesizedchunkedcase, this path still emits an RFC 9114-invalid header for H3.,
🛡️ Proposed fix to guard user-supplied TE for H3
if (headers.fastGet(.TransferEncoding)) |transfer_encoding| { + if (comptime http3) { + // RFC 9114 §4.2: Transfer-Encoding is malformed on HTTP/3 + this.renderMetadata(); + this.endWithoutBody(this.shouldCloseConnection()); + return; + } const transfer_encoding_str = transfer_encoding.toSlice(server.allocator); defer transfer_encoding_str.deinit(); this.renderMetadata(); resp.writeHeader("transfer-encoding", transfer_encoding_str.slice()); this.endWithoutBody(this.shouldCloseConnection()); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/api/server/RequestContext.zig` around lines 1362 - 1371, The user-supplied Transfer-Encoding is being written unconditionally in the response.getFetchHeaders() branch (headers.fastGet(.TransferEncoding) → resp.writeHeader("transfer-encoding", ...)), which violates HTTP/3; update that branch in RequestContext.zig to detect the connection protocol (the same HTTP/3 check used in doWriteHeaders where the synthesized "chunked" case is guarded) and if the protocol is HTTP/3 do NOT write the Transfer-Encoding header from the user-supplied headers—simply proceed to renderMetadata() and endWithoutBody(this.shouldCloseConnection()); otherwise retain the current behavior that writes the header.packages/bun-uws/src/Http3Request.h (1)
53-58:⚠️ Potential issue | 🟠 Major
getMethod()still corrupts and truncates valid HTTP methods.Blind
| 0x20folding mutates non-letters, andmethodLower[16]clips extension methods. That can misroute otherwise valid H3 requests.Proposed fix
+#include <string> + std::string_view getMethod() { - size_t n = method.size() < sizeof(methodLower) ? method.size() : sizeof(methodLower); - for (size_t i = 0; i < n; i++) { - methodLower[i] = (char) (method[i] | 0x20); + methodLower.resize(method.size()); + for (size_t i = 0; i < method.size(); i++) { + char c = method[i]; + methodLower[i] = (c >= 'A' && c <= 'Z') ? (char) (c | 0x20) : c; } - return {methodLower, n}; + return methodLower; } ... - char methodLower[16]; + std::string methodLower;Also applies to: 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bun-uws/src/Http3Request.h` around lines 53 - 58, getMethod() currently lowercases bytes blindly with (c | 0x20) and always writes into methodLower causing non-letter corruption and truncation when method length >= sizeof(methodLower). Fix by: in Http3Request::getMethod(), if method.size() >= sizeof(methodLower) return the original method string_view unchanged (avoid clipping); otherwise copy method.size() bytes into methodLower and lowercase only ASCII letters (e.g. if 'A' <= ch && ch <= 'Z' then ch |= 0x20) instead of applying |0x20 to every byte; use method, methodLower and getMethod() to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bun-usockets/src/quic.c`:
- Around line 530-531: The timer allocation can return NULL and is dereferenced
immediately; modify the context initialization around us_create_timer so that
after calling us_create_timer(loop, 1, sizeof(us_quic_socket_context_t *)) you
check if ctx->timer is NULL and handle the failure (e.g., free/cleanup ctx and
return an error) before calling us_timer_ext or writing through the pointer;
update any callers to propagate/handle the failure accordingly and ensure
references to ctx->timer and us_timer_ext only occur when ctx->timer is
non-NULL.
- Around line 543-551: The code stores strdup(hostname) directly into
ctx->sni[...] and then increments ctx->sni_count without checking for allocation
failure, which can leave a NULL name and later crash us_quic_match_sni(); fix by
strdup into a temporary (e.g., char *name = strdup(hostname)); if name is NULL,
free the newly created SSL context (SSL_CTX_free(ssl)) and return -1 without
touching ctx->sni_count or writing into ctx->sni; only on success assign
ctx->sni[ctx->sni_count].name = name; ctx->sni[ctx->sni_count].ctx = ssl;
ctx->sni_count++ so us_quic_match_sni() never sees a NULL name.
In `@src/bun.js/api/server.zig`:
- Around line 2879-2890: The H3 fallback unconditionally registers
h3_app.any("/*", *ThisServer, this, onH3Request/onH3404) which bypasses the
existing star_methods_covered_by_user / has_*_for_star_path precedence logic
used for H1; change the H3 registration to check the same coverage flags
(star_methods_covered_by_user or the has_<METHOD>_for_star_path booleans you
already compute) and only call h3_app.any for the methods that remain uncovered,
preserving existing checks around this.config.onRequest and reusing onH3Request
vs onH3404 accordingly.
- Around line 2989-2990: The H3 SNI installation currently swallows errors with
`catch {}` when calling `h3a.addServerNameWithOptions(sni_servername,
sni_ssl_config.asUSockets())`; change this to propagate the failure instead of
ignoring it—mirror the TCP app path behavior by returning or forwarding the
error from the enclosing function (or using `try`) so startup fails when
`addServerNameWithOptions` fails; update the code around `if (comptime has_h3)
if (this.h3_app) |h3a|` to remove the empty `catch {}` and propagate the error
up to the caller.
---
Outside diff comments:
In `@src/bun.js/api/server/RequestContext.zig`:
- Around line 2270-2280: The fallback reason phrase in doWriteStatus
(RequestContext) currently formats as "{d} HM" producing non-descriptive phrases
like "999 HM"; change the fallback passed to resp.writeStatus to include a
clearer phrase such as "{d} Unknown" (so unknown codes become "999 Unknown") or
otherwise append "Unknown" instead of "HM". Update the std.fmt.bufPrint call in
doWriteStatus to format the status code with "Unknown" and leave the existing
HTTPStatusText lookup and flags logic unchanged.
---
Duplicate comments:
In `@packages/bun-uws/src/Http3Request.h`:
- Around line 53-58: getMethod() currently lowercases bytes blindly with (c |
0x20) and always writes into methodLower causing non-letter corruption and
truncation when method length >= sizeof(methodLower). Fix by: in
Http3Request::getMethod(), if method.size() >= sizeof(methodLower) return the
original method string_view unchanged (avoid clipping); otherwise copy
method.size() bytes into methodLower and lowercase only ASCII letters (e.g. if
'A' <= ch && ch <= 'Z' then ch |= 0x20) instead of applying |0x20 to every byte;
use method, methodLower and getMethod() to locate the change.
In `@src/bun.js/api/server.zig`:
- Around line 1329-1333: The code uses this.listener directly in many lifecycle
paths while getPort() was updated to consider UDP/h3_listener, causing
mismatches; add a single helper (e.g., getEffectiveListener or
getActiveListener) that returns the currently bound listener (checking
this.listener, this.h3_listener/UDP variant, and falling back to
this.config.address.tcp) and then replace direct reads of this.listener and
direct port/config lookups in stopFromJS(), disposeFromJS(),
getAllClosedPromise(), deinitIfWeCan(), getAddress(), getURLAsString(), and any
other lifecycle code with calls to that helper so the bound port and stop()
invocation are always based on the actual active listener.
In `@src/bun.js/api/server/RequestContext.zig`:
- Around line 1362-1371: The user-supplied Transfer-Encoding is being written
unconditionally in the response.getFetchHeaders() branch
(headers.fastGet(.TransferEncoding) → resp.writeHeader("transfer-encoding",
...)), which violates HTTP/3; update that branch in RequestContext.zig to detect
the connection protocol (the same HTTP/3 check used in doWriteHeaders where the
synthesized "chunked" case is guarded) and if the protocol is HTTP/3 do NOT
write the Transfer-Encoding header from the user-supplied headers—simply proceed
to renderMetadata() and endWithoutBody(this.shouldCloseConnection()); otherwise
retain the current behavior that writes the header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 631e1bb9-e4fe-4003-81a5-d6c26e8fe501
📒 Files selected for processing (11)
packages/bun-usockets/src/quic.cpackages/bun-uws/src/Http3Request.hpackages/bun-uws/src/Http3Response.hsrc/bun.js/api/server.zigsrc/bun.js/api/server/HTMLBundle.zigsrc/bun.js/api/server/RequestContext.zigsrc/bun.js/api/server/ServerConfig.zigsrc/bun.js/bindings/CookieMap.cppsrc/deps/uws/Request.zigsrc/deps/uws/h3.zigtest/js/bun/http/serve-protocols.test.ts
- quic.c: handle us_create_timer/strdup failure on the OOM path.
- setRoutes: skip the H3 "/*" fallback when a user or static "/*"
route was already mirrored — HttpRouter::add replaces same-pattern
entries, so the fallback was overwriting routes:{"/*":...} on H3.
- listen: throw on H3 SNI install failure instead of swallowing it.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/bun.js/api/server.zig (2)
1329-1333:⚠️ Potential issue | 🔴 CriticalTreat
h3_listeneras the active listener everywhere, not just here.These branches know about
h3_listener, but the rest of the lifecycle still keys offthis.listeneronly. Withh1: false,stopFromJS()/disposeFromJS()never callstop(),getAllClosedPromise()resolves immediately, and address/URL metadata still behaves like the server never bound.Also applies to: 1559-1577
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/api/server.zig` around lines 1329 - 1333, The code only treats this.listener as the active listener but must also treat this.h3_listener as an equivalent active listener; update all lifecycle and metadata paths (including stopFromJS(), disposeFromJS(), stop(), getAllClosedPromise(), and any address/URL or port resolution logic that currently uses this.listener or this.config.address.tcp.port) to check for and prefer this.h3_listener when present (or handle both listeners where appropriate), calling stop() on the actual instantiated listener, awaiting its close promise, and using h3_listener.getLocalPort() for bound-address metadata so the server behaves correctly when h1 is false.
2879-2890:⚠️ Potential issue | 🟠 MajorSkipping the H3
"/*"fallback entirely drops uncovered methods.This avoids overwriting mirrored wildcard routes, but it no longer mirrors the H1 precedence logic above. If the app only registers
GET "/*"(or any partial method set), H1 still installs fallback handlers for the remaining methods viastar_methods_covered_by_user; H3 now skips fallback registration completely, so those methods lose the shared fetch/404 path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/api/server.zig` around lines 2879 - 2890, The H3 fallback currently bails out entirely when a "/*" user/static route exists, which drops handlers for methods the user didn't register; update the block that uses this.h3_app to mirror the H1 logic: if there is no user/static star at all, keep the existing h3_app.any("/*", ...) behavior (using onH3Request or onH3404); otherwise iterate the HTTP methods not present in star_methods_covered_by_user and register per-method fallbacks on h3_app (e.g., h3_app.get/post/put/...("/*", *ThisServer, this, onH3Request or onH3404) depending on this.config.onRequest) so uncovered methods still get the shared fetch/404 path; keep the comptime ssl_enabled and Windows guard and reuse the existing symbols has_any_user_route_for_star_path, has_static_route_for_star_path, star_methods_covered_by_user, onH3Request, onH3404, and ThisServer to locate and implement the change.packages/bun-uws/src/Http3Request.h (1)
51-58:⚠️ Potential issue | 🟠 Major
getMethod()still mangles valid HTTP methods.This is still doing a blind
| 0x20fold into a fixed 16-byte buffer. That corrupts non-A-Ztoken bytes and truncates longer extension methods, andHttp3Contextroutes on this value, so otherwise valid HTTP/3 requests can be misrouted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bun-uws/src/Http3Request.h` around lines 51 - 58, getMethod() currently applies a blind | 0x20 to every byte into a fixed 16-byte methodLower buffer which corrupts non-ASCII-uppercase token bytes and truncates long extension methods; update getMethod() to only map ASCII 'A'..'Z' to lowercase (e.g. if (c >= 'A' && c <= 'Z') c |= 0x20) and leave all other bytes untouched, and avoid truncation by if method.size() > sizeof(methodLower) simply returning the original method string_view (not a truncated buffer) so Http3Context routing gets the full, uncorrupted token; keep references to method, methodLower and getMethod when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/bun-usockets/src/quic.c`:
- Around line 15-18: Add the missing stdio declarations by including <stdio.h>
at the top of the file so functions used by us_quic_log_buf (fwrite, fputc,
stderr) are declared; update the include block that currently has sys/socket.h,
netinet/in.h, and netinet/ip.h to also `#include` <stdio.h> so us_quic_log_buf and
any other stdio calls compile on strict C builds.
---
Duplicate comments:
In `@packages/bun-uws/src/Http3Request.h`:
- Around line 51-58: getMethod() currently applies a blind | 0x20 to every byte
into a fixed 16-byte methodLower buffer which corrupts non-ASCII-uppercase token
bytes and truncates long extension methods; update getMethod() to only map ASCII
'A'..'Z' to lowercase (e.g. if (c >= 'A' && c <= 'Z') c |= 0x20) and leave all
other bytes untouched, and avoid truncation by if method.size() >
sizeof(methodLower) simply returning the original method string_view (not a
truncated buffer) so Http3Context routing gets the full, uncorrupted token; keep
references to method, methodLower and getMethod when making the change.
In `@src/bun.js/api/server.zig`:
- Around line 1329-1333: The code only treats this.listener as the active
listener but must also treat this.h3_listener as an equivalent active listener;
update all lifecycle and metadata paths (including stopFromJS(),
disposeFromJS(), stop(), getAllClosedPromise(), and any address/URL or port
resolution logic that currently uses this.listener or
this.config.address.tcp.port) to check for and prefer this.h3_listener when
present (or handle both listeners where appropriate), calling stop() on the
actual instantiated listener, awaiting its close promise, and using
h3_listener.getLocalPort() for bound-address metadata so the server behaves
correctly when h1 is false.
- Around line 2879-2890: The H3 fallback currently bails out entirely when a
"/*" user/static route exists, which drops handlers for methods the user didn't
register; update the block that uses this.h3_app to mirror the H1 logic: if
there is no user/static star at all, keep the existing h3_app.any("/*", ...)
behavior (using onH3Request or onH3404); otherwise iterate the HTTP methods not
present in star_methods_covered_by_user and register per-method fallbacks on
h3_app (e.g., h3_app.get/post/put/...("/*", *ThisServer, this, onH3Request or
onH3404) depending on this.config.onRequest) so uncovered methods still get the
shared fetch/404 path; keep the comptime ssl_enabled and Windows guard and reuse
the existing symbols has_any_user_route_for_star_path,
has_static_route_for_star_path, star_methods_covered_by_user, onH3Request,
onH3404, and ThisServer to locate and implement the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7a095f6b-aa3d-4c47-b6b9-a1739e275111
📒 Files selected for processing (3)
packages/bun-usockets/src/quic.cpackages/bun-uws/src/Http3Request.hsrc/bun.js/api/server.zig
us_quic_global_init() is now a plain function called once from uws_h3_create_app via a C++11 thread-safe static local, so quic.c no longer needs <pthread.h> (and the once-guard ports to Windows for free).
…er-write kicks Replaces the per-context us_timer_t and the seven us_quic_stream_kick call sites with a single per-loop driver: - loop_data gains quic_head (linked list of engines on this loop) and a lazy fallthrough quic_timer. - us_quic_loop_process(loop) walks the list, runs process_conns on each engine, and re-arms the timer to the soonest earliest_adv_tick. Called from us_internal_loop_post (once per epoll iteration) and from drainMicrotasks (after JS microtasks, so an async resp.end() is packetized before the loop blocks). - Http3Response::write/end/tryEnd/markDone no longer kick. Because process_conns never runs from inside an Http3Response method, on_close cannot synchronously free the stream while a method is touching it — the tryEnd UAF guard is no longer load-bearing. Also in this commit: - 400/413 over H3 end the stream instead of CONNECTION_CLOSE-ing every sibling on the conn (RFC 9114 §4.1.2). - withCustomServer test helper has a single stderr owner with a waitForStderr(regex) helper, fixing the reload-test flake where the background drain stole the RELOADED line. - ban-limits: bump hasException counts; createFormat catch → handleOom. - Http3Context: drop dead post-route setParameters.
H1 connections are real us_poll_t entries, so the loop's `while (num_polls)` keeps running while any are open. QUIC connections share one UDP fd and were invisible to that counter, so after a graceful stop() the UDP socket was the only thing holding the loop and nothing ever closed it. on_new_conn / on_conn_closed now ++/-- loop->num_polls and a per-context conn_count. When closing && conn_count == 0 (either immediately in shutdown or when the last conn drains), the UDP listeners are released so the loop can exit. New test starts an h3-only server, hits it, stop()s, and asserts the process exits without process.exit().
- Http3Request::forEachHeader: skip a literal `host` header field when
:authority is set so req.headers.get('host') matches req.url instead
of being comma-joined with an attacker-supplied value (RFC 9114
§4.3.1 allows both to be sent).
- onBufferedBodyChunk 413: call toErrorInstance directly with a
message — toErrorInstance already handles .Locked (rejects the
promise, deinits the readable), so re-resolving a stale copy
afterwards leaked the .Error string when JS had a pending await on
the body.
- stopListening: notify the inspector and set flags.terminated on the
h3-only orelse arm, matching the H1 path.
- fetch-h3.ts: header dump path uses os.tmpdir() instead of /tmp.
- Http3Request::getMethod: only fold A-Z (the blind |0x20 mangled valid method-token chars like '_'/'^') and grow the scratch buffer to 32. - libuwsockets_h3.cpp: ffi_sv() — never write nullptr through a [*]const u8 out-param when the underlying string_view is default-constructed. - writeFetchHeadersToH3Response: skip Proxy-Connection in the uncommon- header loop (Connection/Keep-Alive/Upgrade are already stripped by doWriteHeaders before this runs). - AnyRequestContext.onAbort: keep the union tag check — the matching arm is the only one whose payload type equals *T.Resp, so a mismatch traps in safe builds instead of being silently @ptrCast. - bench/http3-hello: stop the server before exit so the last response flushes.
Add Proxy-Connection to HTTPHeaderName so doWriteHeaders can
fastRemove(.ProxyConnection) instead of string-scanning every uncommon
header on the H3 response path. Touched: .in, .h (enum/count/traits),
.gperf (string table + keyword + sync UChar→char16_t/StringView span in
the verbatim section), .cpp (regenerated with gperf 3.1),
HTTPHeaderStrings.cpp (both case arrays), HTTPHeaderIdentifiers.h
(EACH_NAME macro), and the Zig mirror in FetchHeaders.zig.
libuv.c: scope the has_added_timer_to_event_loop guard in us_timer_set
to the sweep timer only, matching epoll_kqueue.c. The unconditional
guard made every non-sweep timer one-shot at the API level, so the
QUIC fallthrough timer (repeat_ms=0) never re-armed and lsquic's
millisecond-scale PTO/ACK/idle deadlines were lost on Windows.
Http3Request: when :authority is absent, promote a literal Host into
authority in the constructor so getHeader("host"), req.url, and the
forEachHeader synthesis agree (RFC 9114 §4.3.1 permits Host without
:authority).
doRenderHeadResponse: skip the user-supplied Transfer-Encoding branch
on http3 — only the synthesized .Locked arm had the !http3 guard.
h3blast: cap -t at 256 (render_live's per-worker arrays are [256]).
…op free StaticRoute.doWriteHeaders and FileRoute.writeHeaders now emit alt-svc: h3=":<port>" via the new AnyServer.h3AltSvc() — only the RequestContext.renderMetadata path did this before, so a static-route- only server never advertised the QUIC endpoint to browsers. The Alt-Svc test now exercises all three response paths. us_internal_loop_data_free closes quic_timer alongside sweep_timer so a Worker that ran h3 and terminates doesn't leak the libuv-side fallthrough handle.
Flaking on alpine release builds (passed on every other target and 5/5
locally) — assert the full {stdout, exitCode, stderr} object so the
next failure shows what curl reported, and give the in-flight handlers
50ms instead of 20ms after cooldown so the response lands well after
GOAWAY on faster builds.
cirospaciari
approved these changes
Apr 27, 2026
|
Pretty cool. It keeps the idea of 1 shared "App-like" interface for all transports, and builds the Http3 on existing uSockets stuff. lsquic was selected way back for its speed, the results back then were very promising. I think this is a very good path and I think Node.js will have a very steep hill to climb (if I remember correctly, their H3 support is astoundingly slow). |
|
Awesome, 1000 thanks for this hard and elegant work <3 |
dylan-conway
added a commit
that referenced
this pull request
Apr 27, 2026
…CURL_HTTP3 Bun.serve HTTP/3 support landed in #29768; the server tests in test/js/bun/http/serve-http3.test.ts and serve-protocols.test.ts shell out to `curl --http3-only` and skip when no HTTP/3-capable curl is in PATH (discovery: $CURL_HTTP3 -> `curl-h3` -> `curl` with HTTP3 in --version). Distro/system curl on every CI image lacks HTTP/3, so the suite has only ever run locally. Installs the stunnel/static-curl 8.19.0 release (fully static, ngtcp2+nghttp3) as `curl-h3` alongside the system curl on Linux (glibc/musl, x64/aarch64) and Windows (x64/aarch64), and sets CURL_HTTP3 to its path. Bumps bootstrap versions: ps1 18->19, sh 33->34.
dylan-conway
added a commit
that referenced
this pull request
Apr 28, 2026
) Bun.serve HTTP/3 support landed in #29768; the server tests in `test/js/bun/http/serve-http3.test.ts` and `serve-protocols.test.ts` shell out to `curl --http3-only` and currently **skip on every CI platform** because system curl lacks HTTP/3. Test discovery (see `test/js/bun/http/fetch-h3.ts`): `$CURL_HTTP3` → `curl-h3` in PATH → plain `curl` if `--version` includes `HTTP3`. This installs the [stunnel/static-curl](https://github.com/stunnel/static-curl) 8.19.0 release — fully static, built with ngtcp2 + nghttp3 — as `curl-h3` alongside the system curl, and sets `CURL_HTTP3` pointing at it: - **Linux** (`bootstrap.sh`): glibc/musl × x64/aarch64 → `/usr/{local/,}bin/curl-h3`, `CURL_HTTP3` exported via `append_to_profile` - **Windows** (`bootstrap.ps1`): x64/aarch64 → `C:\Windows\System32\curl-h3.exe` (so it survives Sysprep), `CURL_HTTP3` set machine-wide System curl is left untouched. Bootstrap versions bumped: ps1 18→19, sh 33→34. This also republishes the Windows v18→v19 images that #29568's `[publish images]` never landed (those builds failed on the southcentralus IP quota; cleared in robobun#46), so it doubles as the Windows-CI unblock. Verified `Features: ... HTTP3 ...` is present in the binary's `--version` output (the test gates on that exact token).
xhjkl
pushed a commit
to xhjkl/bun
that referenced
this pull request
May 14, 2026
> [!WARNING]
> **Highly experimental.** HTTP/3 support is new, only exercised by the
test suite in this PR, and likely has bugs that the protocol-agnostic
tests don't reach (QPACK edge cases, frame reordering, DoS patterns that
need a raw QUIC client to drive). Do not deploy `h3: true` to production
yet.
Adds an HTTP/3 listener to `Bun.serve` that shares the same port,
routes, and `fetch` handler as the existing HTTP/1.1+2 listener.
```ts
Bun.serve({
port: 443,
tls: { ... },
h3: true, // also listen on UDP/443 for HTTP/3
// h1: false, // optional: serve HTTP/3 only
fetch(req) { return new Response("hi"); },
});
```
When both protocols are enabled, the server binds **TCP/443** for
HTTP/1.1+2 and **UDP/443** for HTTP/3 — these are independent kernel
sockets that don't conflict. The TCP listener binds first; the UDP
listener reads back the actual port (so `port: 0` resolves correctly)
and binds the same number. H1/H2 responses then auto-emit `Alt-Svc:
h3=":<port>"; ma=86400` so browsers discover the QUIC endpoint.
## Performance
| req/s | HTTP/3 | HTTPS/1.1 | HTTP/1.1 |
| --- | --: | --: | --: |
| `routes: { "/hi": new Response("hello!") }` | **400,865** | 160,133 |
236,686 |
| `fetch(req) { return new Response("Hello " + req.url) }` | **176,480**
| 100,357 | 187,034 |
HTTP/3 and HTTPS/1.1 are the same `Bun.serve({ tls, h3: true })`
instance; HTTP/1.1 is the same routes without `tls`. Release build,
Linux x64, single process, loopback. HTTP/3 via `h3blast` (8×8×32),
HTTP/1.1 via `oha -c 50`. ~50% of HTTP/3 CPU is inside lsquic; next
levers are `UDP_SEGMENT` GSO and `SO_REUSEPORT` multi-engine.
## Architecture
### Layering
```
┌────────────────────────────────────────────────────────┐
│ Bun.serve fetch / routes │
├────────────────────────────────────────────────────────┤
│ NewRequestContext(ssl, debug, Server, http3: bool) │ src/bun.js/api/server/RequestContext.zig
│ HTTPServerWritable(ssl, http3) → H3ResponseSink │ src/bun.js/webcore/streams.zig
├────────────────────────────────────────────────────────┤
│ uws.H3.{App,Request,Response} (Zig bindings) │ src/deps/uws/h3.zig
├────────────────────────────────────────────────────────┤
│ uws_h3_app_* / uws_h3_res_* / uws_h3_req_* (C ABI) │ src/deps/libuwsockets_h3.cpp
├────────────────────────────────────────────────────────┤
│ uWS::Http3{App,Context,Request,Response,ResponseData} │ packages/bun-uws/src/Http3*.h
│ (mirrors TemplatedApp / HttpResponse<SSL> 1:1) │
├────────────────────────────────────────────────────────┤
│ us_quic_socket_context / stream / hset │ packages/bun-usockets/src/quic.c
├────────────────────────────────────────────────────────┤
│ lsquic v4.6.2 (engine, QPACK, congestion, crypto) │ vendor/lsquic
├────────────────────────────────────────────────────────┤
│ usockets UDP (recvmmsg / sendmmsg) + loop pre/post │ packages/bun-usockets
└────────────────────────────────────────────────────────┘
```
The `Http3*` C++ classes expose **exactly** the
`TemplatedApp`/`HttpResponse<SSL>`/`HttpRequest` method surface that the
existing Zig code already calls, so the Zig layer doesn't carry
transport-specific logic — `NewRequestContext` is instantiated once for
TCP, once for SSL, and once for H3, and the same body renders the
response in all three.
### Packet flow
**Ingress.** The usockets epoll/kqueue loop already reads UDP via
`bsd_recvmmsg` into a shared packet buffer (`loop.c`). Each packet is
handed to `us_quic_udp_on_data` → `lsquic_engine_packet_in(engine,
payload, len, &local_addr, peer_addr, ls, ecn)`. lsquic owns connection
demux (DCID lookup), handshake state, decryption, loss recovery, and
stream reassembly. The engine docs guarantee `packet_in_data` is not
retained past the call (`PI_OWN_DATA`-gated copy if it needs to outlive
the buffer), so the shared recvmmsg buffer is safe to reuse.
**Stream lifecycle.** lsquic invokes a `lsquic_stream_if` table per
stream:
- `on_new_stream` allocates a `us_quic_stream_t` (extension area sized
for `Http3ResponseData`); the uWS `on_stream_open` lambda placement-news
the `Http3ResponseData` there.
- `lsquic_hset_if` builds the request headers **before** the stream
object exists: `hsi_prepare_decode` returns space in a growable buffer;
`hsi_process_header` records name/value **offsets** (not pointers — the
buffer moves on `realloc`); after the last header,
`us_quic_hset_finalize` resolves offsets into a `us_quic_header_t[]`
once the buffer is stable.
- `on_read`: first call retrieves the finalized hset and fires
`on_stream_headers` (which builds an `Http3Request` on the stack and
routes via `HttpRouter`). Subsequent calls loop `lsquic_stream_read` and
fire `on_stream_data(chunk, last)` with `last=true` on FIN.
- `on_write` fires when the stream can accept bytes; the uWS
`on_stream_writable` lambda calls `Http3Response::drain()` which empties
backpressure then invokes the user `onWritable`.
- `on_close` fires `on_stream_close` (the uWS lambda runs `onAborted` so
the `RequestContext` drops its `*Response` before the stream is freed)
and frees the `us_quic_stream_t`.
**Egress.** `Http3Response::write/end/tryEnd` buffer header pairs
(lowercased) into a `WTF::Vector<char, 256>` until first body byte, then
`sendBufferedHeaders` flattens name+value into one contiguous buf and
calls `lsquic_stream_send_headers` with `lsxpack_header[]` pointing into
it (lsquic requires the single-buffer form). Body bytes go through
`lsquic_stream_write`; on backpressure they're held in
`Http3ResponseData::backpressure` and `lsquic_stream_wantwrite(1)` is
set. Stream writes only bump a per-context `pending_write_bytes` counter
— they **never** call `lsquic_engine_process_conns` inline (that can
fire `on_close` and free the stream the caller is still touching).
**Driving the engine.** There is no per-context timer fd.
`us_quic_loop_process(loop)` walks every QUIC engine on the loop, runs
`lsquic_engine_process_conns`, and records the soonest
`lsquic_engine_earliest_adv_tick` into `loop->data.quic_next_tick_us`.
It is called from three places: `us_internal_loop_pre` /
`us_internal_loop_post` (before and after every epoll/kqueue iteration),
and — gated on `pending_write_bytes` — from `drainQuicIfNecessary()`
after the JS microtask + deferred-task queue, so a `fetch()` handler
that returns a `Response` from a `then` flushes in the same tick. Idle
wake-ups for retransmit/PTO/idle-timeout come from
`Timer.All.getTimeout()` clamping the existing `epoll_pwait2` timeout to
`quic_next_tick_us`, so QUIC scheduling costs zero extra syscalls.
`us_quic_packets_out` (lsquic's `ea_packets_out`) batches outgoing
`lsquic_out_spec[]` through **`sendmmsg(64)`** on Linux (grouped by
`peer_ctx` so each batch targets one UDP socket), with a `sendmsg` loop
on other POSIX. On a short send it forces `errno=EAGAIN` and arms the
UDP poll for `WRITABLE`; the loop's `on_drain` calls
`lsquic_engine_send_unsent_packets`. `loop.c` was changed to clear
`WRITABLE` **before** invoking `on_drain`, so a callback that re-arms it
(because the socket filled again) keeps the re-arm. The Linux UDP poll
dispatch also drains `MSG_ERRQUEUE` on `EPOLLERR` (parsing
`sock_extended_err.ee_errno` from `IP_RECVERR`/`IPV6_RECVERR` cmsgs) —
without this, an ICMP error queued after a peer drops leaves
`EPOLLERR|EPOLLOUT` level-triggered and the loop spins.
### Zig: one code path, three transports
`NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode:
bool, comptime ThisServer: type, comptime http3: bool)` derives:
```zig
const App = if (http3) struct { pub const Response = uws.H3.Response; } else uws.NewApp(ssl_enabled);
pub const Req = if (http3) uws.H3.Request else uws.Request;
pub const ResponseStream = jsc.WebCore.HTTPServerWritable(ssl_enabled, http3);
```
Each TLS server type carries:
```zig
pub const RequestContext = NewRequestContext(ssl_enabled, debug_mode, @this(), false);
pub const H3RequestContext = NewRequestContext(ssl_enabled, debug_mode, @this(), true);
```
`onH3Request`/`onH3UserRouteRequest` are one-line wrappers over the same
`prepareJsRequestContextFor`/`handleRequestFor` generics. The handful of
`bool ssl`-keyed C++ helpers (`WebCore__FetchHeaders__toUWSResponse`,
`CookieMap__write`) became `int kind` (0=TCP, 1=SSL, 2=H3).
`AnyRequestContext`'s twelve hand-rolled switches were collapsed into
one `dispatch()` over the type map so adding two H3 types didn't mean
twenty-four new arms.
`HTTPServerWritable(ssl, http3)` instantiates an **`H3ResponseSink`**
(added to `generate-jssink.ts`), so `new Response(readableStream)`
streams over QUIC the same way it does over TCP.
For static/file/HTML-bundle routes, **`uws.AnyRequest = union(enum) {
h1, h3 }`** with `inline else` dispatch for
`header`/`method`/`url`/`setYield`/`dateForHeader` lets the route
handlers take an explicit transport-agnostic request instead of
`anytype`.
### Lifetime hazards this PR fixes
QUIC streams die after FIN; H1 sockets persist. That asymmetry created
two bugs caught by the adversarial suite and the branch sweep:
- `process_conns` running from inside an `Http3Response` method can fire
`on_close` and free `this` synchronously when the client has already
FIN'd, so the write path never drives the engine inline; `tryEnd`
returns `{ok, ok || hasResponded()}` and the C shim no longer touches
the response after `tryEnd`.
- `Http3ResponseData` keeps a separate `writableUserData` slot — corked
TCP `tryEnd` never reports backpressure, but QUIC does (lsquic needs a
`process_conns` between HEADERS and DATA), so the `H3ResponseSink` and
the `RequestContext` would otherwise stomp each other's `userData`.
`Http3Request` is stack-allocated in the route callback, same hazard as
`uws.Request`. The H3 `prepareJsRequestContextFor` eagerly populates the
JS `Request`'s URL and `FetchHeaders` (via a new
`WebCore__FetchHeaders__createFromH3`) so
`req.url`/`req.headers`/`req.method`/`req.params` survive any `await`;
`AnyRequestContext.getRequest()` returns `null` for H3 contexts so the
lazy H1 path is never taken with the wrong pointer type.
### TLS / SNI / ALPN
`us_create_quic_socket_context` builds the default `SSL_CTX` from the
same `us_bun_socket_context_options_t` the H1 server uses, then forces
`TLS1_3` and installs an `SSL_CTX_set_alpn_select_cb` that picks the
first `h3`/`h3-*` from the client's list (lsquic does **not** set ALPN
on a caller-supplied `SSL_CTX`). The `sni:` array maps to
`us_quic_socket_context_add_server_name`, which builds one `SSL_CTX` per
hostname; lsquic's `ea_lookup_cert` linear-scans on the SNI string with
the default ctx as fallback. A small lsquic patch removes the hardcoded
"SNI is required for H3" check so IP-literal clients work. `0-RTT` is
disabled (`SSL_CTX_set_early_data_enabled(0)`).
### Shutdown
`server.stop(false)` → `us_quic_socket_context_shutdown`: sets
`closing`, `lsquic_engine_cooldown` (sends GOAWAY on every promoted
conn, drops handshaking mini-conns), flushes; the UDP socket and the
per-connection `num_polls` refs stay live so in-flight streams drain,
and the listener is released once `conn_count` hits zero. `on_new_conn`
rejects during cooldown. `server.stop(true)` closes the UDP fd
immediately. The listen socket is freed in
`us_quic_socket_context_free`, **not** in `on_close` — `peer_ctx` on
every live conn still points at it.
### Limits
`es_max_header_list_size = 16K` caps decoded request headers;
`es_init_max_streams_bidi = 100`; `es_idle_to` is driven by
`Bun.serve`'s `idleTimeout`. The QPACK encoder runs static-table-only
(`es_qpack_enc_max_size = 0`, `es_qpack_enc_max_blocked = 0`) and
Extensible Priorities is off (`es_ext_http_prio = 0`); both were
measurable hot spots under load and neither matters for a server that
mostly emits the same handful of response headers. `Transfer-Encoding`
is rejected with 400 per RFC 9114 §4.2 (compliant clients strip it; this
is defense-in-depth against raw QUIC).
### Vendoring
`scripts/build/deps/lsquic.ts` is a `DirectBuild` of lsquic v4.6.2: the
83 `liblsquic` sources plus `lsqpack.c` from the ls-qpack vendor
(compiled in-tree with `LSQPACK_ENC/DEC_LOGGER_HEADER` redirected to
lsquic's logger headers — without those defines, ls-qpack's `E_DEBUG`
calls `fprintf(logger_ctx, …)` and segfaults on a non-`FILE*`). xxHash
is shared with the existing ls-hpack vendor. Three patches: a
pre-generated `lsquic_versions_to_string.c` (replaces the upstream Perl
build step), the SNI relaxation for IP-literal clients, and
`skip-priority-walk.patch` which short-circuits
`lsquic_send_ctl_determine_bpt`'s O(streams) scan to `BPT_HIGHEST_PRIO`
until any stream actually changes priority — that walk was ~8% of CPU at
64 concurrent streams.
## Intentionally out of scope
- WebSocket / `server.upgrade()` over H3 returns `false` (RFC 9220 /
WebTransport is a separate project).
- `node:http` handlers panic on H3.
- DevServer HMR stays HTTP/1.1.
- `unix:` addresses skip the H3 listener with a warning
(QUIC-over-AF_UNIX is non-standard and Alt-Svc can't advertise it).
- 0-RTT disabled.
- No GSO yet (`sendmmsg` only); no trailers; no `Expect: 100-continue`
(matches H1 — Bun.serve has no `Expect:` handling on any transport).
## Tests
`test/js/bun/http/serve-http3.test.ts` (40) + `serve-protocols.test.ts`
(20). The protocol-agnostic suite runs the same assertions for HTTP/1.1
(`fetch`) and HTTP/3 (`curl --http3-only` via the `fetch-h3.ts`
wrapper). Adversarial coverage:
- 64 concurrent streams on one connection
- 7 KB single header + 50×100 B headers
- 8 MB POST byte-exact echo
- slow client read (`--limit-rate`) on a streamed response
- 204 → 200 on the same connection
- HEAD on a large body (content-length, no body)
- lying `Content-Length` (server stays alive)
- client RST mid-response
- **Per-stream isolation**: 8×96 KB and 3×300 KB unique random POST
bodies → server returns each byte +1 → byte-exact verify per stream
(catches any read-buffer reuse or backpressure aliasing)
- `new Response(subprocess.stdout)`, `new Response(req.body)`
passthrough, `new Response(Bun.file().stream())`
- `req.{url,method,headers,params}` re-read after `await
Promise.resolve()` / `await Bun.sleep(0)` / both
- 2 MB `Bun.file()` (over the sendfile threshold; H3 takes the reader
path)
- `requestIP` over H3 (v4-mapped unwrap)
- `server.reload()` clears stale H3 routes
- graceful `server.stop()` lets in-flight H3 requests complete
- `Alt-Svc` present on H1 responses
H3 cases skip when no HTTP/3-capable curl is on `PATH` (set
`CURL_HTTP3=/path/to/curl`).
Fixes oven-sh#13656
Fixes oven-sh#14453
---------
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
xhjkl
pushed a commit
to xhjkl/bun
that referenced
this pull request
May 14, 2026
…n-sh#29786) Bun.serve HTTP/3 support landed in oven-sh#29768; the server tests in `test/js/bun/http/serve-http3.test.ts` and `serve-protocols.test.ts` shell out to `curl --http3-only` and currently **skip on every CI platform** because system curl lacks HTTP/3. Test discovery (see `test/js/bun/http/fetch-h3.ts`): `$CURL_HTTP3` → `curl-h3` in PATH → plain `curl` if `--version` includes `HTTP3`. This installs the [stunnel/static-curl](https://github.com/stunnel/static-curl) 8.19.0 release — fully static, built with ngtcp2 + nghttp3 — as `curl-h3` alongside the system curl, and sets `CURL_HTTP3` pointing at it: - **Linux** (`bootstrap.sh`): glibc/musl × x64/aarch64 → `/usr/{local/,}bin/curl-h3`, `CURL_HTTP3` exported via `append_to_profile` - **Windows** (`bootstrap.ps1`): x64/aarch64 → `C:\Windows\System32\curl-h3.exe` (so it survives Sysprep), `CURL_HTTP3` set machine-wide System curl is left untouched. Bootstrap versions bumped: ps1 18→19, sh 33→34. This also republishes the Windows v18→v19 images that oven-sh#29568's `[publish images]` never landed (those builds failed on the southcentralus IP quota; cleared in robobun#46), so it doubles as the Windows-CI unblock. Verified `Features: ... HTTP3 ...` is present in the binary's `--version` output (the test gates on that exact token).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
Highly experimental. HTTP/3 support is new, only exercised by the test suite in this PR, and likely has bugs that the protocol-agnostic tests don't reach (QPACK edge cases, frame reordering, DoS patterns that need a raw QUIC client to drive). Do not deploy
h3: trueto production yet.Adds an HTTP/3 listener to
Bun.servethat shares the same port, routes, andfetchhandler as the existing HTTP/1.1+2 listener.When both protocols are enabled, the server binds TCP/443 for HTTP/1.1+2 and UDP/443 for HTTP/3 — these are independent kernel sockets that don't conflict. The TCP listener binds first; the UDP listener reads back the actual port (so
port: 0resolves correctly) and binds the same number. H1/H2 responses then auto-emitAlt-Svc: h3=":<port>"; ma=86400so browsers discover the QUIC endpoint.Performance
routes: { "/hi": new Response("hello!") }fetch(req) { return new Response("Hello, World!" + i++) }HTTP/3 and HTTPS/1.1 are the same
Bun.serve({ tls, h3: true })instance; HTTP/1.1 is the same routes withouttls. Release build, Linux x64, single process, loopback. HTTP/3 viah3blast(8×8×32), HTTP/1.1 viaoha -c 50. ~50% of HTTP/3 CPU is inside lsquic; next levers areUDP_SEGMENTGSO andSO_REUSEPORTmulti-engine.Architecture
Layering
The
Http3*C++ classes expose exactly theTemplatedApp/HttpResponse<SSL>/HttpRequestmethod surface that the existing Zig code already calls, so the Zig layer doesn't carry transport-specific logic —NewRequestContextis instantiated once for TCP, once for SSL, and once for H3, and the same body renders the response in all three.Packet flow
Ingress. The usockets epoll/kqueue loop already reads UDP via
bsd_recvmmsginto a shared packet buffer (loop.c). Each packet is handed tous_quic_udp_on_data→lsquic_engine_packet_in(engine, payload, len, &local_addr, peer_addr, ls, ecn). lsquic owns connection demux (DCID lookup), handshake state, decryption, loss recovery, and stream reassembly. The engine docs guaranteepacket_in_datais not retained past the call (PI_OWN_DATA-gated copy if it needs to outlive the buffer), so the shared recvmmsg buffer is safe to reuse.Stream lifecycle. lsquic invokes a
lsquic_stream_iftable per stream:on_new_streamallocates aus_quic_stream_t(extension area sized forHttp3ResponseData); the uWSon_stream_openlambda placement-news theHttp3ResponseDatathere.lsquic_hset_ifbuilds the request headers before the stream object exists:hsi_prepare_decodereturns space in a growable buffer;hsi_process_headerrecords name/value offsets (not pointers — the buffer moves onrealloc); after the last header,us_quic_hset_finalizeresolves offsets into aus_quic_header_t[]once the buffer is stable.on_read: first call retrieves the finalized hset and fireson_stream_headers(which builds anHttp3Requeston the stack and routes viaHttpRouter). Subsequent calls looplsquic_stream_readand fireon_stream_data(chunk, last)withlast=trueon FIN.on_writefires when the stream can accept bytes; the uWSon_stream_writablelambda callsHttp3Response::drain()which empties backpressure then invokes the useronWritable.on_closefireson_stream_close(the uWS lambda runsonAbortedso theRequestContextdrops its*Responsebefore the stream is freed) and frees theus_quic_stream_t.Egress.
Http3Response::write/end/tryEndbuffer header pairs (lowercased) into aWTF::Vector<char, 256>until first body byte, thensendBufferedHeadersflattens name+value into one contiguous buf and callslsquic_stream_send_headerswithlsxpack_header[]pointing into it (lsquic requires the single-buffer form). Body bytes go throughlsquic_stream_write; on backpressure they're held inHttp3ResponseData::backpressureandlsquic_stream_wantwrite(1)is set. Stream writes only bump a per-contextpending_write_bytescounter — they never calllsquic_engine_process_connsinline (that can fireon_closeand free the stream the caller is still touching).Driving the engine. There is no per-context timer fd.
us_quic_loop_process(loop)walks every QUIC engine on the loop, runslsquic_engine_process_conns, and records the soonestlsquic_engine_earliest_adv_tickintoloop->data.quic_next_tick_us. It is called from three places:us_internal_loop_pre/us_internal_loop_post(before and after every epoll/kqueue iteration), and — gated onpending_write_bytes— fromdrainQuicIfNecessary()after the JS microtask + deferred-task queue, so afetch()handler that returns aResponsefrom athenflushes in the same tick. Idle wake-ups for retransmit/PTO/idle-timeout come fromTimer.All.getTimeout()clamping the existingepoll_pwait2timeout toquic_next_tick_us, so QUIC scheduling costs zero extra syscalls.us_quic_packets_out(lsquic'sea_packets_out) batches outgoinglsquic_out_spec[]throughsendmmsg(64)on Linux (grouped bypeer_ctxso each batch targets one UDP socket), with asendmsgloop on other POSIX. On a short send it forceserrno=EAGAINand arms the UDP poll forWRITABLE; the loop'son_draincallslsquic_engine_send_unsent_packets.loop.cwas changed to clearWRITABLEbefore invokingon_drain, so a callback that re-arms it (because the socket filled again) keeps the re-arm. The Linux UDP poll dispatch also drainsMSG_ERRQUEUEonEPOLLERR(parsingsock_extended_err.ee_errnofromIP_RECVERR/IPV6_RECVERRcmsgs) — without this, an ICMP error queued after a peer drops leavesEPOLLERR|EPOLLOUTlevel-triggered and the loop spins.Zig: one code path, three transports
NewRequestContext(comptime ssl_enabled: bool, comptime debug_mode: bool, comptime ThisServer: type, comptime http3: bool)derives:Each TLS server type carries:
onH3Request/onH3UserRouteRequestare one-line wrappers over the sameprepareJsRequestContextFor/handleRequestForgenerics. The handful ofbool ssl-keyed C++ helpers (WebCore__FetchHeaders__toUWSResponse,CookieMap__write) becameint kind(0=TCP, 1=SSL, 2=H3).AnyRequestContext's twelve hand-rolled switches were collapsed into onedispatch()over the type map so adding two H3 types didn't mean twenty-four new arms.HTTPServerWritable(ssl, http3)instantiates anH3ResponseSink(added togenerate-jssink.ts), sonew Response(readableStream)streams over QUIC the same way it does over TCP.For static/file/HTML-bundle routes,
uws.AnyRequest = union(enum) { h1, h3 }withinline elsedispatch forheader/method/url/setYield/dateForHeaderlets the route handlers take an explicit transport-agnostic request instead ofanytype.Lifetime hazards this PR fixes
QUIC streams die after FIN; H1 sockets persist. That asymmetry created two bugs caught by the adversarial suite and the branch sweep:
process_connsrunning from inside anHttp3Responsemethod can fireon_closeand freethissynchronously when the client has already FIN'd, so the write path never drives the engine inline;tryEndreturns{ok, ok || hasResponded()}and the C shim no longer touches the response aftertryEnd.Http3ResponseDatakeeps a separatewritableUserDataslot — corked TCPtryEndnever reports backpressure, but QUIC does (lsquic needs aprocess_connsbetween HEADERS and DATA), so theH3ResponseSinkand theRequestContextwould otherwise stomp each other'suserData.Http3Requestis stack-allocated in the route callback, same hazard asuws.Request. The H3prepareJsRequestContextForeagerly populates the JSRequest's URL andFetchHeaders(via a newWebCore__FetchHeaders__createFromH3) soreq.url/req.headers/req.method/req.paramssurvive anyawait;AnyRequestContext.getRequest()returnsnullfor H3 contexts so the lazy H1 path is never taken with the wrong pointer type.TLS / SNI / ALPN
us_create_quic_socket_contextbuilds the defaultSSL_CTXfrom the sameus_bun_socket_context_options_tthe H1 server uses, then forcesTLS1_3and installs anSSL_CTX_set_alpn_select_cbthat picks the firsth3/h3-*from the client's list (lsquic does not set ALPN on a caller-suppliedSSL_CTX). Thesni:array maps tous_quic_socket_context_add_server_name, which builds oneSSL_CTXper hostname; lsquic'sea_lookup_certlinear-scans on the SNI string with the default ctx as fallback. A small lsquic patch removes the hardcoded "SNI is required for H3" check so IP-literal clients work.0-RTTis disabled (SSL_CTX_set_early_data_enabled(0)).Shutdown
server.stop(false)→us_quic_socket_context_shutdown: setsclosing,lsquic_engine_cooldown(sends GOAWAY on every promoted conn, drops handshaking mini-conns), flushes; the UDP socket and the per-connectionnum_pollsrefs stay live so in-flight streams drain, and the listener is released onceconn_counthits zero.on_new_connrejects during cooldown.server.stop(true)closes the UDP fd immediately. The listen socket is freed inus_quic_socket_context_free, not inon_close—peer_ctxon every live conn still points at it.Limits
es_max_header_list_size = 16Kcaps decoded request headers;es_init_max_streams_bidi = 100;es_idle_tois driven byBun.serve'sidleTimeout. The QPACK encoder runs static-table-only (es_qpack_enc_max_size = 0,es_qpack_enc_max_blocked = 0) and Extensible Priorities is off (es_ext_http_prio = 0); both were measurable hot spots under load and neither matters for a server that mostly emits the same handful of response headers.Transfer-Encodingis rejected with 400 per RFC 9114 §4.2 (compliant clients strip it; this is defense-in-depth against raw QUIC).Vendoring
scripts/build/deps/lsquic.tsis aDirectBuildof lsquic v4.6.2: the 83liblsquicsources pluslsqpack.cfrom the ls-qpack vendor (compiled in-tree withLSQPACK_ENC/DEC_LOGGER_HEADERredirected to lsquic's logger headers — without those defines, ls-qpack'sE_DEBUGcallsfprintf(logger_ctx, …)and segfaults on a non-FILE*). xxHash is shared with the existing ls-hpack vendor. Three patches: a pre-generatedlsquic_versions_to_string.c(replaces the upstream Perl build step), the SNI relaxation for IP-literal clients, andskip-priority-walk.patchwhich short-circuitslsquic_send_ctl_determine_bpt's O(streams) scan toBPT_HIGHEST_PRIOuntil any stream actually changes priority — that walk was ~8% of CPU at 64 concurrent streams.Intentionally out of scope
server.upgrade()over H3 returnsfalse(RFC 9220 / WebTransport is a separate project).node:httphandlers panic on H3.unix:addresses skip the H3 listener with a warning (QUIC-over-AF_UNIX is non-standard and Alt-Svc can't advertise it).sendmmsgonly); no trailers; noExpect: 100-continue(matches H1 — Bun.serve has noExpect:handling on any transport).Tests
test/js/bun/http/serve-http3.test.ts(40) +serve-protocols.test.ts(20). The protocol-agnostic suite runs the same assertions for HTTP/1.1 (fetch) and HTTP/3 (curl --http3-onlyvia thefetch-h3.tswrapper). Adversarial coverage:--limit-rate) on a streamed responseContent-Length(server stays alive)new Response(subprocess.stdout),new Response(req.body)passthrough,new Response(Bun.file().stream())req.{url,method,headers,params}re-read afterawait Promise.resolve()/await Bun.sleep(0)/ bothBun.file()(over the sendfile threshold; H3 takes the reader path)requestIPover H3 (v4-mapped unwrap)server.reload()clears stale H3 routesserver.stop()lets in-flight H3 requests completeAlt-Svcpresent on H1 responsesH3 cases skip when no HTTP/3-capable curl is on
PATH(setCURL_HTTP3=/path/to/curl).Fixes #13656
Fixes #14453