From cd6482ecb8c420a1179541a1e28c2db4d6dda744 Mon Sep 17 00:00:00 2001
From: Aaron Lehmann <aaron.lehmann@docker.com>
Date: Mon, 14 Mar 2016 10:06:30 -0700
Subject: [PATCH] Don't return empty errcode.Errors slices

If this slice ends up empty after parsing the HTTP response body, it
means the body is not well-formed. We've probably encountered an error
message produced by something that uses a different JSON schema, or
an error that just happens to validate as JSON.

An empty errcode.Errors slice is not a very useful thing to return,
since its Error() output is just `<nil>`. Detect this case, and instend
return an UnexpectedHTTPResponseError.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>
---
 registry/client/errors.go      | 19 +++++++++++++++++--
 registry/client/errors_test.go | 19 +++++++++++++++++--
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/registry/client/errors.go b/registry/client/errors.go
index a528a865..043782bf 100644
--- a/registry/client/errors.go
+++ b/registry/client/errors.go
@@ -2,6 +2,7 @@ package client
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -10,6 +11,10 @@ import (
 	"github.com/docker/distribution/registry/api/errcode"
 )
 
+// ErrNoErrorsInBody is returned when a HTTP response body parses to an empty
+// errcode.Errors slice.
+var ErrNoErrorsInBody = errors.New("no error details found in HTTP response body")
+
 // UnexpectedHTTPStatusError is returned when an unexpected HTTP status is
 // returned when making a registry api call.
 type UnexpectedHTTPStatusError struct {
@@ -17,7 +22,7 @@ type UnexpectedHTTPStatusError struct {
 }
 
 func (e *UnexpectedHTTPStatusError) Error() string {
-	return fmt.Sprintf("Received unexpected HTTP status: %s", e.Status)
+	return fmt.Sprintf("received unexpected HTTP status: %s", e.Status)
 }
 
 // UnexpectedHTTPResponseError is returned when an expected HTTP status code
@@ -28,7 +33,7 @@ type UnexpectedHTTPResponseError struct {
 }
 
 func (e *UnexpectedHTTPResponseError) Error() string {
-	return fmt.Sprintf("Error parsing HTTP response: %s: %q", e.ParseErr.Error(), string(e.Response))
+	return fmt.Sprintf("error parsing HTTP response: %s: %q", e.ParseErr.Error(), string(e.Response))
 }
 
 func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
@@ -57,6 +62,16 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 			Response: body,
 		}
 	}
+
+	if len(errors) == 0 {
+		// If there was no error specified in the body, return
+		// UnexpectedHTTPResponseError.
+		return &UnexpectedHTTPResponseError{
+			ParseErr: ErrNoErrorsInBody,
+			Response: body,
+		}
+	}
+
 	return errors
 }
 
diff --git a/registry/client/errors_test.go b/registry/client/errors_test.go
index 80241a5a..1d60cd2d 100644
--- a/registry/client/errors_test.go
+++ b/registry/client/errors_test.go
@@ -59,6 +59,21 @@ func TestHandleErrorResponseExpectedStatusCode400ValidBody(t *testing.T) {
 	}
 }
 
+func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
+	json := `{"randomkey": "randomvalue"}`
+	response := &http.Response{
+		Status:     "404 Not Found",
+		StatusCode: 404,
+		Body:       nopCloser{bytes.NewBufferString(json)},
+	}
+	err := HandleErrorResponse(response)
+
+	expectedMsg := `error parsing HTTP response: no error details found in HTTP response body: "{\"randomkey\": \"randomvalue\"}"`
+	if !strings.Contains(err.Error(), expectedMsg) {
+		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
+	}
+}
+
 func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
 	json := "{invalid json}"
 	response := &http.Response{
@@ -68,7 +83,7 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
 	}
 	err := HandleErrorResponse(response)
 
-	expectedMsg := "Error parsing HTTP response: invalid character 'i' looking for beginning of object key string: \"{invalid json}\""
+	expectedMsg := "error parsing HTTP response: invalid character 'i' looking for beginning of object key string: \"{invalid json}\""
 	if !strings.Contains(err.Error(), expectedMsg) {
 		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 	}
@@ -82,7 +97,7 @@ func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) {
 	}
 	err := HandleErrorResponse(response)
 
-	expectedMsg := "Received unexpected HTTP status: 501 Not Implemented"
+	expectedMsg := "received unexpected HTTP status: 501 Not Implemented"
 	if !strings.Contains(err.Error(), expectedMsg) {
 		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 	}