diff --git a/parameters/path_parameters.go b/parameters/path_parameters.go index c5fd7ac..7655770 100644 --- a/parameters/path_parameters.go +++ b/parameters/path_parameters.go @@ -42,9 +42,11 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p HowToFix: errors.HowToFixPath, }} } - // split the path into segments - submittedSegments := strings.Split(paths.StripRequestPath(request, v.document), helpers.Slash) - pathSegments := strings.Split(pathValue, helpers.Slash) + // split the path into segments, dropping empty segments so that a request + // path containing a double slash (e.g. //test/path) does not shift the + // index alignment between submitted and template segments. + submittedSegments := nonEmptyPathSegments(paths.StripRequestPath(request, v.document)) + pathSegments := nonEmptyPathSegments(pathValue) // get the operation method for error reporting operation := strings.ToLower(request.Method) @@ -54,12 +56,17 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p var validationErrors []*errors.ValidationError for _, p := range params { if p.In == helpers.Path { + // a mismatch in segment counts means the submitted path cannot be + // aligned with the template (e.g. an extra empty segment from a + // double slash); reject it rather than silently mis-validating. + if len(submittedSegments) != len(pathSegments) { + validationErrors = append(validationErrors, + errors.PathParameterMissing(p, pathValue, request.URL.Path)) + continue + } + // var paramTemplate string for x := range pathSegments { - if pathSegments[x] == "" { // skip empty segments - continue - } - var rgx *regexp.Regexp if v.options.RegexCache != nil { @@ -83,6 +90,20 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p } matches := rgx.FindStringSubmatch(submittedSegments[x]) + if matches == nil { + // the submitted segment doesn't satisfy this template + // segment's regex. only flag the current outer-loop + // parameter as missing if this template segment is + // actually the one that declares it; otherwise the + // failure belongs to a different parameter and we leave + // it for that parameter's iteration to report. + if segmentReferencesParam(pathSegments[x], p.Name) { + validationErrors = append(validationErrors, + errors.PathParameterMissing(p, pathValue, request.URL.Path)) + break + } + continue + } matches = matches[1:] // Check if it is well-formed. @@ -380,6 +401,51 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p return true, nil } +// segmentReferencesParam reports whether the given path template segment +// (e.g. "{id:[0-9]+}" or "items.{id}.json") contains a brace-pair whose +// parameter name matches the supplied name. It is used to attribute a +// regex no-match failure to the correct parameter on multi-param routes. +func segmentReferencesParam(segment, name string) bool { + idxs, err := helpers.BraceIndices(segment) + if err != nil || len(idxs) < 2 { + return false + } + for i := 0; i+1 < len(idxs); i += 2 { + raw := segment[idxs[i]+1 : idxs[i+1]-1] + // strip explode suffix + if strings.HasSuffix(raw, helpers.Asterisk) { + raw = raw[:len(raw)-1] + } + // strip label/matrix prefix + if strings.HasPrefix(raw, helpers.Period) || strings.HasPrefix(raw, helpers.SemiColon) { + raw = raw[1:] + } + // drop any constraining regex `name:pattern` + if colon := strings.Index(raw, ":"); colon >= 0 { + raw = raw[:colon] + } + if raw == name { + return true + } + } + return false +} + +// nonEmptyPathSegments splits a path on "/" and drops empty segments, so that +// leading, trailing, or repeated slashes do not introduce empty entries that +// would shift index alignment between submitted and template segments. +func nonEmptyPathSegments(path string) []string { + raw := strings.Split(path, helpers.Slash) + segments := make([]string, 0, len(raw)) + for _, segment := range raw { + if segment == "" { + continue + } + segments = append(segments, segment) + } + return segments +} + func (v *paramValidator) resolveNumber(sch *base.Schema, p *v3.Parameter, isLabel bool, isMatrix bool, paramValue string, pathValue string, renderedSchema string) (string, float64, []*errors.ValidationError) { if isLabel && p.Style == helpers.LabelStyle { paramValueParsed, err := strconv.ParseFloat(paramValue[1:], 64) diff --git a/parameters/path_parameters_test.go b/parameters/path_parameters_test.go index 5972439..3ffdd3b 100644 --- a/parameters/path_parameters_test.go +++ b/parameters/path_parameters_test.go @@ -2348,3 +2348,120 @@ paths: assert.EqualValues(t, 1, cache.storeCount, "No new stores on cache hit") assert.EqualValues(t, 1, cache.hitCount, "Second OData lookup should hit cache") } + +func TestValidatePathParamsWithPathItem_DoubleSlashDoesNotPanic(t *testing.T) { + // Regression test for #274: ValidatePathParamsWithPathItem panics for + // request paths containing a leading double slash (e.g. //test/path/x), + // because path segments and submitted segments differ in length. + spec := `openapi: 3.1.0 +paths: + /test/path/{param}: + get: + operationId: testParam + parameters: + - in: path + name: param + required: true + schema: + type: string + responses: + "200": + description: ok` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model) + + req, _ := http.NewRequest(http.MethodGet, "https://example.com//test/path/fubar", nil) + pathItem := m.Model.Paths.PathItems.GetOrZero("/test/path/{param}") + require.NotNil(t, pathItem) + + // the leading double slash collapses to an empty segment that must be + // dropped, so the remaining segments still align and 'fubar' is validated + // against {param} instead of being silently accepted against the wrong + // segment. + valid, errs := v.ValidatePathParamsWithPathItem(req, pathItem, "/test/path/{param}") + assert.True(t, valid) + assert.Empty(t, errs) + + // When the submitted path has fewer segments than the spec path it can no + // longer be aligned, so it must be rejected rather than mis-validated. + shortReq, _ := http.NewRequest(http.MethodGet, "https://example.com/test/path", nil) + valid, errs = v.ValidatePathParamsWithPathItem(shortReq, pathItem, "/test/path/{param}") + assert.False(t, valid) + assert.Len(t, errs, 1) +} + +func TestValidatePathParamsWithPathItem_RegexNoMatchContinues(t *testing.T) { + // When a submitted path segment does not match the regex for a + // constrained path template segment (e.g. {id:[0-9]+} vs "abc"), the + // guard must not panic and must return a validation error. + spec := `openapi: 3.1.0 +paths: + /items/{id:[0-9]+}: + get: + operationId: getItem + parameters: + - in: path + name: id + required: true + schema: + type: integer + responses: + "200": + description: ok` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model) + + // "abc" does not match ^([0-9]+)$; expect invalid + at least one error. + req, _ := http.NewRequest(http.MethodGet, "https://example.com/items/abc", nil) + pathItem := m.Model.Paths.PathItems.GetOrZero("/items/{id:[0-9]+}") + require.NotNil(t, pathItem) + + valid, errs := v.ValidatePathParamsWithPathItem(req, pathItem, "/items/{id:[0-9]+}") + assert.False(t, valid) + assert.NotEmpty(t, errs) +} + +func TestValidatePathParamsWithPathItem_RegexNoMatchAttributesToCorrectParam(t *testing.T) { + // On a multi-param route /items/{id:[0-9]+}/{slug}, submitting + // /items/abc/foo must report only `id` as the failing parameter; `slug` + // is supplied (foo) and must not be flagged missing. + spec := `openapi: 3.1.0 +paths: + /items/{id:[0-9]+}/{slug}: + get: + operationId: getItem + parameters: + - in: path + name: id + required: true + schema: + type: integer + - in: path + name: slug + required: true + schema: + type: string + responses: + "200": + description: ok` + + doc, _ := libopenapi.NewDocument([]byte(spec)) + m, _ := doc.BuildV3Model() + v := NewParameterValidator(&m.Model) + + req, _ := http.NewRequest(http.MethodGet, "https://example.com/items/abc/foo", nil) + pathItem := m.Model.Paths.PathItems.GetOrZero("/items/{id:[0-9]+}/{slug}") + require.NotNil(t, pathItem) + + valid, errs := v.ValidatePathParamsWithPathItem(req, pathItem, "/items/{id:[0-9]+}/{slug}") + assert.False(t, valid) + require.NotEmpty(t, errs) + for _, e := range errs { + assert.NotContains(t, e.Message, "'slug'", + "slug is present (foo) and must not be flagged missing") + } +}