fixes #246 content type for delete is optional

go-openapi · Feb 9, 2016 · 9bd8bdc · 9bd8bdc
1 parent 84bc143
commit 9bd8bdc
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 14 deletions.
diff --git a/headers.go b/headers.go
@@ -22,12 +22,15 @@ import (
 )
 
 // ContentType parses a content type header
-func ContentType(headers http.Header) (string, string, *errors.ParseError) {
+func ContentType(headers http.Header, optional bool) (string, string, *errors.ParseError) {
 	ct := headers.Get(HeaderContentType)
 	orig := ct
-	if ct == "" {
+	if ct == "" && !optional {
 		ct = DefaultMime
 	}
+	if ct == "" {
+		return "", "", nil
+	}
 
 	mt, opts, err := mime.ParseMediaType(ct)
 	if err != nil {

diff --git a/headers_test.go b/headers_test.go
@@ -46,7 +46,7 @@ func TestParseContentType(t *testing.T) {
 		} else {
 			headers.Del("content-type")
 		}
-		ct, cs, err := ContentType(headers)
+		ct, cs, err := ContentType(headers, false)
 		if v.err == nil {
 			assert.NoError(t, err, "input: %q", v.hdr)
 		} else {

diff --git a/middleware/context.go b/middleware/context.go
@@ -209,7 +209,7 @@ func (c *Context) BindValidRequest(request *http.Request, route *MatchedRoute, b
 
 	// check and validate content type, select consumer
 	if httpkit.CanHaveBody(request.Method) {
-		ct, _, err := httpkit.ContentType(request.Header)
+		ct, _, err := httpkit.ContentType(request.Header, httpkit.IsDelete(request.Method))
 		if err != nil {
 			res = append(res, err)
 		} else {
@@ -221,7 +221,7 @@ func (c *Context) BindValidRequest(request *http.Request, route *MatchedRoute, b
 	}
 
 	// check and validate the response format
-	if len(res) == 0 {
+	if len(res) == 0 && httpkit.NeedsContentType(request.Method) {
 		if str := NegotiateContentType(request, route.Produces, ""); str == "" {
 			res = append(res, errors.InvalidResponseFormat(request.Header.Get(httpkit.HeaderAccept), route.Produces))
 		}
@@ -250,7 +250,7 @@ func (c *Context) ContentType(request *http.Request) (string, string, *errors.Pa
 		}
 	}
 
-	mt, cs, err := httpkit.ContentType(request.Header)
+	mt, cs, err := httpkit.ContentType(request.Header, httpkit.IsDelete(request.Method))
 	if err != nil {
 		return "", "", err
 	}
@@ -364,7 +364,12 @@ func (c *Context) Respond(rw http.ResponseWriter, r *http.Request, produces []st
 		producers := route.Producers
 		prod, ok := producers[format]
 		if !ok {
-			panic(errors.New(http.StatusInternalServerError, "can't find a producer for "+format))
+			prods := c.api.ProducersFor([]string{c.api.DefaultProduces()})
+			pr, ok := prods[c.api.DefaultProduces()]
+			if !ok {
+				panic(errors.New(http.StatusInternalServerError, "can't find a producer for "+format))
+			}
+			prod = pr
 		}
 		resp.WriteResponse(rw, prod)
 		return
@@ -407,7 +412,14 @@ func (c *Context) Respond(rw http.ResponseWriter, r *http.Request, produces []st
 		producers := route.Producers
 		prod, ok := producers[format]
 		if !ok {
-			panic(errors.New(http.StatusInternalServerError, "can't find a producer for "+format))
+			if !ok {
+				prods := c.api.ProducersFor([]string{c.api.DefaultProduces()})
+				pr, ok := prods[c.api.DefaultProduces()]
+				if !ok {
+					panic(errors.New(http.StatusInternalServerError, "can't find a producer for "+format))
+				}
+				prod = pr
+			}
 		}
 		if err := prod.Produce(rw, data); err != nil {
 			panic(err) // let the recovery middleware deal with this

diff --git a/middleware/context_test.go b/middleware/context_test.go
@@ -21,11 +21,39 @@ import (
 	"testing"
 
 	"github.com/go-swagger/go-swagger/httpkit"
+	"github.com/go-swagger/go-swagger/httpkit/middleware/untyped"
 	"github.com/go-swagger/go-swagger/internal/testing/petstore"
+	"github.com/go-swagger/go-swagger/spec"
 	"github.com/gorilla/context"
 	"github.com/stretchr/testify/assert"
 )
 
+type stubOperationHandler struct {
+}
+
+func (s *stubOperationHandler) ParameterModel() interface{} {
+	return nil
+}
+
+func (s *stubOperationHandler) Handle(params interface{}) (interface{}, error) {
+	return nil, nil
+}
+func TestContentType_Issue264(t *testing.T) {
+	swspec, err := spec.Load("../../fixtures/bugs/264/swagger.yml")
+	if assert.NoError(t, err) {
+		api := untyped.NewAPI(swspec)
+		api.RegisterConsumer("application/json", httpkit.JSONConsumer())
+		api.RegisterProducer("application/json", httpkit.JSONProducer())
+		api.RegisterOperation("delete", "/key/{id}", new(stubOperationHandler))
+
+		handler := Serve(swspec, api)
+		request, _ := http.NewRequest("DELETE", "/key/1", nil)
+		recorder := httptest.NewRecorder()
+		handler.ServeHTTP(recorder, request)
+		assert.Equal(t, 200, recorder.Code)
+	}
+}
+
 func TestServe(t *testing.T) {
 	spec, api := petstore.NewAPI(t)
 	handler := Serve(spec, api)
@@ -163,9 +191,9 @@ func TestContextRender(t *testing.T) {
 	recorder = httptest.NewRecorder()
 	assert.Panics(t, func() { ctx.Respond(recorder, request, []string{ct}, ri, map[int]interface{}{1: "hello"}) })
 
-	recorder = httptest.NewRecorder()
-	request, _ = http.NewRequest("GET", "/pets", nil)
-	assert.Panics(t, func() { ctx.Respond(recorder, request, []string{}, ri, map[string]interface{}{"name": "hello"}) })
+	// recorder = httptest.NewRecorder()
+	// request, _ = http.NewRequest("GET", "/pets", nil)
+	// assert.Panics(t, func() { ctx.Respond(recorder, request, []string{}, ri, map[string]interface{}{"name": "hello"}) })
 
 	recorder = httptest.NewRecorder()
 	request, _ = http.NewRequest("DELETE", "/pets/1", nil)

diff --git a/middleware/parameter.go b/middleware/parameter.go
@@ -81,6 +81,8 @@ func (p *untypedParamBinder) typeForSchema(tpe, format string, items *spec.Items
 			return reflect.TypeOf(int32(0))
 		case "int64":
 			return reflect.TypeOf(int64(0))
+		default:
+			return reflect.TypeOf(int64(0))
 		}
 
 	case "number":
@@ -175,7 +177,7 @@ func (p *untypedParamBinder) Bind(request *http.Request, routeParams RouteParams
 		var err error
 		var mt string
 
-		mt, _, e := httpkit.ContentType(request.Header)
+		mt, _, e := httpkit.ContentType(request.Header, httpkit.IsDelete(request.Method))
 		if e != nil {
 			// because of the interface conversion go thinks the error is not nil
 			// so we first check for nil and then set the err var if it's not nil

diff --git a/middleware/validation.go b/middleware/validation.go
@@ -102,7 +102,7 @@ func (v *validation) contentType() {
 		ct, _, err := v.context.ContentType(v.request)
 		if err != nil {
 			v.result = append(v.result, err)
-		} else {
+		} else if httpkit.NeedsContentType(v.request.Method) {
 			if err := validateContentType(v.route.Consumes, ct); err != nil {
 				v.result = append(v.result, err)
 			}
@@ -112,7 +112,7 @@ func (v *validation) contentType() {
 }
 
 func (v *validation) responseFormat() {
-	if str := v.context.ResponseFormat(v.request, v.route.Produces); str == "" {
+	if str := v.context.ResponseFormat(v.request, v.route.Produces); str == "" && httpkit.NeedsContentType(v.request.Method) {
 		v.result = append(v.result, errors.InvalidResponseFormat(v.request.Header.Get(httpkit.HeaderAccept), v.route.Produces))
 	}
 }
diff --git a/request.go b/request.go
@@ -28,6 +28,17 @@ func CanHaveBody(method string) bool {
 	return mn == "POST" || mn == "PUT" || mn == "PATCH" || mn == "DELETE"
 }
 
+// NeedsContentType returns true if this method needs a content-type
+func NeedsContentType(method string) bool {
+	mn := strings.ToUpper(method)
+	return mn == "POST" || mn == "PUT" || mn == "PATCH"
+}
+
+// IsDelete returns true if this method is DELETE
+func IsDelete(method string) bool {
+	return strings.ToUpper(method) == "DELETE"
+}
+
 // JSONRequest creates a new http request with json headers set
 func JSONRequest(method, urlStr string, body io.Reader) (*http.Request, error) {
 	req, err := http.NewRequest(method, urlStr, body)