From 26cb7b35ebfc8aa6ecfde8948fc844d22f72ed77 Mon Sep 17 00:00:00 2001 From: Fernandez Ludovic Date: Thu, 5 Mar 2026 23:07:23 +0100 Subject: [PATCH] refactor: use duration as RetryAfter field type --- acme/api/certificate.go | 2 ++ acme/api/certificate_renewal.go | 7 +---- acme/api/internal/sender/sender.go | 31 +++++++++++++++++- acme/api/internal/sender/sender_test.go | 36 +++++++++++++++++++++ acme/api/service.go | 42 +++++++------------------ acme/api/service_test.go | 37 ---------------------- acme/commons.go | 7 +++-- acme/errors.go | 3 +- challenge/resolver/solver_manager.go | 4 +-- 9 files changed, 88 insertions(+), 81 deletions(-) diff --git a/acme/api/certificate.go b/acme/api/certificate.go index 449c931dc..d8b08c71c 100644 --- a/acme/api/certificate.go +++ b/acme/api/certificate.go @@ -70,6 +70,8 @@ func (c *CertificateService) get(ctx context.Context, certURL string, bundle boo return nil, nil, err } + defer func() { _ = resp.Body.Close() }() + data, err := io.ReadAll(http.MaxBytesReader(nil, resp.Body, maxBodySize)) if err != nil { return nil, resp.Header, err diff --git a/acme/api/certificate_renewal.go b/acme/api/certificate_renewal.go index b5b907964..acbb6c75f 100644 --- a/acme/api/certificate_renewal.go +++ b/acme/api/certificate_renewal.go @@ -37,12 +37,7 @@ func (c *CertificateService) GetRenewalInfo(ctx context.Context, certID string) return nil, err } - if retry := resp.Header.Get("Retry-After"); retry != "" { - info.RetryAfter, err = ParseRetryAfter(retry) - if err != nil { - return nil, fmt.Errorf("failed to parse Retry-After header: %w", err) - } - } + info.RetryAfter = getRetryAfter(resp) return info, nil } diff --git a/acme/api/internal/sender/sender.go b/acme/api/internal/sender/sender.go index bc11fba54..180dab430 100644 --- a/acme/api/internal/sender/sender.go +++ b/acme/api/internal/sender/sender.go @@ -7,7 +7,9 @@ import ( "io" "net/http" "runtime" + "strconv" "strings" + "time" "github.com/go-acme/lego/v5/acme" "github.com/go-acme/lego/v5/internal/errutils" @@ -153,9 +155,11 @@ func checkError(req *http.Request, resp *http.Response) error { return &acme.AlreadyReplacedError{ProblemDetails: errorDetails} case errorDetails.HTTPStatus == http.StatusTooManyRequests && errorDetails.Type == acme.RateLimitedErr: + retryAfter, _ := ParseRetryAfter(resp.Header.Get("Retry-After")) + return &acme.RateLimitedError{ ProblemDetails: errorDetails, - RetryAfter: resp.Header.Get("Retry-After"), + RetryAfter: retryAfter, } default: @@ -187,3 +191,28 @@ func (r *httpsOnly) RoundTrip(req *http.Request) (*http.Response, error) { return r.rt.RoundTrip(req) } + +// ParseRetryAfter parses the Retry-After header value according to RFC 7231. +// The header can be either delay-seconds (numeric) or HTTP-date (RFC 1123 format). +// https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3 +// Returns the duration until the retry time. +func ParseRetryAfter(value string) (time.Duration, error) { + if value == "" { + return 0, nil + } + + if seconds, err := strconv.ParseInt(value, 10, 64); err == nil { + return time.Duration(seconds) * time.Second, nil + } + + if retryTime, err := time.Parse(time.RFC1123, value); err == nil { + duration := time.Until(retryTime) + if duration < 0 { + return 0, nil + } + + return duration, nil + } + + return 0, fmt.Errorf("invalid Retry-After value: %q", value) +} diff --git a/acme/api/internal/sender/sender_test.go b/acme/api/internal/sender/sender_test.go index 3c077b8cf..cc6b95e1a 100644 --- a/acme/api/internal/sender/sender_test.go +++ b/acme/api/internal/sender/sender_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/go-acme/lego/v5/acme" "github.com/stretchr/testify/assert" @@ -148,3 +149,38 @@ func errorAs[T error](t *testing.T, err error) { var zero T assert.ErrorAs(t, err, &zero) } + +func TestParseRetryAfter(t *testing.T) { + testCases := []struct { + desc string + value string + expected time.Duration + }{ + { + desc: "empty header value", + value: "", + expected: time.Duration(0), + }, + { + desc: "delay-seconds", + value: "123", + expected: 123 * time.Second, + }, + { + desc: "HTTP-date", + value: time.Now().Add(3 * time.Second).Format(time.RFC1123), + expected: 3 * time.Second, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + rt, err := ParseRetryAfter(test.value) + require.NoError(t, err) + + assert.InDelta(t, test.expected.Seconds(), rt.Seconds(), 1) + }) + } +} diff --git a/acme/api/service.go b/acme/api/service.go index 22ce05124..19f8571b4 100644 --- a/acme/api/service.go +++ b/acme/api/service.go @@ -1,11 +1,12 @@ package api import ( - "fmt" "net/http" "regexp" - "strconv" "time" + + "github.com/go-acme/lego/v5/acme/api/internal/sender" + "github.com/go-acme/lego/v5/log" ) type service struct { @@ -52,36 +53,15 @@ func getLocation(resp *http.Response) string { } // getRetryAfter get the value of the header Retry-After. -func getRetryAfter(resp *http.Response) string { +func getRetryAfter(resp *http.Response) time.Duration { if resp == nil { - return "" + return 0 } - return resp.Header.Get("Retry-After") -} - -// ParseRetryAfter parses the Retry-After header value according to RFC 7231. -// The header can be either delay-seconds (numeric) or HTTP-date (RFC 1123 format). -// https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3 -// Returns the duration until the retry time. -// TODO(ldez): unexposed this function in v5. -func ParseRetryAfter(value string) (time.Duration, error) { - if value == "" { - return 0, nil - } - - if seconds, err := strconv.ParseInt(value, 10, 64); err == nil { - return time.Duration(seconds) * time.Second, nil - } - - if retryTime, err := time.Parse(time.RFC1123, value); err == nil { - duration := time.Until(retryTime) - if duration < 0 { - return 0, nil - } - - return duration, nil - } - - return 0, fmt.Errorf("invalid Retry-After value: %q", value) + retryAfter, err := sender.ParseRetryAfter(resp.Header.Get("Retry-After")) + if err != nil { + log.Warn("Failed to parse Retry-After header.", log.ErrorAttr(err)) + } + + return retryAfter } diff --git a/acme/api/service_test.go b/acme/api/service_test.go index 57ea45708..2dbd795c9 100644 --- a/acme/api/service_test.go +++ b/acme/api/service_test.go @@ -3,10 +3,8 @@ package api import ( "net/http" "testing" - "time" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func Test_getLink(t *testing.T) { @@ -55,38 +53,3 @@ func Test_getLink(t *testing.T) { }) } } - -func TestParseRetryAfter(t *testing.T) { - testCases := []struct { - desc string - value string - expected time.Duration - }{ - { - desc: "empty header value", - value: "", - expected: time.Duration(0), - }, - { - desc: "delay-seconds", - value: "123", - expected: 123 * time.Second, - }, - { - desc: "HTTP-date", - value: time.Now().Add(3 * time.Second).Format(time.RFC1123), - expected: 3 * time.Second, - }, - } - - for _, test := range testCases { - t.Run(test.desc, func(t *testing.T) { - t.Parallel() - - rt, err := ParseRetryAfter(test.value) - require.NoError(t, err) - - assert.InDelta(t, test.expected.Seconds(), rt.Seconds(), 1) - }) - } -} diff --git a/acme/commons.go b/acme/commons.go index acf955939..579562702 100644 --- a/acme/commons.go +++ b/acme/commons.go @@ -242,12 +242,13 @@ type Authorization struct { Wildcard bool `json:"wildcard,omitempty"` } -// ExtendedChallenge a extended Challenge. +// ExtendedChallenge an extended Challenge. type ExtendedChallenge struct { Challenge // Contains the value of the response header `Retry-After` - RetryAfter string `json:"-"` + RetryAfter time.Duration `json:"-"` + // Contains the value of the response header `Link` rel="up" AuthorizationURL string `json:"-"` } @@ -362,7 +363,7 @@ type ExtendedRenewalInfo struct { // Conforming clients SHOULD query the renewalInfo URL again after the RetryAfter period has passed, // as the server may provide a different suggestedWindow. // https://www.rfc-editor.org/rfc/rfc9773.html#section-4.2 - RetryAfter time.Duration + RetryAfter time.Duration `json:"-"` } // RenewalInfo is the response to GET requests made the renewalInfo endpoint. diff --git a/acme/errors.go b/acme/errors.go index cd447d7b4..8569be260 100644 --- a/acme/errors.go +++ b/acme/errors.go @@ -3,6 +3,7 @@ package acme import ( "fmt" "strings" + "time" ) // Errors types. @@ -85,7 +86,7 @@ func (e *AlreadyReplacedError) Unwrap() error { type RateLimitedError struct { *ProblemDetails - RetryAfter string + RetryAfter time.Duration } func (e *RateLimitedError) Unwrap() error { diff --git a/challenge/resolver/solver_manager.go b/challenge/resolver/solver_manager.go index 1744803ac..48b395fe3 100644 --- a/challenge/resolver/solver_manager.go +++ b/challenge/resolver/solver_manager.go @@ -121,8 +121,8 @@ func validate(ctx context.Context, core *api.Core, domain string, chlg acme.Chal return nil } - retryAfter, err := api.ParseRetryAfter(chlng.RetryAfter) - if err != nil || retryAfter == 0 { + retryAfter := chlng.RetryAfter + if retryAfter == 0 { // The ACME server MUST return a Retry-After. // If it doesn't, or if it's invalid, we'll just poll hard. // Boulder does not implement the ability to retry challenges or the Retry-After header.