Skip to content

Bug in the fix to 624 issue #632

@orensolo

Description

@orensolo

Hi,

There is a bug in the fix to #624 issue.
Using a special character in the parameter causes unmarshalling error.

Running the following:

package main

import (
	"context"
	"fmt"
	"net/http"

	"github.com/getkin/kin-openapi/openapi3"
	"github.com/getkin/kin-openapi/openapi3filter"
	"github.com/getkin/kin-openapi/routers/gorillamux"
)

func main() {
	ctx := context.Background()
	loader := &openapi3.Loader{Context: ctx, IsExternalRefsAllowed: true}
	schema :=
		`
openapi: 3.0.0
info:
 version: 1.0.0
 title: Sample API
paths:
 /items:
  get:
   parameters:
    - description: "test non object"
      explode: true
      style: form
      in: query
      name: test
      required: false
      content:
       application/json:
         schema:
          anyOf:
           - type: string
           - type: integer
   description: Returns a list of stuff              
   responses:
    '200':
     description: Successful response`

	doc, err := loader.LoadFromData([]byte(schema))

	if err != nil {
		fmt.Println("failed to load schema. " + err.Error())
		return
	}
	// Validate document

	if err = doc.Validate(ctx); err != nil {
		fmt.Println("failed to validate schema. " + err.Error())
		return
	}

	router, _ := gorillamux.NewRouter(doc)
	httpReq, _ := http.NewRequest(http.MethodGet, `/items?test=test[1`, nil)  // <== note the input parameter contains [

	// Find route
	route, pathParams, _ := router.FindRoute(httpReq)

	// Validate request
	requestValidationInput := &openapi3filter.RequestValidationInput{
		Request:    httpReq,
		PathParams: pathParams,
		Route:      route,
	}
	err = openapi3filter.ValidateRequest(ctx, requestValidationInput)
	if err != nil {
		fmt.Println("failed to validate request. " + err.Error())
		return
	}

}

Returns:

failed to validate request. parameter "test" in query has an error: error unmarshaling parameter "test"

The fix assumes that there cannot be special characters in the input parameters - which is a wrong assumption.
The correct fix is to use the decodeStyledParameter instead of decodeContentParameter. (with some relevant changes)

Please change the fix to the following fix:

In validate_request.go:

func ValidateParameter(ctx context.Context, input *RequestValidationInput, parameter *openapi3.Parameter) error {
	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 OpenAPI
		// validation allows this to happen.
		return nil
	}

	options := input.Options
	if options == nil {
		options = DefaultOptions
	}

	var value interface{}
	var err error
	var found bool
	var schemaRef *openapi3.SchemaRef

	// Validation will ensure that we either have content or schema.
	if parameter.Content != nil {
		// We only know how to decode a parameter if it has one content, application/json
		if len(parameter.Content) != 1 {
			err = fmt.Errorf("multiple content types for parameter %q", parameter.Name)
			return &RequestError{Input: input, Parameter: parameter, Err: err}
		}

		mt := parameter.Content.Get("application/json")
		if mt == nil {
			err = fmt.Errorf("parameter %q has no content schema", parameter.Name)
			return &RequestError{Input: input, Parameter: parameter, Err: err}
		}
		schemaRef = mt.Schema

	} else {
		schemaRef = parameter.Schema
	}

	if value, found, err = decodeStyledParameter(parameter, input, schemaRef); err != nil {
		return &RequestError{Input: input, Parameter: parameter, Err: err}
	}

	schema := schemaRef.Value
	// Set default value if needed
	if value == nil && schema != nil && schema.Default != nil {
		value = schema.Default
		req := input.Request
		switch parameter.In {
		case openapi3.ParameterInPath:
			// Path parameters are required.
			// Next check `parameter.Required && !found` will catch this.
		case openapi3.ParameterInQuery:
			q := req.URL.Query()
			q.Add(parameter.Name, fmt.Sprintf("%v", value))
			req.URL.RawQuery = q.Encode()
		case openapi3.ParameterInHeader:
			req.Header.Add(parameter.Name, fmt.Sprintf("%v", value))
		case openapi3.ParameterInCookie:
			req.AddCookie(&http.Cookie{
				Name:  parameter.Name,
				Value: fmt.Sprintf("%v", value),
			})
		}
	}

	// Validate a parameter's value and presence.
	if parameter.Required && !found {
		return &RequestError{Input: input, Parameter: parameter, Reason: ErrInvalidRequired.Error(), Err: ErrInvalidRequired}
	}

	if isNilValue(value) {
		if !parameter.AllowEmptyValue && found {
			return &RequestError{Input: input, Parameter: parameter, Reason: ErrInvalidEmptyValue.Error(), Err: ErrInvalidEmptyValue}
		}
		return nil
	}
	if schema == nil {
		// A parameter's schema is not defined so skip validation of a parameter's value.
		return nil
	}

	var opts []openapi3.SchemaValidationOption
	if options.MultiError {
		opts = make([]openapi3.SchemaValidationOption, 0, 1)
		opts = append(opts, openapi3.MultiErrors())
	}
	if err = schema.VisitJSON(value, opts...); err != nil {
		return &RequestError{Input: input, Parameter: parameter, Err: err}
	}
	return nil
}

And in req_resp_decoder.go, change decodeStyledParameter to the following:

func decodeStyledParameter(param *openapi3.Parameter, input *RequestValidationInput, schema *openapi3.SchemaRef) (interface{}, bool, error) {
	sm, err := param.SerializationMethod()
	if err != nil {
		return nil, false, err
	}

	var dec valueDecoder
	switch param.In {
	case openapi3.ParameterInPath:
		if len(input.PathParams) == 0 {
			return nil, false, nil
		}
		dec = &pathParamDecoder{pathParams: input.PathParams}
	case openapi3.ParameterInQuery:
		if len(input.GetQueryParams()) == 0 {
			return nil, false, nil
		}
		dec = &urlValuesDecoder{values: input.GetQueryParams()}
	case openapi3.ParameterInHeader:
		dec = &headerParamDecoder{header: input.Request.Header}
	case openapi3.ParameterInCookie:
		dec = &cookieParamDecoder{req: input.Request}
	default:
		return nil, false, fmt.Errorf("unsupported parameter's 'in': %s", param.In)
	}

	return decodeValue(dec, param.Name, sm, schema, param.Required)
}

Thanks,
Oren

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions