-
Notifications
You must be signed in to change notification settings - Fork 4.9k
http3: split H3Client into h3_client/ + Alt-Svc upgrade flag #29863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
e6670de
1339fa5
eef6351
d936557
d5051c6
52e17e0
a94aa99
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,9 @@ pub var async_http_id_monotonic: std.atomic.Value(u32) = std.atomic.Value(u32).i | |
| /// Set once at startup from `--experimental-http2-fetch` (before the HTTP | ||
| /// thread spawns) and then only read on that thread, so no atomics needed. | ||
| pub var experimental_http2_client_from_cli: bool = false; | ||
| /// Set once at startup from `--experimental-http3-fetch`. Same threading | ||
| /// rules as the http2 flag. | ||
| pub var experimental_http3_client_from_cli: bool = false; | ||
|
|
||
| const MAX_REDIRECT_URL_LENGTH = 128 * 1024; | ||
|
|
||
|
|
@@ -228,6 +231,20 @@ pub fn alpnOffer(client: *const HTTPClient) BoringSSL.SSL.AlpnOffer { | |
| return if (client.flags.force_http2) .h2_only else .h1_or_h2; | ||
| } | ||
|
|
||
| /// Whether the experimental Alt-Svc-driven HTTP/3 upgrade is enabled and this | ||
| /// request shape is eligible for it (HTTPS, no proxy/unix-socket, no sendfile, | ||
| /// not pinned to a specific protocol). When true, `start_()` consults | ||
| /// `H3.AltSvc.lookup` before opening TCP. | ||
| pub fn canTryH3AltSvc(client: *const HTTPClient) bool { | ||
| if (client.flags.force_http1 or client.flags.force_http2) return false; | ||
| if (client.http_proxy != null) return false; | ||
| if (client.flags.is_preconnect_only) return false; | ||
| if (client.unix_socket_path.length() > 0) return false; | ||
| if (client.state.original_request_body == .sendfile) return false; | ||
| return experimental_http3_client_from_cli or | ||
| bun.feature_flag.BUN_FEATURE_FLAG_EXPERIMENTAL_HTTP3_CLIENT.get(); | ||
| } | ||
|
|
||
| pub fn firstCall( | ||
| client: *HTTPClient, | ||
| comptime is_ssl: bool, | ||
|
|
@@ -1180,6 +1197,25 @@ fn start_(this: *HTTPClient, comptime is_ssl: bool) void { | |
| } | ||
| } | ||
|
|
||
| // Port to dial QUIC on. Defaults to the origin port for `protocol:"http3"`; | ||
| // an Alt-Svc hit overrides it with the advertised alt-authority port | ||
| // (RFC 7838 §3 — the alternative may listen on a different port; SNI and | ||
| // certificate validation still use the origin hostname). | ||
| var h3_port: u16 = this.url.getPortAuto(); | ||
|
|
||
| if (comptime is_ssl) { | ||
| // Opportunistic Alt-Svc upgrade: a previous response from this origin | ||
| // advertised `h3`, and the experimental flag is on. This goes through | ||
| // the same engine as `protocol: "http3"` so retry/fallback semantics | ||
| // are shared. | ||
| if (!this.flags.force_http3 and this.canTryH3AltSvc()) { | ||
| if (H3.AltSvc.lookup(this.url.hostname, h3_port)) |alt_port| { | ||
| this.flags.force_http3 = true; | ||
| h3_port = alt_port; | ||
| } | ||
|
Comment on lines
+1217
to
+1226
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Setting Extended reasoning...What the bug isThe new Alt-Svc upgrade path at if (!this.flags.force_http3 and this.canTryH3AltSvc()) {
if (H3.AltSvc.lookup(this.url.hostname, h3_port)) |alt_port| {
this.flags.force_http3 = true;
h3_port = alt_port;
}
}
The specific code pathOn the redirect leg,
Why existing code doesn't prevent itBefore this PR, ImpactThree concrete failure modes for a plain
Step-by-step proof (cross-origin case)
How to fixDon't persist the opportunistic decision into
The first option also fixes the alt-port-on-redirect case automatically, since the lookup runs again against the redirect target. |
||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| if (this.flags.force_http3) { | ||
| if (comptime !is_ssl) { | ||
| this.fail(error.HTTP3Unsupported); | ||
|
|
@@ -1193,7 +1229,7 @@ fn start_(this: *HTTPClient, comptime is_ssl: bool) void { | |
| this.fail(error.HTTP3Unsupported); | ||
| return; | ||
| }; | ||
| if (!ctx.connect(this, this.url.hostname, this.url.getPortAuto())) { | ||
| if (!ctx.connect(this, this.url.hostname, h3_port)) { | ||
| this.fail(error.ConnectionRefused); | ||
| } | ||
| return; | ||
|
|
@@ -2803,6 +2839,11 @@ pub fn handleResponseMetadata( | |
| hashHeaderConst("Last-Modified") => { | ||
| pretend_304 = this.flags.force_last_modified and response.status_code > 199 and response.status_code < 300 and this.if_modified_since.len > 0 and strings.eql(this.if_modified_since, header.value); | ||
| }, | ||
| hashHeaderConst("Alt-Svc") => { | ||
| if (this.isHTTPS() and this.canTryH3AltSvc()) { | ||
| H3.AltSvc.record(this.url.hostname, this.url.getPortAuto(), header.value); | ||
| } | ||
| }, | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| else => {}, | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.