Repository: go-openapi/runtime
Pull request #176 (2020-03-18 14:27:14)

Bugfix: Handle Accept header not provided
As part of #172, the Accept header validation was being enforced.
This resulted in a backwards-compatibility issues where if Accept
header was not provided, it would result in an error.
When an Accept header is not provided the client is indicating it
will accept the default response format. As such it should not result
in an error.

Applies fix to provide a default value when no default could be
identified from the request body or using the defined produces on the
specific route.

Applies fix to validation to consider the request valid if no format
could be identified using just the request and produces is empty.

resolves: #174
Signed-off-by: kenjones <kenjones@cisco.com>


Patch:
diff --git a/fixtures/bugs/172/swagger.yml b/fixtures/bugs/172/swagger.yml
old mode 100755
new mode 100644
diff --git a/fixtures/bugs/174/swagger.yml b/fixtures/bugs/174/swagger.yml
new file mode 100644
index 0000000..8aed13a
--- /dev/null
+++ b/fixtures/bugs/174/swagger.yml
@@ -0,0 +1,99 @@
+swagger: '2.0'
+info:
+  version: 1.0.0
+  title: 'Test'
+schemes:
+  - http
+
+paths:
+  /pets:
+    get:
+      description: Returns all pets from the system that the user has access to
+      operationId: findPets
+      parameters:
+        - name: tags
+          in: query
+          description: tags to filter by
+          required: false
+          type: array
+          items:
+            type: string
+          collectionFormat: csv
+        - name: limit
+          in: query
+          description: maximum number of results to return
+          required: false
+          type: integer
+          format: int32
+      responses:
+        '200':
+          description: pet response
+          schema:
+            type: array
+            items:
+              $ref: '#/definitions/pet'
+        default:
+          description: unexpected error
+          schema:
+            $ref: '#/definitions/errorModel'
+
+    post:
+      description: Creates a new pet in the store.  Duplicates are allowed
+      operationId: addPet
+      consumes:
+        - application/json
+      produces:
+        - application/json
+      parameters:
+        - name: pet
+          in: body
+          description: Pet to add to the store
+          required: true
+          schema:
+            $ref: '#/definitions/newPet'
+      responses:
+        '200':
+          description: pet response
+          schema:
+            $ref: '#/definitions/pet'
+        default:
+          description: unexpected error
+          schema:
+            $ref: '#/definitions/errorModel'
+
+definitions:
+  pet:
+    required:
+      - id
+      - name
+    properties:
+      id:
+        type: integer
+        format: int64
+      name:
+        type: string
+      tag:
+        type: string
+
+  newPet:
+    allOf:
+      - $ref: '#/definitions/pet'
+      - required:
+          - name
+        properties:
+          id:
+            type: integer
+            format: int64
+          name:
+            type: string
+
+  errorModel:
+    required:
+      - code
+      - message
+    properties:
+      code:
+        type: integer
+        format: int32
+      message:
+        type: string
diff --git a/middleware/context.go b/middleware/context.go
index 0ff9e39..ca3e328 100644
--- a/middleware/context.go
+++ b/middleware/context.go
@@ -305,6 +305,12 @@ func (c *Context) BindValidRequest(request *http.Request, route *MatchedRoute, b
 
 	// check and validate the response format
 	if len(res) == 0 {
+		// if the route does not provide Produces and a default contentType could not be identified
+		// based on a body, typical for GET and DELETE requests, then default contentType to.
+		if len(route.Produces) == 0 && requestContentType == "" {
+			requestContentType = "*/*"
+		}
+
 		if str := NegotiateContentType(request, route.Produces, requestContentType); str == "" {
 			res = append(res, errors.InvalidResponseFormat(request.Header.Get(runtime.HeaderAccept), route.Produces))
 		}
diff --git a/middleware/validation.go b/middleware/validation.go
index 9505c66..1f0135b 100644
--- a/middleware/validation.go
+++ b/middleware/validation.go
@@ -116,7 +116,10 @@ func (v *validation) contentType() {
 }
 
 func (v *validation) responseFormat() {
-	if str, rCtx := v.context.ResponseFormat(v.request, v.route.Produces); str == "" {
+	// if the route provides values for Produces and no format could be identify then return an error.
+	// if the route does not specify values for Produces then treat request as valid since the API designer
+	// choose not to specify the format for responses.
+	if str, rCtx := v.context.ResponseFormat(v.request, v.route.Produces); str == "" && len(v.route.Produces) > 0 {
 		v.request = rCtx
 		v.result = append(v.result, errors.InvalidResponseFormat(v.request.Header.Get(runtime.HeaderAccept), v.route.Produces))
 	}


Test patch:
diff --git a/middleware/context_test.go b/middleware/context_test.go
index 3d5d0e2..b50966e 100644
--- a/middleware/context_test.go
+++ b/middleware/context_test.go
@@ -53,7 +53,7 @@ func init() {
 	loads.AddLoader(fmts.YAMLMatcher, fmts.YAMLDoc)
 }
 
-func assertApiError(t *testing.T, wantCode int, err error) {
+func assertAPIError(t *testing.T, wantCode int, err error) {
 	t.Helper()
 
 	assert.NotNil(t, err)
@@ -100,6 +100,22 @@ func TestContentType_Issue172(t *testing.T) {
 	}
 }
 
+func TestContentType_Issue174(t *testing.T) {
+	swspec, err := loads.Spec("../fixtures/bugs/174/swagger.yml")
+	if assert.NoError(t, err) {
+		api := untyped.NewAPI(swspec)
+		api.RegisterConsumer("application/json", runtime.JSONConsumer())
+		api.RegisterProducer("application/json", runtime.JSONProducer())
+		api.RegisterOperation("get", "/pets", new(stubOperationHandler))
+
+		handler := Serve(swspec, api)
+		request, _ := http.NewRequest("GET", "/pets", nil)
+		recorder := httptest.NewRecorder()
+		handler.ServeHTTP(recorder, request)
+		assert.Equal(t, http.StatusOK, recorder.Code)
+	}
+}
+
 func TestServe(t *testing.T) {
 	spec, api := petstore.NewAPI(t)
 	handler := Serve(spec, api)
@@ -226,14 +242,14 @@ func TestContextBindValidRequest(t *testing.T) {
 	request.Header.Add("content-type", "/json")
 
 	ri, request, _ := ctx.RouteInfo(request)
-	assertApiError(t, 400, ctx.BindValidRequest(request, ri, new(stubBindRequester)))
+	assertAPIError(t, 400, ctx.BindValidRequest(request, ri, new(stubBindRequester)))
 
 	// unsupported content-type value
 	request, _ = http.NewRequest("POST", "/api/pets", strings.NewReader(`{"name":"dog"}`))
 	request.Header.Add("content-type", "text/html")
 
 	ri, request, _ = ctx.RouteInfo(request)
-	assertApiError(t, http.StatusUnsupportedMediaType, ctx.BindValidRequest(request, ri, new(stubBindRequester)))
+	assertAPIError(t, http.StatusUnsupportedMediaType, ctx.BindValidRequest(request, ri, new(stubBindRequester)))
 
 	// unacceptable accept value
 	request, _ = http.NewRequest("POST", "/api/pets", nil)
@@ -241,7 +257,24 @@ func TestContextBindValidRequest(t *testing.T) {
 	request.Header.Add("content-type", "application/json")
 
 	ri, request, _ = ctx.RouteInfo(request)
-	assertApiError(t, http.StatusNotAcceptable, ctx.BindValidRequest(request, ri, new(stubBindRequester)))
+	assertAPIError(t, http.StatusNotAcceptable, ctx.BindValidRequest(request, ri, new(stubBindRequester)))
+}
+
+func TestContextBindValidRequest_Issue174(t *testing.T) {
+	spec, err := loads.Spec("../fixtures/bugs/174/swagger.yml")
+	assert.NoError(t, err)
+
+	api := untyped.NewAPI(spec)
+	api.RegisterConsumer("application/json", runtime.JSONConsumer())
+	api.RegisterProducer("application/json", runtime.JSONProducer())
+	api.RegisterOperation("get", "/pets", new(stubOperationHandler))
+
+	ctx := NewContext(spec, api, nil)
+	ctx.router = DefaultRouter(spec, ctx.api)
+
+	request, _ := http.NewRequest("GET", "/pets", nil)
+	ri, request, _ := ctx.RouteInfo(request)
+	assert.NoError(t, ctx.BindValidRequest(request, ri, new(stubBindRequester)))
 }
 
 func TestContextBindAndValidate(t *testing.T) {