Browse Source

api/server/httputils: matchesContentType(): return error instead of logging

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 3 years ago
parent
commit
ef490cae45
2 changed files with 25 additions and 18 deletions
  1. 9 12
      api/server/httputils/httputils.go
  2. 16 6
      api/server/httputils/httputils_test.go

+ 9 - 12
api/server/httputils/httputils.go

@@ -9,7 +9,6 @@ import (
 
 
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/errdefs"
 	"github.com/pkg/errors"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 )
 )
 
 
 // APIVersionKey is the client's requested API version.
 // APIVersionKey is the client's requested API version.
@@ -49,17 +48,12 @@ func CheckForJSON(r *http.Request) error {
 	ct := r.Header.Get("Content-Type")
 	ct := r.Header.Get("Content-Type")
 
 
 	// No Content-Type header is ok as long as there's no Body
 	// No Content-Type header is ok as long as there's no Body
-	if ct == "" {
-		if r.Body == nil || r.ContentLength == 0 {
-			return nil
-		}
+	if ct == "" && (r.Body == nil || r.ContentLength == 0) {
+		return nil
 	}
 	}
 
 
 	// Otherwise it better be json
 	// Otherwise it better be json
-	if matchesContentType(ct, "application/json") {
-		return nil
-	}
-	return errdefs.InvalidParameter(errors.Errorf("Content-Type specified (%s) must be 'application/json'", ct))
+	return matchesContentType(ct, "application/json")
 }
 }
 
 
 // ParseForm ensures the request form is parsed even with invalid content types.
 // ParseForm ensures the request form is parsed even with invalid content types.
@@ -89,10 +83,13 @@ func VersionFromContext(ctx context.Context) string {
 }
 }
 
 
 // matchesContentType validates the content type against the expected one
 // matchesContentType validates the content type against the expected one
-func matchesContentType(contentType, expectedType string) bool {
+func matchesContentType(contentType, expectedType string) error {
 	mimetype, _, err := mime.ParseMediaType(contentType)
 	mimetype, _, err := mime.ParseMediaType(contentType)
 	if err != nil {
 	if err != nil {
-		logrus.Errorf("Error parsing media type: %s error: %v", contentType, err)
+		return errdefs.InvalidParameter(errors.Wrapf(err, "malformed Content-Type header (%s)", contentType))
+	}
+	if mimetype != expectedType {
+		return errdefs.InvalidParameter(errors.Errorf("unsupported Content-Type header (%s): must be '%s'", contentType, expectedType))
 	}
 	}
-	return err == nil && mimetype == expectedType
+	return nil
 }
 }

+ 16 - 6
api/server/httputils/httputils_test.go

@@ -4,15 +4,25 @@ import "testing"
 
 
 // matchesContentType
 // matchesContentType
 func TestJsonContentType(t *testing.T) {
 func TestJsonContentType(t *testing.T) {
-	if !matchesContentType("application/json", "application/json") {
-		t.Fail()
+	err := matchesContentType("application/json", "application/json")
+	if err != nil {
+		t.Error(err)
 	}
 	}
 
 
-	if !matchesContentType("application/json; charset=utf-8", "application/json") {
-		t.Fail()
+	err = matchesContentType("application/json; charset=utf-8", "application/json")
+	if err != nil {
+		t.Error(err)
 	}
 	}
 
 
-	if matchesContentType("dockerapplication/json", "application/json") {
-		t.Fail()
+	expected := "unsupported Content-Type header (dockerapplication/json): must be 'application/json'"
+	err = matchesContentType("dockerapplication/json", "application/json")
+	if err == nil || err.Error() != expected {
+		t.Errorf(`expected "%s", got "%v"`, expected, err)
+	}
+
+	expected = "malformed Content-Type header (foo;;;bar): mime: invalid media parameter"
+	err = matchesContentType("foo;;;bar", "application/json")
+	if err == nil || err.Error() != expected {
+		t.Errorf(`expected "%s", got "%v"`, expected, err)
 	}
 	}
 }
 }