Repository: getkin/kin-openapi
Pull request #90 (2019-04-19 21:05:09)

Add content parameter validation
Fix crash in validation of parameters which are specified with
content rather than schema. The problem was that the presence
of a parameter with a nil schema in the Swagger spec would
crash the loop which validated over all parameters.

Added a callback to the request validation input so the user
can decode custom content types, and also implemented an
alternate code path for validation of content arguments. Added
an internal default parameter validator which is capable of
validating JSON encoded parameters.

mromaszewicz: Hi guys. Any feedback on this PR? I need it to fix some crashing API's and I'd like to avoid forking the repo to do it.


Patch:
diff --git a/openapi3/schema_formats.go b/openapi3/schema_formats.go
index aa7df87..9b2f316 100644
--- a/openapi3/schema_formats.go
+++ b/openapi3/schema_formats.go
@@ -19,7 +19,7 @@ func DefineStringFormat(name string, pattern string) {
 func init() {
 	// This pattern catches only some suspiciously wrong-looking email addresses.
 	// Use DefineStringFormat(...) if you need something stricter.
-	DefineStringFormat("email", `^[^@]+@[^@<>",\\w]+$`)
+	DefineStringFormat("email", `^[^@]+@[^@<>",\s]+$`)
 
 	// Base64
 	// The pattern supports base64 and b./ase64url. Padding ('=') is supported.
diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go
index 6bc1f99..8bf826d 100644
--- a/openapi3filter/req_resp_decoder.go
+++ b/openapi3filter/req_resp_decoder.go
@@ -98,15 +98,111 @@ func invalidSerializationMethodErr(sm *openapi3.SerializationMethod) error {
 	return fmt.Errorf("invalid serialization method: style=%q, explode=%v", sm.Style, sm.Explode)
 }
 
+// Decodes a parameter defined via the content property as an object. It uses
+// the user specified decoder, or our build-in decoder for application/json
+func decodeContentParameter(param *openapi3.Parameter, input *RequestValidationInput) (
+	value interface{}, schema *openapi3.Schema, err error) {
+
+	paramValues := make([]string, 1)
+	var found bool
+	switch param.In {
+	case openapi3.ParameterInPath:
+		paramValues[0], found = input.PathParams[param.Name]
+	case openapi3.ParameterInQuery:
+		paramValues, found = input.GetQueryParams()[param.Name]
+	case openapi3.ParameterInHeader:
+		paramValues[0] = input.Request.Header.Get(http.CanonicalHeaderKey(param.Name))
+		found = paramValues[0] != ""
+	case openapi3.ParameterInCookie:
+		var cookie *http.Cookie
+		cookie, err = input.Request.Cookie(param.Name)
+		if err == http.ErrNoCookie {
+			found = false
+		} else if err != nil {
+			return
+		} else {
+			paramValues[0] = cookie.Value
+			found = true
+		}
+	default:
+		err = fmt.Errorf("unsupported parameter's 'in': %s", param.In)
+		return
+	}
+
+	if !found {
+		if param.Required {
+			err = fmt.Errorf("parameter '%s' is required, but missing", param.Name)
+		}
+		return
+	}
+
+	decoder := input.ParamDecoder
+	if decoder == nil {
+		decoder = defaultContentParameterDecoder
+	}
+
+	value, schema, err = decoder(param, paramValues)
+	return
+}
+
+func defaultContentParameterDecoder(param *openapi3.Parameter, values []string) (
+	outValue interface{}, outSchema *openapi3.Schema, err error) {
+	// Only query parameters can have multiple values.
+	if len(values) > 1 && param.In != "query" {
+		err = fmt.Errorf("%s parameter '%s' can't have multiple values", param.In, param.Name)
+		return
+	}
+
+	content := param.Content
+	if content == nil {
+		err = fmt.Errorf("parameter '%s' expected to have content", param.Name)
+		return
+	}
+
+	// We only know how to decode a parameter if it has one content, application/json
+	if len(content) != 1 {
+		err = fmt.Errorf("multiple content types for parameter '%s'",
+			param.Name)
+		return
+	}
+
+	mt := content.Get("application/json")
+	if mt == nil {
+		err = fmt.Errorf("parameter '%s' has no json content schema", param.Name)
+		return
+	}
+	outSchema = mt.Schema.Value
+
+	if len(values) == 1 {
+		err = json.Unmarshal([]byte(values[0]), &outValue)
+		if err != nil {
+			err = fmt.Errorf("error unmarshaling parameter '%s' as json", param.Name)
+			return
+		}
+	} else {
+		outArray := make([]interface{}, len(values))
+		for i, v := range values {
+			err = json.Unmarshal([]byte(v), &outArray[i])
+			if err != nil {
+				err = fmt.Errorf("error unmarshaling parameter '%s' as json", param.Name)
+				return
+			}
+		}
+		outValue = outArray
+	}
+	return
+}
+
 type valueDecoder interface {
 	DecodePrimitive(param string, sm *openapi3.SerializationMethod, schema *openapi3.SchemaRef) (interface{}, error)
 	DecodeArray(param string, sm *openapi3.SerializationMethod, schema *openapi3.SchemaRef) ([]interface{}, error)
 	DecodeObject(param string, sm *openapi3.SerializationMethod, schema *openapi3.SchemaRef) (map[string]interface{}, error)
 }
 
-// decodeParameter returns a value of an operation's parameter from HTTP request.
+// decodeStyledParameter returns a value of an operation's parameter from HTTP request for
+// parameters defined using the style format.
 // The function returns ParseError when HTTP request contains an invalid value of a parameter.
-func decodeParameter(param *openapi3.Parameter, input *RequestValidationInput) (interface{}, error) {
+func decodeStyledParameter(param *openapi3.Parameter, input *RequestValidationInput) (interface{}, error) {
 	sm, err := param.SerializationMethod()
 	if err != nil {
 		return nil, err
diff --git a/openapi3filter/validate_request.go b/openapi3filter/validate_request.go
index 8e99c85..0231cf4 100644
--- a/openapi3filter/validate_request.go
+++ b/openapi3filter/validate_request.go
@@ -74,11 +74,30 @@ func ValidateRequest(c context.Context, input *RequestValidationInput) error {
 // The function returns RequestError with ErrInvalidRequired cause when a value of a required parameter is not defined.
 // The function returns RequestError with a openapi3.SchemaError cause when a value is invalid by JSON schema.
 func ValidateParameter(c context.Context, input *RequestValidationInput, parameter *openapi3.Parameter) error {
-	value, err := decodeParameter(parameter, input)
-	if err != nil {
-		return &RequestError{Input: input, Parameter: parameter, Err: err}
+	if parameter.Schema == nil && parameter.Content == nil {
+		// We have no schema for the parameter. Assume that everything passes
+		// a schema-less check, but this could also be an error. The Swagger
+		// validation allows this to happen.
+		return nil
 	}
 
+	var value interface{}
+	var err error
+	var schema *openapi3.Schema
+
+	// Validation will ensure that we either have content or schema.
+	if parameter.Content != nil {
+		value, schema, err = decodeContentParameter(parameter, input)
+		if err != nil {
+			return &RequestError{Input: input, Parameter: parameter, Err: err}
+		}
+	} else {
+		value, err = decodeStyledParameter(parameter, input)
+		if err != nil {
+			return &RequestError{Input: input, Parameter: parameter, Err: err}
+		}
+		schema = parameter.Schema.Value
+	}
 	// Validate a parameter's value.
 	if value == nil {
 		if parameter.Required {
@@ -86,7 +105,6 @@ func ValidateParameter(c context.Context, input *RequestValidationInput, paramet
 		}
 		return nil
 	}
-	schema := parameter.Schema.Value
 	if schema == nil {
 		// A parameter's schema is not defined so skip validation of a parameter's value.
 		return nil
diff --git a/openapi3filter/validate_request_input.go b/openapi3filter/validate_request_input.go
index eed8bd7..150e270 100644
--- a/openapi3filter/validate_request_input.go
+++ b/openapi3filter/validate_request_input.go
@@ -3,14 +3,28 @@ package openapi3filter
 import (
 	"net/http"
 	"net/url"
+
+	"github.com/getkin/kin-openapi/openapi3"
 )
 
+// This function takes a parameter definition from the swagger spec, and
+// the value which we received for it. It is expected to return the
+// value unmarshaled into an interface which can be traversed for
+// validation, it should also return the schema to be used for validating the
+// object, since there can be more than one in the content spec.
+//
+// If a query parameter appears multiple times, values[] will have more
+// than one  value, but for all other parameter types it should have just
+// one.
+type ContentParameterDecoder func(param *openapi3.Parameter, values []string) (interface{}, *openapi3.Schema, error)
+
 type RequestValidationInput struct {
-	Request     *http.Request
-	PathParams  map[string]string
-	QueryParams url.Values
-	Route       *Route
-	Options     *Options
+	Request      *http.Request
+	PathParams   map[string]string
+	QueryParams  url.Values
+	Route        *Route
+	Options      *Options
+	ParamDecoder ContentParameterDecoder
 }
 
 func (input *RequestValidationInput) GetQueryParams() url.Values {


Test patch:
diff --git a/openapi3filter/req_resp_decoder_test.go b/openapi3filter/req_resp_decoder_test.go
index 02627fb..bd761b4 100644
--- a/openapi3filter/req_resp_decoder_test.go
+++ b/openapi3filter/req_resp_decoder_test.go
@@ -854,7 +854,7 @@ func TestDecodeParameter(t *testing.T) {
 					require.NoError(t, err, "failed to find a route")
 
 					input := &RequestValidationInput{Request: req, PathParams: pathParams, Route: route}
-					got, err := decodeParameter(tc.param, input)
+					got, err := decodeStyledParameter(tc.param, input)
 
 					if tc.err != nil {
 						require.Error(t, err)
diff --git a/openapi3filter/validation_test.go b/openapi3filter/validation_test.go
index 7ffd33c..c20611a 100644
--- a/openapi3filter/validation_test.go
+++ b/openapi3filter/validation_test.go
@@ -31,6 +31,12 @@ type ExampleResponse struct {
 }
 
 func TestFilter(t *testing.T) {
+	// Declare a schema for an object with name and id properties
+	complexArgSchema := openapi3.NewObjectSchema().
+		WithProperty("name", openapi3.NewStringSchema()).
+		WithProperty("id", openapi3.NewStringSchema().WithMaxLength(2))
+	complexArgSchema.Required = []string{"name", "id"}
+
 	// Declare router
 	swagger := &openapi3.Swagger{
 		Servers: openapi3.Servers{
@@ -56,6 +62,22 @@ func TestFilter(t *testing.T) {
 								Schema: openapi3.NewStringSchema().WithMaxLength(2).NewRef(),
 							},
 						},
+						{
+							Value: &openapi3.Parameter{
+								In:   "query",
+								Name: "contentArg",
+								Content: openapi3.NewContentWithJSONSchema(complexArgSchema),
+							},
+						},
+						{
+							Value: &openapi3.Parameter{
+								In:   "query",
+								Name: "contentArg2",
+								Content: openapi3.Content{
+									"application/something_funny": openapi3.NewMediaType().WithSchema(complexArgSchema),
+								},
+							},
+						},
 					},
 				},
 			},
@@ -63,7 +85,7 @@ func TestFilter(t *testing.T) {
 	}
 
 	router := openapi3filter.NewRouter().WithSwagger(swagger)
-	expect := func(req ExampleRequest, resp ExampleResponse) error {
+	expectWithDecoder := func(req ExampleRequest, resp ExampleResponse, decoder openapi3filter.ContentParameterDecoder) error {
 		t.Logf("Request: %s %s", req.Method, req.URL)
 		httpReq, _ := http.NewRequest(req.Method, req.URL, marshalReader(req.Body))
 		httpReq.Header.Set("Content-Type", req.ContentType)
@@ -77,6 +99,7 @@ func TestFilter(t *testing.T) {
 			Request:    httpReq,
 			PathParams: pathParams,
 			Route:      route,
+			ParamDecoder: decoder,
 		}
 		if err := openapi3filter.ValidateRequest(context.TODO(), requestValidationInput); err != nil {
 			return err
@@ -100,6 +123,10 @@ func TestFilter(t *testing.T) {
 		require.NoError(t, err)
 		return err
 	}
+	expect := func(req ExampleRequest, resp ExampleResponse) error {
+		return expectWithDecoder(req, resp, nil)
+	}
+
 	var err error
 	var req ExampleRequest
 	var resp ExampleResponse
@@ -163,6 +190,46 @@ func TestFilter(t *testing.T) {
 	err = expect(req, resp)
 	// require.IsType(t, &openapi3filter.ResponseError{}, err)
 	require.NoError(t, err)
+
+	// Check that content validation works. This should pass, as ID is short
+	// enough.
+	req = ExampleRequest{
+		Method: "POST",
+		URL:    "http://example.com/api/prefix/v/suffix?contentArg={\"name\":\"bob\", \"id\":\"a\"}",
+	}
+	err = expect(req, resp)
+	require.NoError(t, err)
+
+	// Now it should fail due the ID being too long
+	req = ExampleRequest{
+		Method: "POST",
+		URL:    "http://example.com/api/prefix/v/suffix?contentArg={\"name\":\"bob\", \"id\":\"EXCEEDS_MAX_LENGTH\"}",
+	}
+	err = expect(req, resp)
+	require.IsType(t, &openapi3filter.RequestError{}, err)
+
+	// Now, repeat the above two test cases using a custom parameter decoder.
+	customDecoder := func(param *openapi3.Parameter, values []string) (interface{}, *openapi3.Schema, error) {
+		var value interface{}
+		err := json.Unmarshal([]byte(values[0]), &value)
+		schema := param.Content.Get("application/something_funny").Schema.Value
+		return value, schema, err
+	}
+
+	req = ExampleRequest{
+		Method: "POST",
+		URL:    "http://example.com/api/prefix/v/suffix?contentArg2={\"name\":\"bob\", \"id\":\"a\"}",
+	}
+	err = expectWithDecoder(req, resp, customDecoder)
+	require.NoError(t, err)
+
+	// Now it should fail due the ID being too long
+	req = ExampleRequest{
+		Method: "POST",
+		URL:    "http://example.com/api/prefix/v/suffix?contentArg2={\"name\":\"bob\", \"id\":\"EXCEEDS_MAX_LENGTH\"}",
+	}
+	err = expectWithDecoder(req, resp, customDecoder)
+	require.IsType(t, &openapi3filter.RequestError{}, err)
 }
 
 func marshalReader(value interface{}) io.ReadCloser {