diff --git a/parameters/path_parameters.go b/parameters/path_parameters.go index c5fd7ac..2b162c6 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,11 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p } matches := rgx.FindStringSubmatch(submittedSegments[x]) + if matches == nil { + validationErrors = append(validationErrors, + errors.PathParameterMissing(p, pathValue, request.URL.Path)) + break + } matches = matches[1:] // Check if it is well-formed. @@ -380,6 +392,21 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p return true, nil } +// 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..e1d5fde 100644 --- a/parameters/path_parameters_test.go +++ b/parameters/path_parameters_test.go @@ -2348,3 +2348,79 @@ 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) +}