Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openapi3filter: fix crash when given arrays of objects as query parameters #664

Merged
merged 5 commits into from Nov 24, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
242 changes: 242 additions & 0 deletions openapi3filter/issue625_test.go
@@ -0,0 +1,242 @@
package openapi3filter_test

import (
"net/http"
"testing"

"github.com/stretchr/testify/require"

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

func TestIssue625(t *testing.T) {

anyItemSpec := `
openapi: 3.0.0
info:
version: 1.0.0
title: Sample API
paths:
/items:
get:
description: Returns a list of stuff
parameters:
- description: test object
explode: false
in: query
name: test
required: false
schema:
type: array
items: {} ###
fenollp marked this conversation as resolved.
Show resolved Hide resolved
responses:
'200':
description: Successful response
`[1:]

objectArraySpec := `
openapi: 3.0.0
info:
version: 1.0.0
title: Sample API
paths:
/items:
get:
description: Returns a list of stuff
parameters:
- description: test object
explode: false
in: query
name: test
required: false
schema:
type: array
items:
type: object
properties:
name:
type: string
responses:
'200':
description: Successful response
`[1:]

anyOfArraySpec := `
openapi: 3.0.0
info:
version: 1.0.0
title: Sample API
paths:
/items:
get:
description: Returns a list of stuff
parameters:
- description: test object
explode: false
in: query
name: test
required: false
schema:
type: array
items:
anyOf:
- type: integer
- type: boolean
responses:
'200':
description: Successful response
`[1:]

allOfArraySpec := `
openapi: 3.0.0
info:
version: 1.0.0
title: Sample API
paths:
/items:
get:
description: Returns a list of stuff
parameters:
- description: test object
explode: false
in: query
name: test
required: false
schema:
type: array
items:
allOf:
- type: integer
- type: number
responses:
'200':
description: Successful response
`[1:]

oneOfArraySpec := `
openapi: 3.0.0
info:
version: 1.0.0
title: Sample API
paths:
/items:
get:
description: Returns a list of stuff
parameters:
- description: test object
explode: false
in: query
name: test
required: false
schema:
type: array
items:
oneOf:
- type: integer
- type: boolean
responses:
'200':
description: Successful response
`[1:]

tests := []struct {
name string
spec string
req string
isErr bool
}{
{
name: "successful any item array",
spec: anyItemSpec,
req: "/items?test=3",
isErr: false,
},
{
name: "successful any item object array",
spec: anyItemSpec,
req: `/items?test={"name": "test1"}`,
fenollp marked this conversation as resolved.
Show resolved Hide resolved
isErr: false,
},
{
name: "successful object array",
spec: objectArraySpec,
req: `/items?test={"name": "test1"}`,
isErr: false,
},
{
name: "failed object array",
spec: objectArraySpec,
req: "/items?test=3",
isErr: true,
},
{
name: "success anyof object array",
spec: anyOfArraySpec,
req: "/items?test=3,7",
isErr: false,
},
{
name: "failed anyof object array",
spec: anyOfArraySpec,
req: "/items?test=s1,s2",
isErr: true,
},

{
name: "success allof object array",
spec: allOfArraySpec,
req: `/items?test=1,3`,
isErr: false,
},
{
name: "failed allof object array",
spec: allOfArraySpec,
req: `/items?test=1.2,3.1`,
isErr: true,
},
{
name: "success oneof object array",
spec: oneOfArraySpec,
req: `/items?test=true,3`,
isErr: false,
},
{
name: "faled oneof object array",
spec: oneOfArraySpec,
req: `/items?test="val1","val2"`,
isErr: true,
},
}

for _, testcase := range tests {
t.Run(testcase.name, func(t *testing.T) {
loader := openapi3.NewLoader()
ctx := loader.Context

doc, err := loader.LoadFromData([]byte(testcase.spec))
require.NoError(t, err)

err = doc.Validate(ctx)
require.NoError(t, err)

router, err := gorillamux.NewRouter(doc)
require.NoError(t, err)
httpReq, err := http.NewRequest(http.MethodGet, testcase.req, nil)
require.NoError(t, err)

route, pathParams, err := router.FindRoute(httpReq)
require.NoError(t, err)

requestValidationInput := &openapi3filter.RequestValidationInput{
Request: httpReq,
PathParams: pathParams,
Route: route,
}
err = openapi3filter.ValidateRequest(ctx, requestValidationInput)
require.Equal(t, testcase.isErr, err != nil)
fenollp marked this conversation as resolved.
Show resolved Hide resolved
},
)
}
}
99 changes: 98 additions & 1 deletion openapi3filter/req_resp_decoder.go
Expand Up @@ -526,10 +526,107 @@ func (d *urlValuesDecoder) DecodeArray(param string, sm *openapi3.SerializationM
}
values = strings.Split(values[0], delim)
}
val, err := parseArray(values, schema)
val, err := d.parseArray(values, sm, schema)
return val, ok, err
}

// parseArray returns an array that contains items from a raw array.
// Every item is parsed as a primitive value.
// The function returns an error when an error happened while parse array's items.
func (d *urlValuesDecoder) parseArray(raw []string, sm *openapi3.SerializationMethod, schemaRef *openapi3.SchemaRef) ([]interface{}, error) {
var value []interface{}

for i, v := range raw {
item, err := d.parseValue(v, schemaRef.Value.Items)
if err != nil {
if v, ok := err.(*ParseError); ok {
return nil, &ParseError{path: []interface{}{i}, Cause: v}
}
return nil, fmt.Errorf("item %d: %w", i, err)
}

// If the items are nil, then the array is nil. There shouldn't be case where some values are actual primitive
// values and some are nil values.
if item == nil {
return nil, nil
}
value = append(value, item)
}
return value, nil
}

func (d *urlValuesDecoder) parseValue(v string, schema *openapi3.SchemaRef) (interface{}, error) {
var found bool

if len(schema.Value.AllOf) > 0 {
var value interface{}
var err error
for _, sr := range schema.Value.AllOf {
var f bool
value, err = d.parseValue(v, sr)
found = found || f
if value == nil || err != nil {
break
}
}
return value, err
fenollp marked this conversation as resolved.
Show resolved Hide resolved
}

if len(schema.Value.AnyOf) > 0 {
var value interface{}
var err error
for _, sr := range schema.Value.AnyOf {
value, err = d.parseValue(v, sr)
if err == nil {
return value, nil
}
}

return nil, err
}

if len(schema.Value.OneOf) > 0 {
isMatched := 0
var value interface{}
var err error
for _, sr := range schema.Value.OneOf {
result, err := d.parseValue(v, sr)
if err == nil {
value = result
isMatched++
}
}
if isMatched == 1 {
return value, nil
} else if isMatched > 1 {
return nil, fmt.Errorf("decoding oneOf failed: %d schemas matched", isMatched)
} else if isMatched == 0 {
return nil, fmt.Errorf("decoding oneOf failed: %d schemas matched", isMatched)
}

return nil, err
}

if schema.Value.Not != nil {
// TODO(decode not): handle decoding "not" JSON Schema
return nil, errors.New("not implemented: decoding 'not'")
}

schemaType := schema.Value.Type
isPrimitive := schemaType == "integer" || schemaType == "number" ||
schemaType == "boolean" || schemaType == "string"
if isPrimitive {
return parsePrimitive(v, schema)
}

var value interface{}
if err := json.NewDecoder(strings.NewReader(v)).Decode(&value); err != nil {
return nil, err
}
return value, nil

}

func (d *urlValuesDecoder) DecodeObject(param string, sm *openapi3.SerializationMethod, schema *openapi3.SchemaRef) (map[string]interface{}, bool, error) {
var propsFn func(url.Values) (map[string]string, error)
switch sm.Style {
Expand Down