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>master
							parent
							
								
									1d5d54978a
								
							
						
					
					
						commit
						c94c2a47a3
					
				| 
						 | 
					@ -2,6 +2,7 @@ package client
 | 
				
			||||||
 | 
					
 | 
				
			||||||
import (
 | 
					import (
 | 
				
			||||||
	"encoding/json"
 | 
						"encoding/json"
 | 
				
			||||||
 | 
						"errors"
 | 
				
			||||||
	"fmt"
 | 
						"fmt"
 | 
				
			||||||
	"io"
 | 
						"io"
 | 
				
			||||||
	"io/ioutil"
 | 
						"io/ioutil"
 | 
				
			||||||
| 
						 | 
					@ -10,6 +11,10 @@ import (
 | 
				
			||||||
	"github.com/docker/distribution/registry/api/errcode"
 | 
						"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
 | 
					// UnexpectedHTTPStatusError is returned when an unexpected HTTP status is
 | 
				
			||||||
// returned when making a registry api call.
 | 
					// returned when making a registry api call.
 | 
				
			||||||
type UnexpectedHTTPStatusError struct {
 | 
					type UnexpectedHTTPStatusError struct {
 | 
				
			||||||
| 
						 | 
					@ -17,7 +22,7 @@ type UnexpectedHTTPStatusError struct {
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func (e *UnexpectedHTTPStatusError) Error() string {
 | 
					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
 | 
					// UnexpectedHTTPResponseError is returned when an expected HTTP status code
 | 
				
			||||||
| 
						 | 
					@ -28,7 +33,7 @@ type UnexpectedHTTPResponseError struct {
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
func (e *UnexpectedHTTPResponseError) Error() string {
 | 
					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 {
 | 
					func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 | 
				
			||||||
| 
						 | 
					@ -57,6 +62,16 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 | 
				
			||||||
			Response: body,
 | 
								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
 | 
						return errors
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -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) {
 | 
					func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
 | 
				
			||||||
	json := "{invalid json}"
 | 
						json := "{invalid json}"
 | 
				
			||||||
	response := &http.Response{
 | 
						response := &http.Response{
 | 
				
			||||||
| 
						 | 
					@ -68,7 +83,7 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	err := HandleErrorResponse(response)
 | 
						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) {
 | 
						if !strings.Contains(err.Error(), expectedMsg) {
 | 
				
			||||||
		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 | 
							t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					@ -82,7 +97,7 @@ func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) {
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	err := HandleErrorResponse(response)
 | 
						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) {
 | 
						if !strings.Contains(err.Error(), expectedMsg) {
 | 
				
			||||||
		t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 | 
							t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error())
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue