From a004c57d62e13f820c7ab31d06dc7794637cb147 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 1 Dec 2023 17:04:13 -0500 Subject: [PATCH 1/2] Don't retry after "invalid request header" error. --- client.go | 10 ++++++++++ client_test.go | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/client.go b/client.go index c9edbd0..0ef1eda 100644 --- a/client.go +++ b/client.go @@ -73,6 +73,11 @@ var ( // specifically so we resort to matching on the error string. schemeErrorRe = regexp.MustCompile(`unsupported protocol scheme`) + // A regular expression to match the error returned by net/http when a + // request header or value is invalid. This error isn't typed + // specifically so we resort to matching on the error string. + invalidHeaderErrorRe = regexp.MustCompile(`invalid header`) + // A regular expression to match the error returned by net/http when the // TLS certificate is not trusted. This error isn't typed // specifically so we resort to matching on the error string. @@ -494,6 +499,11 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) { return false, v } + // Don't retry if the error was due to an invalid header. + if invalidHeaderErrorRe.MatchString(v.Error()) { + return false, v + } + // Don't retry if the error was due to TLS cert verification failure. if notTrustedErrorRe.MatchString(v.Error()) { return false, v diff --git a/client_test.go b/client_test.go index c5e98a5..85eb1d7 100644 --- a/client_test.go +++ b/client_test.go @@ -758,6 +758,60 @@ func TestClient_DefaultRetryPolicy_invalidscheme(t *testing.T) { } } +func TestClient_DefaultRetryPolicy_invalidheadername(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer ts.Close() + + attempts := 0 + client := NewClient() + client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) { + attempts++ + return DefaultRetryPolicy(context.TODO(), resp, err) + } + + req, err := http.NewRequest(http.MethodGet, ts.URL, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Header.Set("Header-Name-\033", "header value") + _, err = client.StandardClient().Do(req) + if err == nil { + t.Fatalf("expected header error, got nil") + } + if attempts != 1 { + t.Fatalf("expected 1 attempt, got %d", attempts) + } +} + +func TestClient_DefaultRetryPolicy_invalidheadervalue(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + })) + defer ts.Close() + + attempts := 0 + client := NewClient() + client.CheckRetry = func(_ context.Context, resp *http.Response, err error) (bool, error) { + attempts++ + return DefaultRetryPolicy(context.TODO(), resp, err) + } + + req, err := http.NewRequest(http.MethodGet, ts.URL, nil) + if err != nil { + t.Fatalf("err: %v", err) + } + req.Header.Set("Header-Name", "bad header value \033") + _, err = client.StandardClient().Do(req) + if err == nil { + t.Fatalf("expected header value error, got nil") + } + if attempts != 1 { + t.Fatalf("expected 1 attempt, got %d", attempts) + } +} + func TestClient_CheckRetryStop(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "test_500_body", http.StatusInternalServerError) From a1a8ab82eb1779b8e09b2d6d2605bbf6fd059a17 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Fri, 1 Dec 2023 17:05:21 -0500 Subject: [PATCH 2/2] Fix certificate error detection on Go 1.20 and 1.21. --- cert_error_go119.go | 14 ++++++++++++++ cert_error_go120.go | 14 ++++++++++++++ client.go | 3 +-- 3 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 cert_error_go119.go create mode 100644 cert_error_go120.go diff --git a/cert_error_go119.go b/cert_error_go119.go new file mode 100644 index 0000000..b2b27e8 --- /dev/null +++ b/cert_error_go119.go @@ -0,0 +1,14 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build !go1.20 +// +build !go1.20 + +package retryablehttp + +import "crypto/x509" + +func isCertError(err error) bool { + _, ok := err.(x509.UnknownAuthorityError) + return ok +} diff --git a/cert_error_go120.go b/cert_error_go120.go new file mode 100644 index 0000000..a3cd315 --- /dev/null +++ b/cert_error_go120.go @@ -0,0 +1,14 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +//go:build go1.20 +// +build go1.20 + +package retryablehttp + +import "crypto/tls" + +func isCertError(err error) bool { + _, ok := err.(*tls.CertificateVerificationError) + return ok +} diff --git a/client.go b/client.go index 0ef1eda..bcfd19e 100644 --- a/client.go +++ b/client.go @@ -27,7 +27,6 @@ package retryablehttp import ( "bytes" "context" - "crypto/x509" "fmt" "io" "io/ioutil" @@ -508,7 +507,7 @@ func baseRetryPolicy(resp *http.Response, err error) (bool, error) { if notTrustedErrorRe.MatchString(v.Error()) { return false, v } - if _, ok := v.Err.(x509.UnknownAuthorityError); ok { + if isCertError(v.Err) { return false, v } }