Make it easier to instantiate ProblemDetails (letsencrypt#1851)

cpu · jsha · commit 77030c3eb100 · 2016-05-31T14:05:37.000-07:00
Several of the `ProblemType`s had convenience functions to instantiate `ProblemDetails`s using their type and a detail message. Where these existed I did a quick scan of the codebase to convert places where callers were explicitly constructing the `ProblemDetails` to use the convenience function. For the `ProblemType`s that did not have such a function, I created one and then converted callers to use it. Solves letsencrypt#1837.
diff --git a/bdns/problem.go b/bdns/problem.go
@@ -60,13 +60,7 @@ const detailServerFailure = "server failure at resolver"
 // record type and domain given.
 func ProblemDetailsFromDNSError(err error) *probs.ProblemDetails {
 	if dnsErr, ok := err.(*DNSError); ok {
-		return &probs.ProblemDetails{
-			Type:   probs.ConnectionProblem,
-			Detail: dnsErr.Error(),
-		}
-	}
-	return &probs.ProblemDetails{
-		Type:   probs.ConnectionProblem,
-		Detail: detailServerFailure,
+		return probs.ConnectionFailure(dnsErr.Error())
 	}
+	return probs.ConnectionFailure(detailServerFailure)
 }
diff --git a/core/util.go b/core/util.go
@@ -131,11 +131,7 @@ func ProblemDetailsForError(err error, msg string) *probs.ProblemDetails {
 	case SignatureValidationError:
 		return probs.Malformed(fmt.Sprintf("%s :: %s", msg, err))
 	case RateLimitedError:
-		return &probs.ProblemDetails{
-			Type:       probs.RateLimitedProblem,
-			Detail:     fmt.Sprintf("%s :: %s", msg, err),
-			HTTPStatus: statusTooManyRequests,
-		}
+		return probs.RateLimited(fmt.Sprintf("%s :: %s", msg, err))
 	case BadNonceError:
 		return probs.BadNonce(fmt.Sprintf("%s :: %s", msg, err))
 	default:
diff --git a/probs/probs.go b/probs/probs.go
@@ -142,3 +142,50 @@ func ContentLengthRequired() *ProblemDetails {
 		HTTPStatus: http.StatusLengthRequired,
 	}
 }
+
+// InvalidEmail returns a ProblemDetails representing an invalid email address
+// error
+func InvalidEmail(detail string) *ProblemDetails {
+	return &ProblemDetails{
+		Type:       InvalidEmailProblem,
+		Detail:     detail,
+		HTTPStatus: http.StatusBadRequest,
+	}
+}
+
+// ConnectionFailure returns a ProblemDetails representing a ConnectionProblem
+// error
+func ConnectionFailure(detail string) *ProblemDetails {
+	return &ProblemDetails{
+		Type:       ConnectionProblem,
+		Detail:     detail,
+		HTTPStatus: http.StatusBadRequest,
+	}
+}
+
+// UnknownHost returns a ProblemDetails representing an UnknownHostProblem error
+func UnknownHost(detail string) *ProblemDetails {
+	return &ProblemDetails{
+		Type:       UnknownHostProblem,
+		Detail:     detail,
+		HTTPStatus: http.StatusBadRequest,
+	}
+}
+
+// RateLimited returns a ProblemDetails representing a RateLimitedProblem error
+func RateLimited(detail string) *ProblemDetails {
+	return &ProblemDetails{
+		Type:       RateLimitedProblem,
+		Detail:     detail,
+		HTTPStatus: statusTooManyRequests,
+	}
+}
+
+// TLSError returns a ProblemDetails representing a TLSProblem error
+func TLSError(detail string) *ProblemDetails {
+	return &ProblemDetails{
+		Type:       TLSProblem,
+		Detail:     detail,
+		HTTPStatus: http.StatusBadRequest,
+	}
+}
diff --git a/probs/probs_test.go b/probs/probs_test.go
@@ -43,3 +43,36 @@ func TestProblemDetailsToStatusCode(t *testing.T) {
 		}
 	}
 }
+
+func TestProblemDetailsConvenience(t *testing.T) {
+	testCases := []struct {
+		pb           *ProblemDetails
+		expectedType ProblemType
+		statusCode   int
+		detail       string
+	}{
+		{InvalidEmail("invalid email detail"), InvalidEmailProblem, http.StatusBadRequest, "invalid email detail"},
+		{ConnectionFailure("connection failure detail"), ConnectionProblem, http.StatusBadRequest, "connection failure detail"},
+		{Malformed("malformed detail"), MalformedProblem, http.StatusBadRequest, "malformed detail"},
+		{ServerInternal("internal error detail"), ServerInternalProblem, http.StatusInternalServerError, "internal error detail"},
+		{Unauthorized("unauthorized detail"), UnauthorizedProblem, http.StatusForbidden, "unauthorized detail"},
+		{UnknownHost("unknown host detail"), UnknownHostProblem, http.StatusBadRequest, "unknown host detail"},
+		{RateLimited("rate limited detail"), RateLimitedProblem, statusTooManyRequests, "rate limited detail"},
+		{BadNonce("bad nonce detail"), BadNonceProblem, http.StatusBadRequest, "bad nonce detail"},
+		{TLSError("TLS error detail"), TLSProblem, http.StatusBadRequest, "TLS error detail"},
+	}
+
+	for _, c := range testCases {
+		if c.pb.Type != c.expectedType {
+			t.Errorf("Incorrect problem type. Expected %s got %s", c.expectedType, c.pb.Type)
+		}
+
+		if c.pb.HTTPStatus != c.statusCode {
+			t.Errorf("Incorrect HTTP Status. Expected %d got %d", c.statusCode, c.pb.HTTPStatus)
+		}
+
+		if c.pb.Detail != c.detail {
+			t.Errorf("Incorrect detail message. Expected %s got %s", c.detail, c.pb.Detail)
+		}
+	}
+}
diff --git a/ra/registration-authority.go b/ra/registration-authority.go
@@ -104,16 +104,10 @@ const (
 func validateEmail(ctx context.Context, address string, resolver bdns.DNSResolver) (prob *probs.ProblemDetails) {
 	emails, err := mail.ParseAddressList(address)
 	if err != nil {
-		return &probs.ProblemDetails{
-			Type:   probs.InvalidEmailProblem,
-			Detail: unparseableEmailDetail,
-		}
+		return probs.InvalidEmail(unparseableEmailDetail)
 	}
 	if len(emails) > 1 {
-		return &probs.ProblemDetails{
-			Type:   probs.InvalidEmailProblem,
-			Detail: multipleAddressDetail,
-		}
+		return probs.InvalidEmail(multipleAddressDetail)
 	}
 	splitEmail := strings.SplitN(emails[0].Address, "@", -1)
 	domain := strings.ToLower(splitEmail[len(splitEmail)-1])
@@ -147,10 +141,7 @@ func validateEmail(ctx context.Context, address string, resolver bdns.DNSResolve
 		return nil
 	}
 
-	return &probs.ProblemDetails{
-		Type:   probs.InvalidEmailProblem,
-		Detail: emptyDNSResponseDetail,
-	}
+	return probs.InvalidEmail(emptyDNSResponseDetail)
 }
 
 type certificateRequestEvent struct {
diff --git a/va/validation-authority.go b/va/validation-authority.go
