Skip to content

Commit 5e470e1

Browse files
dmcgowandmcgowan
committed
Update HTTPFallback to handle tls handshake timeout
Signed-off-by: Derek McGowan <[email protected]>
1 parent 7c50784 commit 5e470e1

5 files changed

Lines changed: 106 additions & 24 deletions

File tree

‎core/remotes/docker/config/hosts.go‎

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

245245
// When TLS has been configured for the operation or host and
246246
// the protocol from the port number is ambiguous, use the
247-
// docker.HTTPFallback roundtripper to catch TLS errors and re-attempt the
247+
// docker.NewHTTPFallback roundtripper to catch TLS errors and re-attempt the
248248
// request as http. This allows preference for https when configured but
249249
// also catches TLS errors early enough in the request to avoid sending
250250
// the request twice or consuming the request body.
@@ -253,7 +253,7 @@ func ConfigureHosts(ctx context.Context, options HostOptions) docker.RegistryHos
253253
if port != "" && port != "80" {
254254
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")
255255
host.scheme = "https"
256-
rhosts[i].Client.Transport = docker.HTTPFallback{RoundTripper: rhosts[i].Client.Transport}
256+
rhosts[i].Client.Transport = docker.NewHTTPFallback(rhosts[i].Client.Transport)
257257
}
258258
}
259259

‎core/remotes/docker/config/hosts_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -472,11 +472,11 @@ func TestHTTPFallback(t *testing.T) {
472472
if testHosts[0].Scheme != tc.expectedScheme {
473473
t.Fatalf("expected %s scheme for localhost with tls config, got %q", tc.expectedScheme, testHosts[0].Scheme)
474474
}
475-
_, ok := testHosts[0].Client.Transport.(docker.HTTPFallback)
476-
if tc.usesFallback && !ok {
475+
_, defaultTransport := testHosts[0].Client.Transport.(*http.Transport)
476+
if tc.usesFallback && defaultTransport {
477477
t.Fatal("expected http fallback configured for defaulted localhost endpoint")
478-
} else if ok && !tc.usesFallback {
479-
t.Fatal("expected no http fallback configured for defaulted localhost endpoint")
478+
} else if !defaultTransport && !tc.usesFallback {
479+
t.Fatalf("expected no http fallback configured for defaulted localhost endpoint")
480480
}
481481
})
482482
}

‎core/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) {

‎core/remotes/docker/resolver.go‎

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

717-
// HTTPFallback is an http.RoundTripper which allows fallback from https to http
718-
// for registry endpoints with configurations for both http and TLS, such as
719-
// defaulted localhost endpoints.
720-
type HTTPFallback struct {
721-
http.RoundTripper
717+
// NewHTTPFallback returns http.RoundTripper which allows fallback from https to
718+
// http for registry endpoints with configurations for both http and TLS,
719+
// such as defaulted localhost endpoints.
720+
func NewHTTPFallback(transport http.RoundTripper) http.RoundTripper {
721+
return &httpFallback{
722+
super: transport,
723+
}
722724
}
723725

724-
func (f HTTPFallback) RoundTrip(r *http.Request) (*http.Response, error) {
725-
resp, err := f.RoundTripper.RoundTrip(r)
726-
var tlsErr tls.RecordHeaderError
727-
if errors.As(err, &tlsErr) && string(tlsErr.RecordHeader[:]) == "HTTP/" {
728-
// server gave HTTP response to HTTPS client
729-
plainHTTPUrl := *r.URL
730-
plainHTTPUrl.Scheme = "http"
726+
type httpFallback struct {
727+
super http.RoundTripper
728+
host string
729+
}
730+
731+
func (f *httpFallback) RoundTrip(r *http.Request) (*http.Response, error) {
732+
// only fall back if the same host had previously fell back
733+
if f.host != r.URL.Host {
734+
resp, err := f.super.RoundTrip(r)
735+
if !isTLSError(err) {
736+
return resp, err
737+
}
738+
}
739+
740+
plainHTTPUrl := *r.URL
741+
plainHTTPUrl.Scheme = "http"
731742

732-
plainHTTPRequest := *r
733-
plainHTTPRequest.URL = &plainHTTPUrl
743+
plainHTTPRequest := *r
744+
plainHTTPRequest.URL = &plainHTTPUrl
734745

746+
if f.host != r.URL.Host {
747+
f.host = r.URL.Host
748+
749+
// update body on the second attempt
735750
if r.Body != nil && r.GetBody != nil {
736751
body, err := r.GetBody()
737752
if err != nil {
738753
return nil, err
739754
}
740755
plainHTTPRequest.Body = body
741756
}
757+
}
742758

743-
return f.RoundTripper.RoundTrip(&plainHTTPRequest)
759+
return f.super.RoundTrip(&plainHTTPRequest)
760+
}
761+
762+
func isTLSError(err error) bool {
763+
if err == nil {
764+
return false
765+
}
766+
var tlsErr tls.RecordHeaderError
767+
if errors.As(err, &tlsErr) && string(tlsErr.RecordHeader[:]) == "HTTP/" {
768+
return true
769+
}
770+
if strings.Contains(err.Error(), "TLS handshake timeout") {
771+
return true
744772
}
745773

746-
return resp, err
774+
return false
747775
}

‎core/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"
@@ -406,7 +407,7 @@ func TestHTTPFallbackResolver(t *testing.T) {
406407
}
407408

408409
client := &http.Client{
409-
Transport: HTTPFallback{http.DefaultTransport},
410+
Transport: NewHTTPFallback(http.DefaultTransport),
410411
}
411412
options := ResolverOptions{
412413
Hosts: func(host string) ([]RegistryHost, error) {
@@ -427,6 +428,59 @@ func TestHTTPFallbackResolver(t *testing.T) {
427428
runBasicTest(t, "testname", sf)
428429
}
429430

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

0 commit comments

Comments
 (0)