mirror of
https://github.com/netbirdio/netbird.git
synced 2026-09-17 12:19:07 +02:00
Keep the HTTP/1.1 pin for an upstream that asked for it
HTTP_1_1_REQUIRED is not a fault: it is the upstream describing its own configuration. IIS answers it for sites using Windows Authentication — connection-oriented auth that h2 multiplexing cannot carry — and for client-certificate sites, where the per-request certificate needs a TLS renegotiation h2 forbids. Browsers retry those on HTTP/1.1 silently, which is why such a site works in a browser and fails behind this proxy. Expiring that pin every 10 minutes would buy nothing but one failed request per interval, since nothing about the upstream has changed. So a pin the upstream asked for holds until the transport goes away with the proxy or the account's client, and the bounded TTL stays for the ambiguous protocol errors, where the failure may well clear on its own. A later ambiguous failure never weakens a permanent pin.
This commit is contained in:
Viktor Liu
@@ -10,11 +10,33 @@ import (
|
||||
)
|
||||
|
||||
// upstreamDowngradeTTL is how long an upstream stays pinned to HTTP/1.1
|
||||
// after it proved it cannot serve the h2 it advertised. Bounded rather
|
||||
// than permanent so a fixed or replaced backend returns to h2 without
|
||||
// restarting the proxy.
|
||||
// after an h2 failure that only implied it cannot serve h2. Bounded so a
|
||||
// fixed or replaced backend returns to h2 without restarting the proxy.
|
||||
// A pin the upstream asked for itself does not expire — see downgrade.
|
||||
const upstreamDowngradeTTL = 10 * time.Minute
|
||||
|
||||
// downgrade is an upstream's HTTP/1.1 pin.
|
||||
type downgrade struct {
|
||||
// expiry is when the pin lapses and the upstream is offered h2
|
||||
// again. The zero time means it never does: the upstream answered
|
||||
// HTTP_1_1_REQUIRED, which is a statement about how it is
|
||||
// configured, not a fault that may clear on its own. Re-probing
|
||||
// that every upstreamDowngradeTTL would buy nothing but a failed
|
||||
// request per interval, so the pin holds until the transport goes
|
||||
// away with the proxy or the account's client.
|
||||
expiry time.Time
|
||||
}
|
||||
|
||||
// permanent reports whether the upstream asked for this pin itself.
|
||||
func (d downgrade) permanent() bool {
|
||||
return d.expiry.IsZero()
|
||||
}
|
||||
|
||||
// active reports whether the pin still stands at now.
|
||||
func (d downgrade) active(now time.Time) bool {
|
||||
return d.permanent() || now.Before(d.expiry)
|
||||
}
|
||||
|
||||
// upstreamTransport carries requests to a single upstream family (one
|
||||
// TLS configuration) and implements what upstreamHTTPAuto means.
|
||||
//
|
||||
@@ -44,9 +66,8 @@ type upstreamTransport struct {
|
||||
fallback *http.Transport
|
||||
|
||||
mu sync.RWMutex
|
||||
// downgraded maps an upstream host to the time its HTTP/1.1 pin
|
||||
// expires.
|
||||
downgraded map[string]time.Time
|
||||
// downgraded maps an upstream host to its HTTP/1.1 pin.
|
||||
downgraded map[string]downgrade
|
||||
}
|
||||
|
||||
// newUpstreamTransport wraps base for the requested HTTP version. base
|
||||
@@ -62,7 +83,7 @@ func newUpstreamTransport(base *http.Transport, version upstreamHTTPVersion, log
|
||||
primary: base,
|
||||
version: version,
|
||||
logger: logger,
|
||||
downgraded: make(map[string]time.Time),
|
||||
downgraded: make(map[string]downgrade),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,7 +103,11 @@ func (t *upstreamTransport) RoundTrip(req *http.Request) (*http.Response, error)
|
||||
return resp, err
|
||||
}
|
||||
|
||||
t.markDowngraded(host)
|
||||
// HTTP_1_1_REQUIRED is the upstream saying it will not serve this
|
||||
// request over h2 however often it is asked — IIS answers it for
|
||||
// Windows Authentication and for client-certificate sites, where
|
||||
// the cause is site configuration rather than a passing fault.
|
||||
t.markDowngraded(host, isHTTP11Required(err))
|
||||
|
||||
retry, ok := replayable(req)
|
||||
if !ok {
|
||||
@@ -111,21 +136,23 @@ func (t *upstreamTransport) mayDowngrade(req *http.Request) bool {
|
||||
}
|
||||
|
||||
func (t *upstreamTransport) isDowngraded(host string) bool {
|
||||
now := time.Now()
|
||||
|
||||
t.mu.RLock()
|
||||
expiry, ok := t.downgraded[host]
|
||||
pin, ok := t.downgraded[host]
|
||||
t.mu.RUnlock()
|
||||
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
if time.Now().Before(expiry) {
|
||||
if pin.active(now) {
|
||||
return true
|
||||
}
|
||||
|
||||
t.mu.Lock()
|
||||
// Re-check under the write lock: a concurrent request may have
|
||||
// re-pinned the host after the read above.
|
||||
if expiry, ok := t.downgraded[host]; ok && !time.Now().Before(expiry) {
|
||||
if pin, ok := t.downgraded[host]; ok && !pin.active(time.Now()) {
|
||||
delete(t.downgraded, host)
|
||||
}
|
||||
t.mu.Unlock()
|
||||
@@ -133,24 +160,41 @@ func (t *upstreamTransport) isDowngraded(host string) bool {
|
||||
return false
|
||||
}
|
||||
|
||||
func (t *upstreamTransport) markDowngraded(host string) {
|
||||
// markDowngraded pins host to HTTP/1.1. permanent marks a pin the
|
||||
// upstream asked for; anything else lapses after upstreamDowngradeTTL so
|
||||
// a repaired backend is offered h2 again.
|
||||
func (t *upstreamTransport) markDowngraded(host string, permanent bool) {
|
||||
now := time.Now()
|
||||
pin := downgrade{expiry: now.Add(upstreamDowngradeTTL)}
|
||||
if permanent {
|
||||
pin = downgrade{}
|
||||
}
|
||||
|
||||
t.mu.Lock()
|
||||
_, pinned := t.downgraded[host]
|
||||
t.downgraded[host] = now.Add(upstreamDowngradeTTL)
|
||||
for h, expiry := range t.downgraded {
|
||||
if !now.Before(expiry) {
|
||||
previous, pinned := t.downgraded[host]
|
||||
// A permanent pin is never weakened back into an expiring one: the
|
||||
// upstream has already said h2 is not on offer.
|
||||
if !pinned || !previous.permanent() {
|
||||
t.downgraded[host] = pin
|
||||
}
|
||||
for h, existing := range t.downgraded {
|
||||
if !existing.active(now) {
|
||||
delete(t.downgraded, h)
|
||||
}
|
||||
}
|
||||
t.mu.Unlock()
|
||||
|
||||
if !pinned {
|
||||
t.logger.WithField("upstream", host).
|
||||
Warnf("upstream negotiated HTTP/2 but failed to serve it, using HTTP/1.1 for the next %s (set %s=1.1 to pin it)",
|
||||
upstreamDowngradeTTL, EnvUpstreamHTTPVersion)
|
||||
if pinned {
|
||||
return
|
||||
}
|
||||
|
||||
entry := t.logger.WithField("upstream", host)
|
||||
if permanent {
|
||||
entry.Warnf("upstream answered HTTP_1_1_REQUIRED, using HTTP/1.1 for it from now on")
|
||||
return
|
||||
}
|
||||
entry.Warnf("upstream negotiated HTTP/2 but failed to serve it, using HTTP/1.1 for the next %s (set %s=1.1 to pin it)",
|
||||
upstreamDowngradeTTL, EnvUpstreamHTTPVersion)
|
||||
}
|
||||
|
||||
// http1 returns the HTTP/1.1-only clone, creating it on first use.
|
||||
@@ -216,9 +260,20 @@ var http2ErrorMarkers = []string{
|
||||
"stream error: stream ID",
|
||||
// http2.ConnectionError, e.g. "connection error: PROTOCOL_ERROR".
|
||||
"connection error: ",
|
||||
// The GOAWAY code an upstream sends to say the request must be
|
||||
// retried over HTTP/1.1.
|
||||
"HTTP_1_1_REQUIRED",
|
||||
// The code an upstream sends to say the request must be retried
|
||||
// over HTTP/1.1, as a GOAWAY or on the stream.
|
||||
http11RequiredMarker,
|
||||
}
|
||||
|
||||
// http11RequiredMarker is the error code an upstream sends to say the
|
||||
// request belongs on HTTP/1.1. Unlike the other markers it is not a
|
||||
// fault: the upstream is describing its own configuration.
|
||||
const http11RequiredMarker = "HTTP_1_1_REQUIRED"
|
||||
|
||||
// isHTTP11Required reports whether the upstream itself asked for
|
||||
// HTTP/1.1, rather than merely failing at h2.
|
||||
func isHTTP11Required(err error) bool {
|
||||
return err != nil && strings.Contains(err.Error(), http11RequiredMarker)
|
||||
}
|
||||
|
||||
// isHTTP2ProtocolError reports whether err says the upstream cannot
|
||||
|
||||
@@ -47,6 +47,11 @@ func TestUpstreamTransport_AutoFallsBackOnBrokenHTTP2(t *testing.T) {
|
||||
|
||||
assert.True(t, mt.insecure.isDowngraded(srv.addr),
|
||||
"the upstream must stay pinned to HTTP/1.1 after proving it cannot serve h2")
|
||||
mt.insecure.mu.RLock()
|
||||
pin := mt.insecure.downgraded[srv.addr]
|
||||
mt.insecure.mu.RUnlock()
|
||||
assert.True(t, pin.permanent(),
|
||||
"an upstream answering HTTP_1_1_REQUIRED must not be re-probed for h2")
|
||||
|
||||
// The second request must not repeat the h2 attempt: the server
|
||||
// counts h2 handshakes, so a repeat would show up here.
|
||||
@@ -110,11 +115,11 @@ func TestUpstreamTransport_MayDowngrade(t *testing.T) {
|
||||
|
||||
func TestUpstreamTransport_DowngradeExpires(t *testing.T) {
|
||||
transport := newUpstreamTransport(&http.Transport{}, upstreamHTTPAuto, nil)
|
||||
transport.markDowngraded("backend.invalid:443")
|
||||
transport.markDowngraded("backend.invalid:443", false)
|
||||
require.True(t, transport.isDowngraded("backend.invalid:443"))
|
||||
|
||||
transport.mu.Lock()
|
||||
transport.downgraded["backend.invalid:443"] = time.Now().Add(-time.Second)
|
||||
transport.downgraded["backend.invalid:443"] = downgrade{expiry: time.Now().Add(-time.Second)}
|
||||
transport.mu.Unlock()
|
||||
|
||||
assert.False(t, transport.isDowngraded("backend.invalid:443"),
|
||||
@@ -127,13 +132,75 @@ func TestUpstreamTransport_DowngradeExpires(t *testing.T) {
|
||||
|
||||
func TestUpstreamTransport_DowngradeIsPerUpstream(t *testing.T) {
|
||||
transport := newUpstreamTransport(&http.Transport{}, upstreamHTTPAuto, nil)
|
||||
transport.markDowngraded("broken.invalid:443")
|
||||
transport.markDowngraded("broken.invalid:443", false)
|
||||
|
||||
assert.True(t, transport.isDowngraded("broken.invalid:443"))
|
||||
assert.False(t, transport.isDowngraded("healthy.invalid:443"),
|
||||
"one broken upstream must not drop the others to HTTP/1.1")
|
||||
}
|
||||
|
||||
// TestUpstreamTransport_HTTP11RequiredPinIsPermanent covers the IIS
|
||||
// case: HTTP_1_1_REQUIRED describes how the upstream is configured
|
||||
// (Windows Authentication, client certificates), so re-probing it every
|
||||
// upstreamDowngradeTTL would only buy a failed request per interval.
|
||||
func TestUpstreamTransport_HTTP11RequiredPinIsPermanent(t *testing.T) {
|
||||
transport := newUpstreamTransport(&http.Transport{}, upstreamHTTPAuto, nil)
|
||||
transport.markDowngraded("iis.invalid:443", true)
|
||||
|
||||
transport.mu.RLock()
|
||||
pin := transport.downgraded["iis.invalid:443"]
|
||||
transport.mu.RUnlock()
|
||||
|
||||
assert.True(t, pin.permanent(), "an upstream that asked for HTTP/1.1 must not be re-probed")
|
||||
assert.True(t, pin.active(time.Now().Add(100*upstreamDowngradeTTL)),
|
||||
"a permanent pin must outlive any TTL")
|
||||
}
|
||||
|
||||
func TestUpstreamTransport_PermanentPinSurvivesLaterFailures(t *testing.T) {
|
||||
transport := newUpstreamTransport(&http.Transport{}, upstreamHTTPAuto, nil)
|
||||
transport.markDowngraded("iis.invalid:443", true)
|
||||
// A later ambiguous failure for the same upstream must not turn the
|
||||
// permanent pin into an expiring one.
|
||||
transport.markDowngraded("iis.invalid:443", false)
|
||||
|
||||
transport.mu.RLock()
|
||||
pin := transport.downgraded["iis.invalid:443"]
|
||||
transport.mu.RUnlock()
|
||||
|
||||
assert.True(t, pin.permanent(), "a permanent pin must never be weakened")
|
||||
}
|
||||
|
||||
func TestIsHTTP11Required(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err error
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "goaway",
|
||||
err: errors.New(`http2: server sent GOAWAY and closed the connection; LastStreamID=0, ErrCode=HTTP_1_1_REQUIRED, debug=""`),
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "stream error",
|
||||
err: http2.StreamError{StreamID: 1, Code: http2.ErrCodeHTTP11Required},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "other h2 failure",
|
||||
err: http2.StreamError{StreamID: 1, Code: http2.ErrCodeProtocol},
|
||||
want: false,
|
||||
},
|
||||
{name: "nil", err: nil, want: false},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
assert.Equal(t, tc.want, isHTTP11Required(tc.err))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsHTTP2ProtocolError(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
|
||||
Reference in New Issue
Block a user