From fc2189bb49a4d67e3c776679e58658c82875b68d Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Thu, 3 Sep 2026 15:12:44 +0200 Subject: [PATCH] Make the upstream HTTP/2 attempt configurable on the proxy transports --- proxy/internal/roundtrip/multi.go | 2 +- proxy/internal/roundtrip/multi_test.go | 50 ++++++++++++++++++++++++++ proxy/internal/roundtrip/netbird.go | 2 +- proxy/internal/roundtrip/transport.go | 13 +++++++ 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/proxy/internal/roundtrip/multi.go b/proxy/internal/roundtrip/multi.go index 567249437..497e6191b 100644 --- a/proxy/internal/roundtrip/multi.go +++ b/proxy/internal/roundtrip/multi.go @@ -53,7 +53,7 @@ func NewMultiTransport(embedded http.RoundTripper, logger *log.Logger) *MultiTra } direct := &http.Transport{ DialContext: dialWithTimeout(dialer.DialContext), - ForceAttemptHTTP2: true, + ForceAttemptHTTP2: cfg.forceAttemptHTTP2, MaxIdleConns: cfg.maxIdleConns, MaxIdleConnsPerHost: cfg.maxIdleConnsPerHost, MaxConnsPerHost: cfg.maxConnsPerHost, diff --git a/proxy/internal/roundtrip/multi_test.go b/proxy/internal/roundtrip/multi_test.go index 5c6cf1c97..e32e50d23 100644 --- a/proxy/internal/roundtrip/multi_test.go +++ b/proxy/internal/roundtrip/multi_test.go @@ -85,6 +85,56 @@ func TestMultiTransport_AppliesEnvOverridesToDirect(t *testing.T) { "env tuning must also apply to the insecure-skip-verify direct transport") } +// TestMultiTransport_ForceAttemptHTTP2 pins the protocol actually +// negotiated with an HTTPS upstream that offers both h2 and http/1.1. +// The direct transports dial through a custom DialContext, so net/http +// only reaches for h2 while ForceAttemptHTTP2 is set; clearing it via +// NB_PROXY_FORCE_ATTEMPT_HTTP2 must leave the request on HTTP/1.1. +func TestMultiTransport_ForceAttemptHTTP2(t *testing.T) { + tests := []struct { + name string + env string + wantProto string + }{ + {name: "default negotiates h2", env: "", wantProto: "HTTP/2.0"}, + {name: "opt-out stays on http/1.1", env: "false", wantProto: "HTTP/1.1"}, + {name: "explicit opt-in negotiates h2", env: "true", wantProto: "HTTP/2.0"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.env != "" { + t.Setenv(EnvForceAttemptHTTP2, tc.env) + } + + // The test server's certificate isn't in any root pool, so the + // request rides the insecure branch via WithSkipTLSVerify. + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, r.Proto) + })) + srv.EnableHTTP2 = true + srv.StartTLS() + defer srv.Close() + + mt := NewDirectOnly(nil) + ctx := WithSkipTLSVerify(WithDirectUpstream(context.Background())) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, srv.URL, nil) + require.NoError(t, err) + + resp, err := mt.RoundTrip(req) + require.NoError(t, err) + body, err := io.ReadAll(resp.Body) + _ = resp.Body.Close() + require.NoError(t, err) + + assert.Equal(t, tc.wantProto, resp.Proto, + "client-side protocol must follow %s=%q", EnvForceAttemptHTTP2, tc.env) + assert.Equal(t, tc.wantProto, string(body), + "the upstream must see the same protocol the client negotiated") + }) + } +} + // TestMultiTransport_NilEmbeddedErrorsWhenWGPathRequested guards // against the previous silent fallback: a MultiTransport constructed // without an embedded transport must reject requests that don't diff --git a/proxy/internal/roundtrip/netbird.go b/proxy/internal/roundtrip/netbird.go index ae3308a3e..bc71a7a64 100644 --- a/proxy/internal/roundtrip/netbird.go +++ b/proxy/internal/roundtrip/netbird.go @@ -414,7 +414,7 @@ func (n *NetBird) createClientEntry(ctx context.Context, accountID types.Account // not work with reverse proxied requests. transport := &http.Transport{ DialContext: dialWithTimeout(client.DialContext), - ForceAttemptHTTP2: true, + ForceAttemptHTTP2: n.transportCfg.forceAttemptHTTP2, MaxIdleConns: n.transportCfg.maxIdleConns, MaxIdleConnsPerHost: n.transportCfg.maxIdleConnsPerHost, MaxConnsPerHost: n.transportCfg.maxConnsPerHost, diff --git a/proxy/internal/roundtrip/transport.go b/proxy/internal/roundtrip/transport.go index 7c450bbb7..7d44b18d0 100644 --- a/proxy/internal/roundtrip/transport.go +++ b/proxy/internal/roundtrip/transport.go @@ -21,6 +21,7 @@ const ( EnvReadBufferSize = "NB_PROXY_READ_BUFFER_SIZE" EnvDisableCompression = "NB_PROXY_DISABLE_COMPRESSION" EnvMaxInflight = "NB_PROXY_MAX_INFLIGHT" + EnvForceAttemptHTTP2 = "NB_PROXY_FORCE_ATTEMPT_HTTP2" ) // transportConfig holds tunable parameters for the per-account HTTP transport. @@ -37,6 +38,13 @@ type transportConfig struct { disableCompression bool // maxInflight limits per-backend concurrent requests. 0 means unlimited. maxInflight int + // forceAttemptHTTP2 sets http.Transport.ForceAttemptHTTP2. Both proxy + // transports dial through a custom DialContext, which makes net/http + // disable HTTP/2 unless it is forced, so this defaults to true. + // Setting it to false restores that conservative default, leaving + // HTTPS upstreams on HTTP/1.1 for backends whose h2 support is + // advertised but unusable. + forceAttemptHTTP2 bool } func defaultTransportConfig() transportConfig { @@ -47,6 +55,7 @@ func defaultTransportConfig() transportConfig { idleConnTimeout: 90 * time.Second, tlsHandshakeTimeout: 10 * time.Second, expectContinueTimeout: 1 * time.Second, + forceAttemptHTTP2: true, } } @@ -86,6 +95,9 @@ func loadTransportConfig(logger *log.Logger) transportConfig { if v, ok := envInt(EnvMaxInflight, logger); ok { cfg.maxInflight = v } + if v, ok := envBool(EnvForceAttemptHTTP2, logger); ok { + cfg.forceAttemptHTTP2 = v + } logger.WithFields(log.Fields{ "max_idle_conns": cfg.maxIdleConns, @@ -99,6 +111,7 @@ func loadTransportConfig(logger *log.Logger) transportConfig { "read_buffer_size": cfg.readBufferSize, "disable_compression": cfg.disableCompression, "max_inflight": cfg.maxInflight, + "force_attempt_http2": cfg.forceAttemptHTTP2, }).Debug("backend transport configuration") return cfg