Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retrying on UnknownAuthorityError #201

Open
ahamza360 opened this issue Aug 9, 2023 · 0 comments · May be fixed by #202
Open

Retrying on UnknownAuthorityError #201

ahamza360 opened this issue Aug 9, 2023 · 0 comments · May be fixed by #202
Labels

Comments

@ahamza360
Copy link

In the default behavior x509.UnknownAuthorityError should not be retried. However, by default retires are done on this error. Instead of regular expression matching for other error, type matching is done here which I suspect is causing the issue.

			// Don't retry if the error was due to TLS cert verification failure.
			if notTrustedErrorRe.MatchString(v.Error()) {
				return false, v
			}
			if _, ok := v.Err.(x509.UnknownAuthorityError); ok {
				return false, v
			}

Below is a unit test to verify this:

func TestHTTPClientWithTLSFailure(t *testing.T) {
	// Create a mock server with a handler
	mockServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("Mock Response"))
	}))
	defer mockServer.Close()

	// Set up the HTTP client with retryablehttp using a custom transport without InsecureSkipVerify
	tr := &http.Transport{
		TLSClientConfig: &tls.Config{},
	}

	// Set up the retryable HTTP client with the custom transport
	client := retryablehttp.NewClient()
	client.HTTPClient.Transport = tr
	client.RetryMax = 2

	// Make a GET request using the retryable HTTP client
	_, err := client.Get(mockServer.URL)

	// Check that the error is indeed related to x509 certificate validation
	var x509Error *x509.CertificateInvalidError
	if assert.Error(t, err) && errors.As(err, &x509Error) {
		assert.Contains(t, x509Error.Error(), "x509: certificate is not valid for any names")
	}
}

== RUN TestHTTPClientWithTLSFailure
2023/08/09 14:58:27 [DEBUG] GET https://127.0.0.1:44641
2023/08/09 14:58:27 [ERR] GET https://127.0.0.1:44641 request failed: Get "https://127.0.0.1:44641": tls: failed to verify certificate: x509: certificate signed by unknown authority
2023/08/09 14:58:27 [DEBUG] GET https://127.0.0.1:44641: retrying in 1s (2 left)
2023/08/09 14:58:27 http: TLS handshake error from 127.0.0.1:56316: remote error: tls: bad certificate
2023/08/09 14:58:28 [ERR] GET https://127.0.0.1:44641 request failed: Get "https://127.0.0.1:44641": tls: failed to verify certificate: x509: certificate signed by unknown authority
2023/08/09 14:58:28 [DEBUG] GET https://127.0.0.1:44641: retrying in 2s (1 left)
2023/08/09 14:58:28 http: TLS handshake error from 127.0.0.1:56318: remote error: tls: bad certificate
2023/08/09 14:58:30 [ERR] GET https://127.0.0.1:44641 request failed: Get "https://127.0.0.1:44641": tls: failed to verify certificate: x509: certificate signed by unknown authority
2023/08/09 14:58:30 http: TLS handshake error from 127.0.0.1:56328: read tcp 127.0.0.1:44641->127.0.0.1:56328: use of closed network connection

@fatelei fatelei linked a pull request Aug 22, 2023 that will close this issue
@manicminer manicminer added the bug label May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants