Skip to content

Commit

Permalink
Include original https error on http fallback failures (#125)
Browse files Browse the repository at this point in the history
  • Loading branch information
codygibb committed Mar 14, 2019
1 parent 80cb255 commit b3f9726
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
33 changes: 24 additions & 9 deletions utils/httputil/httputil.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func Send(method, rawurl string, options ...SendOption) (*http.Response, error)
if err != nil {
return nil, fmt.Errorf("parse url: %s", err)
}
opts := sendOptions{
opts := &sendOptions{
body: nil,
timeout: 60 * time.Second,
acceptedCodes: map[int]bool{http.StatusOK: true},
Expand All @@ -272,15 +272,15 @@ func Send(method, rawurl string, options ...SendOption) (*http.Response, error)
httpFallbackDisabled: false,
}
for _, o := range options {
o(&opts)
o(opts)
}

req, err := newRequest(method, opts)
if err != nil {
return nil, err
}

client := http.Client{
client := &http.Client{
Timeout: opts.timeout,
CheckRedirect: opts.redirect,
Transport: opts.transport,
Expand All @@ -294,13 +294,16 @@ func Send(method, rawurl string, options ...SendOption) (*http.Response, error)
// TODO (@evelynl): disable retry after tls migration.
if err != nil && req.URL.Scheme == "https" && !opts.httpFallbackDisabled {
log.Warnf("Failed to send https request: %s. Retrying with http...", err)
var httpReq *http.Request
httpReq, err = newRequest(method, opts)
originalErr := err
resp, err = fallbackToHTTP(client, method, opts)
if err != nil {
return nil, err
// Sometimes the request fails for a reason unrelated to https.
// To keep this reason visible, we always include the original
// error.
err = fmt.Errorf(
"failed to fallback https to http, original https error: %s,\n"+
"fallback http error: %s", originalErr, err)
}
httpReq.URL.Scheme = "http"
resp, err = client.Do(httpReq)
}
if err != nil ||
(resp.StatusCode >= 500 && !opts.acceptedCodes[resp.StatusCode]) ||
Expand Down Expand Up @@ -414,7 +417,7 @@ func ParseDigest(r *http.Request, name string) (core.Digest, error) {
return d, nil
}

func newRequest(method string, opts sendOptions) (*http.Request, error) {
func newRequest(method string, opts *sendOptions) (*http.Request, error) {
req, err := http.NewRequest(method, opts.url.String(), opts.body)
if err != nil {
return nil, fmt.Errorf("new request: %s", err)
Expand All @@ -429,6 +432,18 @@ func newRequest(method string, opts sendOptions) (*http.Request, error) {
return req, nil
}

func fallbackToHTTP(
client *http.Client, method string, opts *sendOptions) (*http.Response, error) {

req, err := newRequest(method, opts)
if err != nil {
return nil, err
}
req.URL.Scheme = "http"

return client.Do(req)
}

func min(a, b time.Duration) time.Duration {
if a < b {
return a
Expand Down
11 changes: 11 additions & 0 deletions utils/httputil/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,14 @@ func TestTLSClientFallback(t *testing.T) {
require.NoError(err)
require.Equal(http.StatusOK, resp.StatusCode)
}

func TestTLSClientFallbackError(t *testing.T) {
require := require.New(t)

c := &TLSConfig{}
tls, err := c.BuildClient()
require.NoError(err)

_, err = Get("https://some-non-existent-addr/", SendTLS(tls))
require.Error(err)
}

0 comments on commit b3f9726

Please sign in to comment.