From 0c58592b1e99a284dedd700c025bd2906695fdb1 Mon Sep 17 00:00:00 2001 From: solomon Date: Mon, 14 Nov 2022 17:40:39 +0000 Subject: [PATCH 1/5] fix: parameter array crush --- openapi3/issue625_test.go | 242 +++++++++++++++++++++++++++++ openapi3filter/req_resp_decoder.go | 99 +++++++++++- 2 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 openapi3/issue625_test.go diff --git a/openapi3/issue625_test.go b/openapi3/issue625_test.go new file mode 100644 index 000000000..673cd3f67 --- /dev/null +++ b/openapi3/issue625_test.go @@ -0,0 +1,242 @@ +package openapi3_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: {} ### + 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"}`, + 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 anyof object array", + spec: oneOfArraySpec, + req: `/items?test=true,3`, + isErr: false, + }, + { + name: "faled anyof 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) + }, + ) + } +} diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index ca8194432..8ae8fdfb2 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -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 + } + + 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 { From 70125a68dd7528b9409063f5f57c31686a9a12de Mon Sep 17 00:00:00 2001 From: solomon Date: Tue, 15 Nov 2022 11:08:04 +0000 Subject: [PATCH 2/5] fix: parameter array crush --- {openapi3 => openapi3filter}/issue625_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename {openapi3 => openapi3filter}/issue625_test.go (99%) diff --git a/openapi3/issue625_test.go b/openapi3filter/issue625_test.go similarity index 99% rename from openapi3/issue625_test.go rename to openapi3filter/issue625_test.go index 673cd3f67..f3fdcdbee 100644 --- a/openapi3/issue625_test.go +++ b/openapi3filter/issue625_test.go @@ -1,4 +1,4 @@ -package openapi3_test +package openapi3filter_test import ( "net/http" From 257e85c2faa52761fa95482f5d4ade3d04a853f4 Mon Sep 17 00:00:00 2001 From: solomon Date: Tue, 15 Nov 2022 14:16:57 +0000 Subject: [PATCH 3/5] fix: parameter array crush --- openapi3filter/issue625_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openapi3filter/issue625_test.go b/openapi3filter/issue625_test.go index f3fdcdbee..377a14df1 100644 --- a/openapi3filter/issue625_test.go +++ b/openapi3filter/issue625_test.go @@ -197,13 +197,13 @@ paths: isErr: true, }, { - name: "success anyof object array", + name: "success oneof object array", spec: oneOfArraySpec, req: `/items?test=true,3`, isErr: false, }, { - name: "faled anyof object array", + name: "faled oneof object array", spec: oneOfArraySpec, req: `/items?test="val1","val2"`, isErr: true, From d960fb5feba734f65ae1c5559468acef2d976d0e Mon Sep 17 00:00:00 2001 From: solomon Date: Thu, 17 Nov 2022 08:18:53 +0000 Subject: [PATCH 4/5] fix: query array crush --- openapi3filter/issue625_test.go | 187 ++++++----------------------- openapi3filter/req_resp_decoder.go | 13 +- 2 files changed, 35 insertions(+), 165 deletions(-) diff --git a/openapi3filter/issue625_test.go b/openapi3filter/issue625_test.go index 377a14df1..f00940b26 100644 --- a/openapi3filter/issue625_test.go +++ b/openapi3filter/issue625_test.go @@ -2,6 +2,7 @@ package openapi3filter_test import ( "net/http" + "strings" "testing" "github.com/stretchr/testify/require" @@ -13,56 +14,6 @@ import ( 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: {} ### - 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: @@ -89,124 +40,50 @@ paths: 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 := strings.ReplaceAll(anyOfArraySpec, "anyOf", "oneOf") - 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:] + allOfArraySpec := strings.ReplaceAll(strings.ReplaceAll(anyOfArraySpec, "anyOf", "allOf"), + "type: boolean", "type: number") tests := []struct { - name string - spec string - req string - isErr bool + name string + spec string + req string + errStr string }{ { - 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"}`, - isErr: false, - }, - { - name: "successful object array", - spec: objectArraySpec, - req: `/items?test={"name": "test1"}`, - isErr: false, + name: "success anyof object array", + spec: anyOfArraySpec, + req: "/items?test=3,7", }, { - 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: "failed anyof object array", + spec: anyOfArraySpec, + req: "/items?test=s1,s2", + errStr: `parameter "test" in query has an error: path 0: value s1: an invalid boolean: invalid syntax`, }, { - name: "success allof object array", - spec: allOfArraySpec, - req: `/items?test=1,3`, - isErr: false, + name: "success allof object array", + spec: allOfArraySpec, + req: `/items?test=1,3`, }, { - name: "failed allof object array", - spec: allOfArraySpec, - req: `/items?test=1.2,3.1`, - isErr: true, + name: "failed allof object array", + spec: allOfArraySpec, + req: `/items?test=1.2,3.1`, + errStr: `parameter "test" in query has an error: Value must be an integer`, }, { - name: "success oneof object array", - spec: oneOfArraySpec, - req: `/items?test=true,3`, - isErr: false, + name: "success oneof object array", + spec: oneOfArraySpec, + req: `/items?test=true,3`, }, { - name: "faled oneof object array", - spec: oneOfArraySpec, - req: `/items?test="val1","val2"`, - isErr: true, + name: "faled oneof object array", + spec: oneOfArraySpec, + req: `/items?test="val1","val2"`, + errStr: `parameter "test" in query has an error: item 0: decoding oneOf failed: 0 schemas matched`, }, } @@ -235,7 +112,11 @@ paths: Route: route, } err = openapi3filter.ValidateRequest(ctx, requestValidationInput) - require.Equal(t, testcase.isErr, err != nil) + if testcase.errStr == "" { + require.NoError(t, err) + } else { + require.Contains(t, err.Error(), testcase.errStr) + } }, ) } diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 8ae8fdfb2..5906bc346 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -612,18 +612,7 @@ func (d *urlValuesDecoder) parseValue(v string, schema *openapi3.SchemaRef) (int 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 + return parsePrimitive(v, schema) } From 76bae911878d61f1a57bc4013ff30ff4c030ac1b Mon Sep 17 00:00:00 2001 From: solomon Date: Thu, 17 Nov 2022 08:32:23 +0000 Subject: [PATCH 5/5] fix: query array crush --- openapi3filter/req_resp_decoder.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index 5906bc346..300b2705f 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -556,15 +556,11 @@ func (d *urlValuesDecoder) parseArray(raw []string, sm *openapi3.SerializationMe } 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 }