From f04b5e8cef2dbea52f34125ebdd569a83eb201eb Mon Sep 17 00:00:00 2001 From: Aleksey Mikhaylov <mikhailov.av@exmo.com> Date: Tue, 24 Dec 2024 00:52:24 +0300 Subject: [PATCH 1/2] improper validation of x-www-form-urlencoded with arbitrary nested allOf (#1045) --- openapi3filter/issue1045_test.go | 128 +++++++++++++++++++++++++++++ openapi3filter/req_resp_decoder.go | 24 +++--- 2 files changed, 140 insertions(+), 12 deletions(-) create mode 100644 openapi3filter/issue1045_test.go diff --git a/openapi3filter/issue1045_test.go b/openapi3filter/issue1045_test.go new file mode 100644 index 00000000..f1885560 --- /dev/null +++ b/openapi3filter/issue1045_test.go @@ -0,0 +1,128 @@ +package openapi3filter + +import ( + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/getkin/kin-openapi/routers/gorillamux" +) + +func TestIssue1045(t *testing.T) { + spec := ` +openapi: 3.0.3 +info: + version: 1.0.0 + title: sample api + description: api service paths to test the issue +paths: + /api/path: + post: + summary: path + tags: + - api + requestBody: + required: true + content: + application/json: + schema: { $ref: '#/components/schemas/PathRequest' } + application/x-www-form-urlencoded: + schema: { $ref: '#/components/schemas/PathRequest' } + responses: + '200': + description: Ok + content: + application/json: + schema: { $ref: '#/components/schemas/PathResponse' } +components: + schemas: + Msg_Opt: + properties: + msg: { type: string } + Msg: + allOf: + - $ref: '#/components/schemas/Msg_Opt' + - required: [ msg ] + Name: + properties: + name: { type: string } + required: [ name ] + Id: + properties: + id: + type: string + format: uint64 + required: [ id ] + PathRequest: + type: object + allOf: + - $ref: '#/components/schemas/Msg' + - $ref: '#/components/schemas/Name' + PathResponse: + type: object + allOf: + - $ref: '#/components/schemas/PathRequest' + - $ref: '#/components/schemas/Id' + `[1:] + + loader := openapi3.NewLoader() + + doc, err := loader.LoadFromData([]byte(spec)) + require.NoError(t, err) + + err = doc.Validate(loader.Context) + require.NoError(t, err) + + router, err := gorillamux.NewRouter(doc) + require.NoError(t, err) + + for _, testcase := range []struct { + name string + endpoint string + ct string + data string + shouldFail bool + }{ + { + name: "json", + endpoint: "/api/path", + ct: "application/json", + data: `{"msg":"message", "name":"some+name"}`, + shouldFail: false, + }, + + // application/x-www-form-urlencoded + { + name: "form", + endpoint: "/api/path", + ct: "application/x-www-form-urlencoded", + data: "msg=message&name=some+name", + shouldFail: false, + }, + } { + t.Run(testcase.ct, func(t *testing.T) { + data := strings.NewReader(testcase.data) + req, err := http.NewRequest("POST", testcase.endpoint, data) + require.NoError(t, err) + req.Header.Add("Content-Type", testcase.ct) + + route, pathParams, err := router.FindRoute(req) + require.NoError(t, err) + + validationInput := &RequestValidationInput{ + Request: req, + PathParams: pathParams, + Route: route, + } + err = ValidateRequest(loader.Context, validationInput) + if testcase.shouldFail { + require.Error(t, err, "This test case should fail "+testcase.data) + } else { + require.NoError(t, err, "This test case should pass "+testcase.data) + } + }) + } +} diff --git a/openapi3filter/req_resp_decoder.go b/openapi3filter/req_resp_decoder.go index c77289ff..998a91f8 100644 --- a/openapi3filter/req_resp_decoder.go +++ b/openapi3filter/req_resp_decoder.go @@ -1340,18 +1340,6 @@ func urlencodedBodyDecoder(body io.Reader, header http.Header, schema *openapi3. obj := make(map[string]any) dec := &urlValuesDecoder{values: values} - // Decode schema constructs (allOf, anyOf, oneOf) - if err := decodeSchemaConstructs(dec, schema.Value.AllOf, obj, encFn); err != nil { - return nil, err - } - if err := decodeSchemaConstructs(dec, schema.Value.AnyOf, obj, encFn); err != nil { - return nil, err - } - if err := decodeSchemaConstructs(dec, schema.Value.OneOf, obj, encFn); err != nil { - return nil, err - } - - // Decode properties from the main schema if err := decodeSchemaConstructs(dec, []*openapi3.SchemaRef{schema}, obj, encFn); err != nil { return nil, err } @@ -1363,6 +1351,18 @@ func urlencodedBodyDecoder(body io.Reader, header http.Header, schema *openapi3. // This function is for decoding purposes only and not for validation. func decodeSchemaConstructs(dec *urlValuesDecoder, schemas []*openapi3.SchemaRef, obj map[string]any, encFn EncodingFn) error { for _, schemaRef := range schemas { + + // Decode schema constructs (allOf, anyOf, oneOf) + if err := decodeSchemaConstructs(dec, schemaRef.Value.AllOf, obj, encFn); err != nil { + return err + } + if err := decodeSchemaConstructs(dec, schemaRef.Value.AnyOf, obj, encFn); err != nil { + return err + } + if err := decodeSchemaConstructs(dec, schemaRef.Value.OneOf, obj, encFn); err != nil { + return err + } + for name, prop := range schemaRef.Value.Properties { value, _, err := decodeProperty(dec, name, prop, encFn) if err != nil { From ce98f41a8e9052d9c2ade6bd92cd166763b855f1 Mon Sep 17 00:00:00 2001 From: Aleksey Mikhaylov <mikhailov.av@exmo.com> Date: Tue, 24 Dec 2024 13:08:03 +0300 Subject: [PATCH 2/2] extend test cases --- openapi3filter/issue1045_test.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/openapi3filter/issue1045_test.go b/openapi3filter/issue1045_test.go index f1885560..048dbd37 100644 --- a/openapi3filter/issue1045_test.go +++ b/openapi3filter/issue1045_test.go @@ -87,23 +87,37 @@ components: shouldFail bool }{ { - name: "json", + name: "json success", endpoint: "/api/path", ct: "application/json", data: `{"msg":"message", "name":"some+name"}`, shouldFail: false, }, + { + name: "json failure", + endpoint: "/api/path", + ct: "application/json", + data: `{"name":"some+name"}`, + shouldFail: true, + }, // application/x-www-form-urlencoded { - name: "form", + name: "form success", endpoint: "/api/path", ct: "application/x-www-form-urlencoded", data: "msg=message&name=some+name", shouldFail: false, }, + { + name: "form failure", + endpoint: "/api/path", + ct: "application/x-www-form-urlencoded", + data: "name=some+name", + shouldFail: true, + }, } { - t.Run(testcase.ct, func(t *testing.T) { + t.Run(testcase.name, func(t *testing.T) { data := strings.NewReader(testcase.data) req, err := http.NewRequest("POST", testcase.endpoint, data) require.NoError(t, err)