Skip to content

Commit 51c649d

Browse files
dmcgowandmcgowan
committed
Update HTTPFallback to handle tls handshake timeout
Signed-off-by: Derek McGowan <[email protected]> (cherry picked from commit 5e470e1) Signed-off-by: Derek McGowan <[email protected]>
1 parent aa14890 commit 51c649d

5 files changed

Lines changed: 106 additions & 24 deletions

File tree

‎remotes/docker/config/hosts.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
247247

248248
// When TLS has been configured for the operation or host and
249249
// the protocol from the port number is ambiguous, use the
250-
// docker.HTTPFallback roundtripper to catch TLS errors and re-attempt the
250+
// docker.NewHTTPFallback roundtripper to catch TLS errors and re-attempt the
251251
// request as http. This allows preference for https when configured but
252252
// also catches TLS errors early enough in the request to avoid sending
253253
// the request twice or consuming the request body.
@@ -256,7 +256,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
256256
if port != "" && port != "80" {
257257
log.G(ctx).WithField("host", host.host).Info("host will try HTTPS first since it is configured for HTTP with a TLS configuration, consider changing host to HTTPS or removing unused TLS configuration")
258258
host.scheme = "https"
259-
rhosts[i].Client.Transport = docker.HTTPFallback{RoundTripper: rhosts[i].Client.Transport}
259+
rhosts[i].Client.Transport = docker.NewHTTPFallback(rhosts[i].Client.Transport)
260260
}
261261
}
262262

‎remotes/docker/config/hosts_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -462,11 +462,11 @@ func TestHTTPFallback(t *testing.T) {
462462
if testHosts[0].Scheme != tc.expectedScheme {
463463
t.Fatalf("expected %s scheme for localhost with tls config, got %q", tc.expectedScheme, testHosts[0].Scheme)
464464
}
465-
_, ok := testHosts[0].Client.Transport.(docker.HTTPFallback)
466-
if tc.usesFallback && !ok {
465+
_, defaultTransport := testHosts[0].Client.Transport.(*http.Transport)
466+
if tc.usesFallback && defaultTransport {
467467
t.Fatal("expected http fallback configured for defaulted localhost endpoint")
468-
} else if ok && !tc.usesFallback {
469-
t.Fatal("expected no http fallback configured for defaulted localhost endpoint")
468+
} else if !defaultTransport && !tc.usesFallback {
469+
t.Fatalf("expected no http fallback configured for defaulted localhost endpoint")
470470
}
471471
})
472472
}

‎remotes/docker/pusher_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func TestPusherHTTPFallback(t *testing.T) {
110110
if client.Transport == nil {
111111
client.Transport = http.DefaultTransport
112112
}
113-
client.Transport = HTTPFallback{client.Transport}
113+
client.Transport = NewHTTPFallback(client.Transport)
114114
p.hosts[0].Client = client
115115
phost := p.hosts[0].Host
116116
p.hosts[0].Authorizer = NewDockerAuthorizer(WithAuthCreds(func(host string) (string, string, error) {

‎remotes/docker/resolver.go‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -704,34 +704,62 @@ func IsLocalhost(host string) bool {
704704
return ip.IsLoopback()
705705
}
706706

707-
// HTTPFallback is an http.RoundTripper which allows fallback from https to http
708-
// for registry endpoints with configurations for both http and TLS, such as
709-
// defaulted localhost endpoints.
710-
type HTTPFallback struct {
711-
http.RoundTripper
707+
// NewHTTPFallback returns http.RoundTripper which allows fallback from https to
708+
// http for registry endpoints with configurations for both http and TLS,
709+
// such as defaulted localhost endpoints.
710+
func NewHTTPFallback(transport http.RoundTripper) http.RoundTripper {
711+
return &httpFallback{
712+
super: transport,
713+
}
712714
}
713715

714-
func (f HTTPFallback) RoundTrip(r *http.Request) (*http.Response, error) {
715-
resp, err := f.RoundTripper.RoundTrip(r)
716-
var tlsErr tls.RecordHeaderError
717-
if errors.As(err, &tlsErr) && string(tlsErr.RecordHeader[:]) == "HTTP/" {
718-
// server gave HTTP response to HTTPS client
719-
plainHTTPUrl := *r.URL
720-
plainHTTPUrl.Scheme = "http"
716+
type httpFallback struct {
717+
super http.RoundTripper
718+
host string
719+
}
720+
721+
func (f *httpFallback) RoundTrip(r *http.Request) (*http.Response, error) {
722+
// only fall back if the same host had previously fell back
723+
if f.host != r.URL.Host {
724+
resp, err := f.super.RoundTrip(r)
725+
if !isTLSError(err) {
726+
return resp, err
727+
}
728+
}
729+
730+
plainHTTPUrl := *r.URL
731+
plainHTTPUrl.Scheme = "http"
721732

722-
plainHTTPRequest := *r
723-
plainHTTPRequest.URL = &plainHTTPUrl
733+
plainHTTPRequest := *r
734+
plainHTTPRequest.URL = &plainHTTPUrl
724735

736+
if f.host != r.URL.Host {
737+
f.host = r.URL.Host
738+
739+
// update body on the second attempt
725740
if r.Body != nil && r.GetBody != nil {
726741
body, err := r.GetBody()
727742
if err != nil {
728743
return nil, err
729744
}
730745
plainHTTPRequest.Body = body
731746
}
747+
}
732748

733-
return f.RoundTripper.RoundTrip(&plainHTTPRequest)
749+
return f.super.RoundTrip(&plainHTTPRequest)
750+
}
751+
752+
func isTLSError(err error) bool {
753+
if err == nil {
754+
return false
755+
}
756+
var tlsErr tls.RecordHeaderError
757+
if errors.As(err, &tlsErr) && string(tlsErr.RecordHeader[:]) == "HTTP/" {
758+
return true
759+
}
760+
if strings.Contains(err.Error(), "TLS handshake timeout") {
761+
return true
734762
}
735763

736-
return resp, err
764+
return false
737765
}

‎remotes/docker/resolver_test.go‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"errors"
2525
"fmt"
2626
"io"
27+
"net"
2728
"net/http"
2829
"net/http/httptest"
2930
"net/url"
@@ -404,7 +405,7 @@ func TestHTTPFallbackResolver(t *testing.T) {
404405
}
405406

406407
client := &http.Client{
407-
Transport: HTTPFallback{http.DefaultTransport},
408+
Transport: NewHTTPFallback(http.DefaultTransport),
408409
}
409410
options := ResolverOptions{
410411
Hosts: func(host string) ([]RegistryHost, error) {
@@ -425,6 +426,59 @@ func TestHTTPFallbackResolver(t *testing.T) {
425426
runBasicTest(t, "testname", sf)
426427
}
427428

429+
func TestHTTPFallbackTimeoutResolver(t *testing.T) {
430+
sf := func(h http.Handler) (string, ResolverOptions, func()) {
431+
432+
l, err := net.Listen("tcp", "127.0.0.1:0")
433+
if err != nil {
434+
t.Fatal(err)
435+
}
436+
server := &http.Server{
437+
Handler: h,
438+
ReadHeaderTimeout: time.Second,
439+
}
440+
go func() {
441+
// Accept first connection but do not do anything with it
442+
// to force TLS handshake to timeout. Subsequent connection
443+
// will be HTTP and should work.
444+
c, err := l.Accept()
445+
if err != nil {
446+
return
447+
}
448+
go func() {
449+
time.Sleep(time.Second)
450+
c.Close()
451+
}()
452+
server.Serve(l)
453+
}()
454+
host := l.Addr().String()
455+
456+
defaultTransport := &http.Transport{
457+
TLSHandshakeTimeout: time.Millisecond,
458+
}
459+
client := &http.Client{
460+
Transport: NewHTTPFallback(defaultTransport),
461+
}
462+
463+
options := ResolverOptions{
464+
Hosts: func(host string) ([]RegistryHost, error) {
465+
return []RegistryHost{
466+
{
467+
Client: client,
468+
Host: host,
469+
Scheme: "https",
470+
Path: "/v2",
471+
Capabilities: HostCapabilityPull | HostCapabilityResolve | HostCapabilityPush,
472+
},
473+
}, nil
474+
},
475+
}
476+
return host, options, func() { l.Close() }
477+
}
478+
479+
runBasicTest(t, "testname", sf)
480+
}
481+
428482
func TestResolveProxy(t *testing.T) {
429483
var (
430484
ctx = context.Background()

0 commit comments

Comments
 (0)