TrueCloudLab/distribution
13
0
Fork 3
Code Issues 2 Pull requests Projects Releases 2 Packages Wiki Activity Actions

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

Browse source 
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>
This commit is contained in:
Milos Gajdos 2023-08-24 08:08:04 +01:00
parent 4f7424c8eb
commit 45b7b9cec3
No known key found for this signature in database
GPG key ID: 01300E5E6D417439
4 changed files with 70 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
registry/client/blob_writer_test.go
View file

@ -86,6 +86,7 @@ func TestUploadReadFrom(t *testing.T) {
},
Response: testutil.Response{
StatusCode: http.StatusBadRequest,
Headers: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
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; charset=utf-8"}},
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"),
},
},

50
registry/client/errors.go
View file

@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"mime"
"net/http"
"github.com/distribution/distribution/v3/registry/api/errcode"
@ -37,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))
}
func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
func parseHTTPErrorResponse(resp *http.Response) error {
var errors errcode.Errors
body, err := io.ReadAll(r)
body, err := io.ReadAll(resp.Body)
if err != nil {
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
// messages that contain a "details" field.
var detailsErr struct {
@ -51,16 +68,7 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
}
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 +92,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 +141,10 @@ func HandleErrorResponse(resp *http.Response) error {
} else {
err.Message = err.Code.Message()
}
return mergeErrors(err, parseHTTPErrorResponse(resp.StatusCode, resp.Body))
return mergeErrors(err, parseHTTPErrorResponse(resp))
}
}
err := parseHTTPErrorResponse(resp.StatusCode, resp.Body)
err := parseHTTPErrorResponse(resp)
if uErr, ok := err.(*UnexpectedHTTPResponseError); ok && resp.StatusCode == 401 {
return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response)
}

39
registry/client/errors_test.go
View file

@ -15,17 +15,18 @@ 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; charset=utf-8"}},
}
err := HandleErrorResponse(response)
expectedMsg := "unauthorized: action requires authentication"
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",
StatusCode: 401,
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
}
err := HandleErrorResponse(response)
expectedMsg := "unauthorized: authentication required"
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) {
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)
expectedMsg := "digest invalid: provided digest does not match"
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",
StatusCode: 404,
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}},
}
err := HandleErrorResponse(response)
expectedMsg := `error parsing HTTP 404 response body: 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())
t.Errorf("Expected %q, got: %q", expectedMsg, err.Error())
}
}
@ -80,12 +84,13 @@ 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)
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) {
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",
StatusCode: 501,
Body: nopCloser{bytes.NewBufferString("{\"Error Encountered\" : \"Function not implemented.\"}")},
Header: http.Header{"Content-Type": []string{"application/json"}},
}
err := HandleErrorResponse(response)
expectedMsg := "received unexpected HTTP status: 501 Not Implemented"
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",
StatusCode: 403,
Body: nopCloser{bytes.NewBufferString(json)},
Header: http.Header{"Content-Type": []string{"application/json"}},
}
err := HandleErrorResponse(response)
expectedMsg := "denied: requesting higher privileges than access token allows"
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())
}
}

1
registry/client/repository_test.go
View file

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