diff --git a/acme/errors.go b/acme/errors.go index 06b45114..bd37e7bd 100644 --- a/acme/errors.go +++ b/acme/errors.go @@ -310,8 +310,9 @@ func (e *Error) AddSubproblems(subproblems ...Subproblem) *Error { // more information to the ACME client. func (e *Error) WithAdditionalErrorDetail() *Error { // prevent internal server errors from disclosing - // the internal error to the client at all times. - if e == nil || e.Status >= 500 { + // the internal error to the client at all times and + // prevent nil pointers. + if e == nil || e.Status >= 500 || e.Err == nil { return e } diff --git a/acme/errors_test.go b/acme/errors_test.go new file mode 100644 index 00000000..98040739 --- /dev/null +++ b/acme/errors_test.go @@ -0,0 +1,55 @@ +package acme + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func mustJSON(t *testing.T, m map[string]interface{}) string { + t.Helper() + + b, err := json.Marshal(m) + require.NoError(t, err) + + return string(b) +} + +func TestError_WithAdditionalErrorDetail(t *testing.T) { + internalJSON := mustJSON(t, map[string]interface{}{ + "detail": "The server experienced an internal error", + "type": "urn:ietf:params:acme:error:serverInternal", + }) + malformedErr := NewError(ErrorMalformedType, "malformed error") + malformedErr.Err = nil + malformedJSON := mustJSON(t, map[string]interface{}{ + "detail": "The request message was malformed", + "type": "urn:ietf:params:acme:error:malformed", + }) + withDetailJSON := mustJSON(t, map[string]interface{}{ + "detail": "Attestation statement cannot be verified: invalid property", + "type": "urn:ietf:params:acme:error:badAttestationStatement", + }) + tests := []struct { + name string + err *Error + want string + }{ + {"internal", NewError(ErrorServerInternalType, "").WithAdditionalErrorDetail(), internalJSON}, + {"nil err", malformedErr.WithAdditionalErrorDetail(), malformedJSON}, + {"detailed", NewError(ErrorBadAttestationStatementType, "invalid property").WithAdditionalErrorDetail(), withDetailJSON}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := json.Marshal(tt.err) + require.NoError(t, err) + + // tests if the additional error detail is included in the JSON representation + // of the ACME error. This is what is returned to ACME clients and being logged + // by the CA. + assert.JSONEq(t, tt.want, string(b)) + }) + } +}