feat: improve ACME error types (#2761)

This commit is contained in:
Ludovic Fernandez 2025-12-18 14:29:52 +01:00 committed by GitHub
commit a5cc0e1555
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 123 additions and 24 deletions

View file

@ -120,39 +120,46 @@ func (d *Doer) formatUserAgent() string {
}
func checkError(req *http.Request, resp *http.Response) error {
if resp.StatusCode >= http.StatusBadRequest {
body, err := io.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("%d :: %s :: %s :: %w", resp.StatusCode, req.Method, req.URL, err)
}
var errorDetails *acme.ProblemDetails
err = json.Unmarshal(body, &errorDetails)
if err != nil {
return fmt.Errorf("%d ::%s :: %s :: %w :: %s", resp.StatusCode, req.Method, req.URL, err, string(body))
}
errorDetails.Method = req.Method
errorDetails.URL = req.URL.String()
if errorDetails.HTTPStatus == 0 {
errorDetails.HTTPStatus = resp.StatusCode
}
// Check for errors we handle specifically
if errorDetails.HTTPStatus == http.StatusBadRequest && errorDetails.Type == acme.BadNonceErr {
return &acme.NonceError{ProblemDetails: errorDetails}
}
if errorDetails.HTTPStatus == http.StatusConflict && errorDetails.Type == acme.AlreadyReplacedErr {
return &acme.AlreadyReplacedError{ProblemDetails: errorDetails}
}
return errorDetails
if resp.StatusCode < http.StatusBadRequest {
return nil
}
return nil
body, err := io.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("%d :: %s :: %s :: %w", resp.StatusCode, req.Method, req.URL, err)
}
var errorDetails *acme.ProblemDetails
err = json.Unmarshal(body, &errorDetails)
if err != nil {
return fmt.Errorf("%d ::%s :: %s :: %w :: %s", resp.StatusCode, req.Method, req.URL, err, string(body))
}
errorDetails.Method = req.Method
errorDetails.URL = req.URL.String()
if errorDetails.HTTPStatus == 0 {
errorDetails.HTTPStatus = resp.StatusCode
}
// Check for errors we handle specifically
switch {
case errorDetails.HTTPStatus == http.StatusBadRequest && errorDetails.Type == acme.BadNonceErr:
return &acme.NonceError{ProblemDetails: errorDetails}
case errorDetails.HTTPStatus == http.StatusConflict && errorDetails.Type == acme.AlreadyReplacedErr:
return &acme.AlreadyReplacedError{ProblemDetails: errorDetails}
case errorDetails.HTTPStatus == http.StatusTooManyRequests && errorDetails.Type == acme.RateLimitedErr:
return &acme.RateLimitedError{
ProblemDetails: errorDetails,
RetryAfter: resp.Header.Get("Retry-After"),
}
default:
return errorDetails
}
}
type httpsOnly struct {

View file

@ -1,11 +1,14 @@
package sender
import (
"bytes"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/go-acme/lego/v4/acme"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@ -78,3 +81,70 @@ func TestDo_failWithHTTP(t *testing.T) {
_, err := sender.Post(server.URL, strings.NewReader("data"), "text/plain", nil)
require.ErrorContains(t, err, "HTTPS is required: http://")
}
func Test_checkError(t *testing.T) {
testCases := []struct {
desc string
resp *http.Response
assert func(t *testing.T, err error)
}{
{
desc: "default",
resp: &http.Response{
StatusCode: http.StatusNotFound,
Body: io.NopCloser(bytes.NewBufferString(`{"type":"urn:ietf:params:acme:error:example","detail":"message","status":404}`)),
},
assert: errorAs[*acme.ProblemDetails],
},
{
desc: "badNonce",
resp: &http.Response{
StatusCode: http.StatusBadRequest,
Body: io.NopCloser(bytes.NewBufferString(`{"type":"urn:ietf:params:acme:error:badNonce","detail":"message","status":400}`)),
},
assert: errorAs[*acme.NonceError],
},
{
desc: "alreadyReplaced",
resp: &http.Response{
StatusCode: http.StatusConflict,
Body: io.NopCloser(bytes.NewBufferString(`{"type":"urn:ietf:params:acme:error:alreadyReplaced","detail":"message","status":409}`)),
},
assert: errorAs[*acme.AlreadyReplacedError],
},
{
desc: "rateLimited",
resp: &http.Response{
StatusCode: http.StatusConflict,
Header: http.Header{
"Retry-After": []string{"1"},
},
Body: io.NopCloser(bytes.NewBufferString(`{"type":"urn:ietf:params:acme:error:rateLimited","detail":"message","status":429}`)),
},
assert: errorAs[*acme.RateLimitedError],
},
}
for _, test := range testCases {
t.Run(test.desc, func(t *testing.T) {
t.Parallel()
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "https://example.com", nil)
err := checkError(req, test.resp)
require.Error(t, err)
pb := &acme.ProblemDetails{}
assert.ErrorAs(t, err, &pb)
test.assert(t, err)
})
}
}
func errorAs[T error](t *testing.T, err error) {
t.Helper()
var zero T
assert.ErrorAs(t, err, &zero)
}

View file

@ -10,6 +10,7 @@ const (
errNS = "urn:ietf:params:acme:error:"
BadNonceErr = errNS + "badNonce"
AlreadyReplacedErr = errNS + "alreadyReplaced"
RateLimitedErr = errNS + "rateLimited"
)
// ProblemDetails the problem details object.
@ -63,9 +64,30 @@ type NonceError struct {
*ProblemDetails
}
func (e *NonceError) Unwrap() error {
return e.ProblemDetails
}
// AlreadyReplacedError represents the error which is returned
// If the Server rejects the request because the identified certificate has already been marked as replaced.
// if the Server rejects the request because the identified certificate has already been marked as replaced.
// - https://www.rfc-editor.org/rfc/rfc9773.html#section-5
type AlreadyReplacedError struct {
*ProblemDetails
}
func (e *AlreadyReplacedError) Unwrap() error {
return e.ProblemDetails
}
// RateLimitedError represents the error which is returned
// if the server rejects the request because the client has exceeded the rate limit.
// - https://www.rfc-editor.org/rfc/rfc8555.html#section-6.6
type RateLimitedError struct {
*ProblemDetails
RetryAfter string
}
func (e *RateLimitedError) Unwrap() error {
return e.ProblemDetails
}