net.http: enable HTTP/2 by default for https requests#27384
Conversation
Flip the `enable_http2` default on Request and FetchConfig from false to true, so https requests made with http.fetch / http.get negotiate HTTP/2 when the server offers it (ALPN `h2`), and transparently fall back to HTTP/1.1 otherwise. Set `enable_http2: false` to force HTTP/1.1. This is opt-out rather than opt-in now that the HTTP/2 client supports redirects, streaming response callbacks, and flow control, and has been exercised against real servers. Plain http:// is unaffected (no ALPN), and the Windows SChannel backend keeps using HTTP/1.1 until it gains ALPN (vlang#27383). Updates the gated real-server test to assert the new default (a plain get() negotiates HTTP/2) and that `enable_http2: false` still forces HTTP/1.1. Notes for review: - No latency regression: the client sends the preface, SETTINGS, and the request together without waiting for the server's SETTINGS, so there is no extra round trip; HPACK also shrinks request headers. - Each fetch is still one connection / one request (no pooling yet), so the multiplexing win does not yet apply; this change is about defaulting to the newer protocol, with pooling/multiplexing as a follow-up. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
• Codex Review Findings
Tests Run
|
Address review feedback on enabling HTTP/2 by default: - test_http2_fetch_real_server now skips on the default Windows/SChannel configuration (`$if windows && !no_vschannel ?`), since SChannel has no ALPN yet (vlang#27383) and a default get() there stays on HTTP/1.1. The path remains covered with `-d no_vschannel` (mbedtls client). - Refresh the now-stale comment in backend.c.v: with enable_http2 defaulting to true, ordinary get()/fetch() calls advertise ALPN and use HTTP/2 when the server selects it (opt out with enable_http2: false). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Both addressed in fa3d8c1: Medium (Windows network test): Low (stale comment): Updated the comment in Verified: full |
…27393) test_server_tls_h2_negotiation asserted that a plain `http.fetch()` (no `enable_http2` field) is served as HTTP/1.1. That held before #27384, but #27384 flipped `enable_http2` to default `true`, so the client now advertises `h2` ALPN by default and the server correctly upgrades — making the test's `resp_h1.version() == .v1_1` assertion fail. This is currently red on master CI (clang/gcc/tcc-linux, clang-macos). The server behavior is correct; the test just needs to opt out explicitly. Pass `enable_http2: false` on the HTTP/1.1 leg, which is what the test means to exercise (server still serves h1 to a client that does not negotiate h2). Co-authored-by: Richard Wheeler <quaesitor.scientiam@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Flips the
enable_http2default (onRequestandFetchConfig) fromfalseto
true, sohttp.fetch/http.getto anhttps://URL negotiate HTTP/2when the server offers it, and fall back to HTTP/1.1 otherwise. To force
HTTP/1.1, set
enable_http2: false.This is the opt-out counterpart to the opt-in support added in #27362, now that
the HTTP/2 client also handles redirects, streaming callbacks (#27369), and flow
control, and has been exercised against real servers.
Scope
The change is three lines (the two field defaults + the doc) plus a test update.
request.v,http.venable_http2 bool→enable_http2 bool = true, with updated docsh2_client_test.vget()negotiates HTTP/2 and thatenable_http2: falseforces HTTP/1.1Behaviour / compatibility
http://(no ALPN, never HTTP/2), and the WindowsSChannel backend, which has no ALPN yet and stays on HTTP/1.1 (tracked in
net.http: add ALPN (and HTTP/2) support to the Windows SChannel TLS backend #27383).
SETTINGS, and the request together without waiting for the server's SETTINGS,
so there's no added round trip; HPACK also shrinks request headers.
h2, http/1.1is safe — servers thatdon't understand ALPN simply ignore it and the client uses HTTP/1.1.
Honest tradeoff for the maintainer to weigh
Each
fetchis still one connection / one request (no pooling ormultiplexing yet), so HTTP/2's headline win — multiplexing many requests over
one connection — doesn't yet materialize. This PR is purely about defaulting to
the newer protocol; pooling/multiplexing is a planned follow-up. If you'd rather
keep HTTP/2 opt-in until pooling lands, this PR can simply wait — the opt-in
path (#27362) already works.
Tests
vlib/net/httpsuite green (mbedtls; 1 Windows-only skip).http_httpbin_test.v:GET/POST/PUT/PATCH/DELETE/HEAD against a live server) passes through the new
default HTTP/2 path unchanged.
-d networktest confirms a defaultget('https://www.google.com/')returns
HTTP/2.0, andenable_http2: falsereturnsHTTP/1.1.-W -cstrict -cc clangclean.🤖 Generated with Claude Code