From 25120a3f1ec5a6da12d6dca2c5b2e1eb0bb94091 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 23 Jul 2021 13:58:29 +0200 Subject: [PATCH 1/2] reproduce and fix issue #382 Signed-off-by: Pierre Fenoll --- openapi3/issue382_test.go | 15 ++++++ openapi3/paths.go | 60 +++++++++++++++-------- openapi3/testdata/Test_param_override.yml | 40 +++++++++++++++ 3 files changed, 95 insertions(+), 20 deletions(-) create mode 100644 openapi3/issue382_test.go create mode 100644 openapi3/testdata/Test_param_override.yml diff --git a/openapi3/issue382_test.go b/openapi3/issue382_test.go new file mode 100644 index 000000000..c29b7e981 --- /dev/null +++ b/openapi3/issue382_test.go @@ -0,0 +1,15 @@ +package openapi3 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestOverridingGlobalParametersValidation(t *testing.T) { + loader := NewLoader() + doc, err := loader.LoadFromFile("testdata/Test_param_override.yml") + require.NoError(t, err) + err = doc.Validate(loader.Context) + require.NoError(t, err) +} diff --git a/openapi3/paths.go b/openapi3/paths.go index c6ddbf3bf..c31ef4b6e 100644 --- a/openapi3/paths.go +++ b/openapi3/paths.go @@ -21,31 +21,43 @@ func (value Paths) Validate(ctx context.Context) error { pathItem = value[path] } - normalizedPath, pathParamsCount := normalizeTemplatedPath(path) + normalizedPath, _, varsInPath := normalizeTemplatedPath(path) if oldPath, ok := normalizedPaths[normalizedPath]; ok { return fmt.Errorf("conflicting paths %q and %q", path, oldPath) } normalizedPaths[path] = path - var globalCount uint + var commonParams []string for _, parameterRef := range pathItem.Parameters { if parameterRef != nil { if parameter := parameterRef.Value; parameter != nil && parameter.In == ParameterInPath { - globalCount++ + commonParams = append(commonParams, parameter.Name) } } } for method, operation := range pathItem.Operations() { - var count uint + var setParams []string for _, parameterRef := range operation.Parameters { if parameterRef != nil { if parameter := parameterRef.Value; parameter != nil && parameter.In == ParameterInPath { - count++ + setParams = append(setParams, parameter.Name) } } } - if count+globalCount != pathParamsCount { - return fmt.Errorf("operation %s %s must define exactly all path parameters", method, path) + if expected := len(setParams) + len(commonParams); expected != len(varsInPath) { + expected -= len(varsInPath) + if expected < 0 { + expected *= -1 + } + missing := make(map[string]struct{}, expected) + for _, name := range append(setParams, commonParams...) { + if _, ok := varsInPath[name]; !ok { + missing[name] = struct{}{} + } + } + if len(missing) != 0 { + return fmt.Errorf("operation %s %s must define exactly all path parameters (missing: %v)", method, path, missing) + } } } @@ -75,9 +87,9 @@ func (paths Paths) Find(key string) *PathItem { return pathItem } - normalizedPath, expected := normalizeTemplatedPath(key) + normalizedPath, expected, _ := normalizeTemplatedPath(key) for path, pathItem := range paths { - pathNormalized, got := normalizeTemplatedPath(path) + pathNormalized, got, _ := normalizeTemplatedPath(path) if got == expected && pathNormalized == normalizedPath { return pathItem } @@ -85,43 +97,51 @@ func (paths Paths) Find(key string) *PathItem { return nil } -func normalizeTemplatedPath(path string) (string, uint) { +func normalizeTemplatedPath(path string) (string, uint, map[string]struct{}) { if strings.IndexByte(path, '{') < 0 { - return path, 0 + return path, 0, nil } - var buf strings.Builder - buf.Grow(len(path)) + var buffTpl strings.Builder + buffTpl.Grow(len(path)) var ( cc rune count uint isVariable bool + vars = make(map[string]struct{}) + buffVar strings.Builder ) for i, c := range path { if isVariable { if c == '}' { - // End path variables + // End path variable + isVariable = false + + vars[buffVar.String()] = struct{}{} + buffVar = strings.Builder{} + // First append possible '*' before this character // The character '}' will be appended if i > 0 && cc == '*' { - buf.WriteRune(cc) + buffTpl.WriteRune(cc) } - isVariable = false } else { - // Skip this character + buffVar.WriteRune(c) continue } + } else if c == '{' { // Begin path variable - // The character '{' will be appended isVariable = true + + // The character '{' will be appended count++ } // Append the character - buf.WriteRune(c) + buffTpl.WriteRune(c) cc = c } - return buf.String(), count + return buffTpl.String(), count, vars } diff --git a/openapi3/testdata/Test_param_override.yml b/openapi3/testdata/Test_param_override.yml new file mode 100644 index 000000000..d6982414a --- /dev/null +++ b/openapi3/testdata/Test_param_override.yml @@ -0,0 +1,40 @@ +openapi: 3.0.0 +info: + title: customer + version: '1.0' +servers: + - url: 'httpbin.kwaf-demo.test' +paths: + '/customers/{customer_id}': + parameters: + - schema: + type: integer + name: customer_id + in: path + required: true + get: + parameters: + - schema: + type: integer + maximum: 100 + name: customer_id + in: path + required: true + summary: customer + tags: [] + responses: + '200': + description: OK + content: + application/json: + schema: + type: object + properties: + customer_id: + type: integer + customer_name: + type: string + operationId: get-customers-customer_id + description: Retrieve a specific customer by ID +components: + schemas: {} From 198ec83cb4f37baafd15197066a48a8792dd7914 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 23 Jul 2021 13:58:50 +0200 Subject: [PATCH 2/2] re-fix issue 223 Signed-off-by: Pierre Fenoll --- openapi3/parameter_issue223_test.go | 2 +- openapi3/paths.go | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/openapi3/parameter_issue223_test.go b/openapi3/parameter_issue223_test.go index 336d42fb1..9b86954f4 100644 --- a/openapi3/parameter_issue223_test.go +++ b/openapi3/parameter_issue223_test.go @@ -112,5 +112,5 @@ components: doc, err := NewLoader().LoadFromData([]byte(spec)) require.NoError(t, err) err = doc.Validate(context.Background()) - require.EqualError(t, err, `invalid paths: operation GET /pets/{petId} must define exactly all path parameters`) + require.EqualError(t, err, `invalid paths: operation GET /pets/{petId} must define exactly all path parameters (missing: [petId])`) } diff --git a/openapi3/paths.go b/openapi3/paths.go index c31ef4b6e..bdb87ae7d 100644 --- a/openapi3/paths.go +++ b/openapi3/paths.go @@ -50,13 +50,30 @@ func (value Paths) Validate(ctx context.Context) error { expected *= -1 } missing := make(map[string]struct{}, expected) - for _, name := range append(setParams, commonParams...) { + definedParams := append(setParams, commonParams...) + for _, name := range definedParams { if _, ok := varsInPath[name]; !ok { missing[name] = struct{}{} } } + for name := range varsInPath { + got := false + for _, othername := range definedParams { + if othername == name { + got = true + break + } + } + if !got { + missing[name] = struct{}{} + } + } if len(missing) != 0 { - return fmt.Errorf("operation %s %s must define exactly all path parameters (missing: %v)", method, path, missing) + missings := make([]string, 0, len(missing)) + for name := range missing { + missings = append(missings, name) + } + return fmt.Errorf("operation %s %s must define exactly all path parameters (missing: %v)", method, path, missings) } } }