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>
(cherry picked from commit 45b7b9cec3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Milos Gajdos 2023-08-24 08:08:04 +01:00 committed by Sebastiaan van Stijn
parent 10a7e4c483
commit 2ec0471bb5
No known key found for this signature in database
GPG key ID: 76698F39D527CE8C
4 changed files with 68 additions and 25 deletions

View file

@ -85,6 +85,7 @@ func TestUploadReadFrom(t *testing.T) {
}, },
Response: testutil.Response{ Response: testutil.Response{
StatusCode: http.StatusBadRequest, StatusCode: http.StatusBadRequest,
Headers: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
Body: []byte(` Body: []byte(`
{ "errors": { "errors":
[ [
@ -106,6 +107,7 @@ func TestUploadReadFrom(t *testing.T) {
}, },
Response: testutil.Response{ Response: testutil.Response{
StatusCode: http.StatusBadRequest, StatusCode: http.StatusBadRequest,
Headers: http.Header{"Content-Type": []string{"application/json"}},
Body: []byte("something bad happened"), Body: []byte("something bad happened"),
}, },
}, },

View file

@ -4,8 +4,8 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"io"
"io/ioutil" "io/ioutil"
"mime"
"net/http" "net/http"
"github.com/docker/distribution/registry/api/errcode" "github.com/docker/distribution/registry/api/errcode"
@ -38,13 +38,29 @@ func (e *UnexpectedHTTPResponseError) Error() string {
return fmt.Sprintf("error parsing HTTP %d response body: %s: %q", e.StatusCode, e.ParseErr.Error(), string(e.Response)) 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(resp *http.Response) error {
var errors errcode.Errors var errors errcode.Errors
body, err := ioutil.ReadAll(r) body, err := ioutil.ReadAll(resp.Body)
if err != nil { if err != nil {
return err return err
} }
statusCode := resp.StatusCode
ctHeader := resp.Header.Get("Content-Type")
if ctHeader == "" {
return makeError(statusCode, string(body))
}
contentType, _, err := mime.ParseMediaType(ctHeader)
if err != nil {
return fmt.Errorf("failed parsing content-type: %w", err)
}
if contentType != "application/json" && contentType != "application/vnd.api+json" {
return makeError(statusCode, string(body))
}
// For backward compatibility, handle irregularly formatted // For backward compatibility, handle irregularly formatted
// messages that contain a "details" field. // messages that contain a "details" field.
var detailsErr struct { var detailsErr struct {
@ -52,16 +68,7 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
} }
err = json.Unmarshal(body, &detailsErr) err = json.Unmarshal(body, &detailsErr)
if err == nil && detailsErr.Details != "" { if err == nil && detailsErr.Details != "" {
switch statusCode { return makeError(statusCode, detailsErr.Details)
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)
}
} }
if err := json.Unmarshal(body, &errors); err != nil { if err := json.Unmarshal(body, &errors); err != nil {
@ -85,6 +92,19 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
return errors 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 { func makeErrorList(err error) []error {
if errL, ok := err.(errcode.Errors); ok { if errL, ok := err.(errcode.Errors); ok {
return []error(errL) return []error(errL)
@ -121,11 +141,10 @@ func HandleErrorResponse(resp *http.Response) error {
} else { } else {
err.Message = err.Code.Message() err.Message = err.Code.Message()
} }
return mergeErrors(err, parseHTTPErrorResponse(resp))
return mergeErrors(err, parseHTTPErrorResponse(resp.StatusCode, resp.Body))
} }
} }
err := parseHTTPErrorResponse(resp.StatusCode, resp.Body) err := parseHTTPErrorResponse(resp)
if uErr, ok := err.(*UnexpectedHTTPResponseError); ok && resp.StatusCode == 401 { if uErr, ok := err.(*UnexpectedHTTPResponseError); ok && resp.StatusCode == 401 {
return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response) return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response)
} }

View file

@ -15,17 +15,18 @@ type nopCloser struct {
func (nopCloser) Close() error { return nil } func (nopCloser) Close() error { return nil }
func TestHandleErrorResponse401ValidBody(t *testing.T) { func TestHandleErrorResponse401ValidBody(t *testing.T) {
json := "{\"errors\":[{\"code\":\"UNAUTHORIZED\",\"message\":\"action requires authentication\"}]}" json := `{"errors":[{"code":"UNAUTHORIZED","message":"action requires authentication"}]}`
response := &http.Response{ response := &http.Response{
Status: "401 Unauthorized", Status: "401 Unauthorized",
StatusCode: 401, StatusCode: 401,
Body: nopCloser{bytes.NewBufferString(json)}, Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
} }
err := HandleErrorResponse(response) err := HandleErrorResponse(response)
expectedMsg := "unauthorized: action requires authentication" expectedMsg := "unauthorized: action requires authentication"
if !strings.Contains(err.Error(), expectedMsg) { if !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) t.Errorf("Expected %q, got: %q", expectedMsg, err.Error())
} }
} }
@ -35,27 +36,29 @@ func TestHandleErrorResponse401WithInvalidBody(t *testing.T) {
Status: "401 Unauthorized", Status: "401 Unauthorized",
StatusCode: 401, StatusCode: 401,
Body: nopCloser{bytes.NewBufferString(json)}, Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
} }
err := HandleErrorResponse(response) err := HandleErrorResponse(response)
expectedMsg := "unauthorized: authentication required" expectedMsg := "unauthorized: authentication required"
if !strings.Contains(err.Error(), expectedMsg) { if !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) t.Errorf("Expected %q, got: %q", expectedMsg, err.Error())
} }
} }
func TestHandleErrorResponseExpectedStatusCode400ValidBody(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{ response := &http.Response{
Status: "400 Bad Request", Status: "400 Bad Request",
StatusCode: 400, StatusCode: 400,
Body: nopCloser{bytes.NewBufferString(json)}, Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json"}},
} }
err := HandleErrorResponse(response) err := HandleErrorResponse(response)
expectedMsg := "digest invalid: provided digest does not match" expectedMsg := "digest invalid: provided digest does not match"
if !strings.Contains(err.Error(), expectedMsg) { if !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) t.Errorf("Expected %q, got: %q", expectedMsg, err.Error())
} }
} }
@ -65,12 +68,13 @@ func TestHandleErrorResponseExpectedStatusCode404EmptyErrorSlice(t *testing.T) {
Status: "404 Not Found", Status: "404 Not Found",
StatusCode: 404, StatusCode: 404,
Body: nopCloser{bytes.NewBufferString(json)}, Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
} }
err := HandleErrorResponse(response) err := HandleErrorResponse(response)
expectedMsg := `error parsing HTTP 404 response body: no error details found in HTTP response body: "{\"randomkey\": \"randomvalue\"}"` expectedMsg := `error parsing HTTP 404 response body: no error details found in HTTP response body: "{\"randomkey\": \"randomvalue\"}"`
if !strings.Contains(err.Error(), expectedMsg) { if !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) t.Errorf("Expected %q, got: %q", expectedMsg, err.Error())
} }
} }
@ -80,12 +84,13 @@ func TestHandleErrorResponseExpectedStatusCode404InvalidBody(t *testing.T) {
Status: "404 Not Found", Status: "404 Not Found",
StatusCode: 404, StatusCode: 404,
Body: nopCloser{bytes.NewBufferString(json)}, Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json"}},
} }
err := HandleErrorResponse(response) err := HandleErrorResponse(response)
expectedMsg := "error parsing HTTP 404 response body: invalid character 'i' looking for beginning of object key string: \"{invalid json}\"" expectedMsg := "error parsing HTTP 404 response body: 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 %q, got: %q", expectedMsg, err.Error())
} }
} }
@ -94,12 +99,13 @@ func TestHandleErrorResponseUnexpectedStatusCode501(t *testing.T) {
Status: "501 Not Implemented", Status: "501 Not Implemented",
StatusCode: 501, StatusCode: 501,
Body: nopCloser{bytes.NewBufferString("{\"Error Encountered\" : \"Function not implemented.\"}")}, Body: nopCloser{bytes.NewBufferString("{\"Error Encountered\" : \"Function not implemented.\"}")},
Header: http.Header{"Content-Type": []string{"application/json"}},
} }
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 %q, got: %q", expectedMsg, err.Error())
} }
} }
@ -109,11 +115,26 @@ func TestHandleErrorResponseInsufficientPrivileges403(t *testing.T) {
Status: "403 Forbidden", Status: "403 Forbidden",
StatusCode: 403, StatusCode: 403,
Body: nopCloser{bytes.NewBufferString(json)}, Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json"}},
} }
err := HandleErrorResponse(response) err := HandleErrorResponse(response)
expectedMsg := "denied: requesting higher privileges than access token allows" expectedMsg := "denied: requesting higher privileges than access token allows"
if !strings.Contains(err.Error(), expectedMsg) { if !strings.Contains(err.Error(), expectedMsg) {
t.Errorf("Expected \"%s\", got: \"%s\"", expectedMsg, err.Error()) t.Errorf("Expected %q, got: %q", 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 %q, got: %q", msg, err.Error())
} }
} }

View file

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