Dont parse errors as JSON unless Content-Type is set to JSON

Client attempts to parse the body of every error it receives as JSON
regardless of the content-type. This commit rectifies by only parsing
he error body as JSON if the Content-Type header is set to
either "application/json" or "application/vnd.api+json".

Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>

registry/client/blob_writer_test.go

@@ -86,6 +86,7 @@ func TestUploadReadFrom(t *testing.T) {
 			},
 			Response: testutil.Response{
 				StatusCode: http.StatusBadRequest,
+				Headers:    http.Header{"Content-Type": []string{"application/json"}},
 				Body: []byte(`
 					{ "errors":
 						[
@@ -107,6 +108,7 @@ func TestUploadReadFrom(t *testing.T) {
 			},
 			Response: testutil.Response{
 				StatusCode: http.StatusBadRequest,
+				Headers:    http.Header{"Content-Type": []string{"application/json"}},
 				Body:       []byte("something bad happened"),
 			},
 		},
@@ -372,6 +374,7 @@ func TestUploadWrite(t *testing.T) {
 			},
 			Response: testutil.Response{
 				StatusCode: http.StatusBadRequest,
+				Headers:    http.Header{"Content-Type": []string{"application/json"}},
 				Body: []byte(`
 					{ "errors":
 						[
@@ -393,6 +396,7 @@ func TestUploadWrite(t *testing.T) {
 			},
 			Response: testutil.Response{
 				StatusCode: http.StatusBadRequest,
+				Headers:    http.Header{"Content-Type": []string{"application/json"}},
 				Body:       []byte("something bad happened"),
 			},
 		},

registry/client/errors.go

@@ -37,30 +37,25 @@ func (e *UnexpectedHTTPResponseError) Error() string {
 	return fmt.Sprintf("error parsing HTTP %d response body: %s: %q", e.StatusCode, e.ParseErr.Error(), string(e.Response))
 }
 
-func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
+func parseHTTPErrorResponse(statusCode int, r io.Reader, contentType string) error {
 	var errors errcode.Errors
 	body, err := io.ReadAll(r)
 	if err != nil {
 		return err
 	}
 
+	if contentType != "application/json" && contentType != "application/vnd.api+json" {
+		return makeError(statusCode, string(body))
+	}
+
 	// For backward compatibility, handle irregularly formatted
 	// messages that contain a "details" field.
 	var detailsErr struct {
 		Details string `json:"details"`
 	}
 	err = json.Unmarshal(body, &detailsErr)
 	if err == nil && detailsErr.Details != "" {
-		switch statusCode {
-		case http.StatusUnauthorized:
-			return errcode.ErrorCodeUnauthorized.WithMessage(detailsErr.Details)
-		case http.StatusForbidden:
-			return errcode.ErrorCodeDenied.WithMessage(detailsErr.Details)
-		case http.StatusTooManyRequests:
-			return errcode.ErrorCodeTooManyRequests.WithMessage(detailsErr.Details)
-		default:
-			return errcode.ErrorCodeUnknown.WithMessage(detailsErr.Details)
-		}
+		return makeError(statusCode, detailsErr.Details)
 	}
 
 	if err := json.Unmarshal(body, &errors); err != nil {
@@ -84,6 +79,19 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 	return errors
 }
 
+func makeError(statusCode int, details string) error {
+	switch statusCode {
+	case http.StatusUnauthorized:
+		return errcode.ErrorCodeUnauthorized.WithMessage(details)
+	case http.StatusForbidden:
+		return errcode.ErrorCodeDenied.WithMessage(details)
+	case http.StatusTooManyRequests:
+		return errcode.ErrorCodeTooManyRequests.WithMessage(details)
+	default:
+		return errcode.ErrorCodeUnknown.WithMessage(details)
+	}
+}
+
 func makeErrorList(err error) []error {
 	if errL, ok := err.(errcode.Errors); ok {
 		return []error(errL)
@@ -120,11 +128,11 @@ func HandleErrorResponse(resp *http.Response) error {
 				} else {
 					err.Message = err.Code.Message()
 				}
-
-				return mergeErrors(err, parseHTTPErrorResponse(resp.StatusCode, resp.Body))
+				errResp := parseHTTPErrorResponse(resp.StatusCode, resp.Body, resp.Header.Get("Content-ype"))
+				return mergeErrors(err, errResp)
 			}
 		}
-		err := parseHTTPErrorResponse(resp.StatusCode, resp.Body)
+		err := parseHTTPErrorResponse(resp.StatusCode, resp.Body, resp.Header.Get("Content-Type"))
 		if uErr, ok := err.(*UnexpectedHTTPResponseError); ok && resp.StatusCode == 401 {
 			return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response)
 		}

registry/client/errors_test.go

@@ -15,11 +15,12 @@ type nopCloser struct {
 func (nopCloser) Close() error { return nil }
 
 func TestHandleErrorResponse401ValidBody(t *testing.T) {
-	json := "{\"errors\":[{\"code\":\"UNAUTHORIZED\",\"message\":\"action requires authentication\"}]}"
+	json := `{"errors":[{"code":"UNAUTHORIZED","message":"action requires authentication"}]}`
 	response := &http.Response{
 		Status:     "401 Unauthorized",
 		StatusCode: 401,
 		Body:       nopCloser{bytes.NewBufferString(json)},
+		Header:     http.Header{"Content-Type": []string{"application/json"}},
 	}
 	err := HandleErrorResponse(response)
 
@@ -35,6 +36,7 @@ func TestHandleErrorResponse401WithInvalidBody(t *testing.T) {
 		Status:     "401 Unauthorized",
 		StatusCode: 401,
 		Body:       nopCloser{bytes.NewBufferString(json)},
+		Header:     http.Header{"Content-Type": []string{"application/json"}},
 	}
 	err := HandleErrorResponse(response)
 
@@ -45,11 +47,12 @@ func TestHandleErrorResponse401WithInvalidBody(t *testing.T) {
 }
 
 func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) {
-	json := "{\"errors\":[{\"code\":\"DIGEST_INVALID\",\"message\":\"provided digest does not match\"}]}"
+	json := `{"errors":[{"code":"DIGEST_INVALID","message":"provided digest does not match"}]}`
 	response := &http.Response{
 		Status:     "400 Bad Request",
 		StatusCode: 400,
 		Body:       nopCloser{bytes.NewBufferString(json)},
+		Header:     http.Header{"Content-Type": []string{"application/json"}},
 	}
 	err := HandleErrorResponse(response)
 
@@ -65,6 +68,7 @@ func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
 		Status:     "404 Not Found",
 		StatusCode: 404,
 		Body:       nopCloser{bytes.NewBufferString(json)},
+		Header:     http.Header{"Content-Type": []string{"application/json"}},
 	}
 	err := HandleErrorResponse(response)
 
@@ -80,6 +84,7 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
 		Status:     "404 Not Found",
 		StatusCode: 404,
 		Body:       nopCloser{bytes.NewBufferString(json)},
+		Header:     http.Header{"Content-Type": []string{"application/json"}},
 	}
 	err := HandleErrorResponse(response)
 
@@ -94,6 +99,7 @@ func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) {
 		Status:     "501 Not Implemented",
 		StatusCode: 501,
 		Body:       nopCloser{bytes.NewBufferString("{\"Error Encountered\" : \"Function not implemented.\"}")},
+		Header:     http.Header{"Content-Type": []string{"application/json"}},
 	}
 	err := HandleErrorResponse(response)
 
@@ -109,6 +115,7 @@ func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) {
 		Status:     "403 Forbidden",
 		StatusCode: 403,
 		Body:       nopCloser{bytes.NewBufferString(json)},
+		Header:     http.Header{"Content-Type": []string{"application/json"}},
 	}
 	err := HandleErrorResponse(response)
 
@@ -117,3 +124,17 @@ func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) {
 		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 	}
 }
+
+func TestHandleErrorResponseNonJson(t *testing.T) {
+	msg := `{"details":"requesting higher privileges than access token allows"}`
+	response := &http.Response{
+		Status:     "403 Forbidden",
+		StatusCode: 403,
+		Body:       nopCloser{bytes.NewBufferString(msg)},
+	}
+	err := HandleErrorResponse(response)
+
+	if !strings.Contains(err.Error(), msg) {
+		t.Errorf("Expected \"%s\", got: \"%s\"", msg, err.Error())
+	}
+}

registry/client/repository_test.go

@@ -1603,6 +1603,7 @@ func TestManifestUnauthorized(t *testing.T) {
 		},
 		Response: testutil.Response{
 			StatusCode: http.StatusUnauthorized,
+			Headers:    http.Header{"Content-Type": []string{"application/json"}},
 			Body:       []byte("<html>garbage</html>"),
 		},
 	})