Skip to content

fix(parameters): avoid panic on double-slash request paths #275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SAY-5 wants to merge 6 commits into
base: main
Choose a base branch
from
Open

fix(parameters): avoid panic on double-slash request paths #275

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ff1378e
fix(parameters): avoid panic on double-slash request paths
SAY-5 May 21, 2026
f90e394
test: cover length-mismatch guard
SAY-5 May 21, 2026
cce08bf
fix(parameters): align path segments by dropping empty entries
SAY-5 May 24, 2026
3c36636
test: cover nil-match guard for constrained path template segments
SAY-5 May 25, 2026
d9a349e
fix(parameters): return error on regex no-match for constrained templ…
SAY-5 May 26, 2026
3fa334d
fix(parameters): attribute regex no-match to the correct param on mul…
SAY-5 Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions parameters/path_parameters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p
continue
}

// guard against length mismatch (e.g. request path containing
// a double slash producing extra empty segments).
if x >= len(submittedSegments) {
continue
}

var rgx *regexp.Regexp

if v.options.RegexCache != nil {
Expand All @@ -83,6 +89,9 @@ func (v *paramValidator) ValidatePathParamsWithPathItem(request *http.Request, p
}

matches := rgx.FindStringSubmatch(submittedSegments[x])
if matches == nil {

@daveshanley daveshanley May 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this avoids the panic, but it can silently accept the bad request. For //test/path/fubar against /test/path/{param}, the extra empty segment shifts indexing, so {param} is validated against "path" instead of "fubar", and ValidateHttpRequestWithPathItem returns valid=true, errs=0. This should either normalize segments consistently and validate fubar, or return a validation error; current behavior is neither.

@daveshanley daveshanley May 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a suggestion.

submittedSegments := nonEmptyPathSegments(paths.StripRequestPath(request, v.document))
  pathSegments := nonEmptyPathSegments(pathValue)

  if len(submittedSegments) != len(pathSegments) {
        return false, []*errors.ValidationError{
                errors.PathParameterMissing(p, pathValue, request.URL.Path),
        }
  }

  for x := range pathSegments {
        var rgx *regexp.Regexp
        // existing regex cache/build code...

        matches := rgx.FindStringSubmatch(submittedSegments[x])
        if matches == nil {
                continue
        }

        matches = matches[1:]
        // existing match validation code...
  }

  Helper:

  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
  }

The key is: don’t keep the original indexes after dropping/skipping empty template segments.
Either compact both sides first, or return a path/missing-param error when the segment counts
differ.

@SAY-5 SAY-5 May 25, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in the latest push: the approach now uses to drop empty segments from both sides before aligning them, so correctly validates against . For paths where segment counts still differ after stripping, the validator now returns a error (modeled on your suggestion). Test at line 2383 asserts the exact outcomes.

@daveshanley daveshanley May 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there... don’t silently continue on matches == nil for a templated segment. Return a path/parameter validation error there, and update the regression test to assert invalid/no panic, not only no panic.

continue
}
matches = matches[1:]

// Check if it is well-formed.
Expand Down
39 changes: 39 additions & 0 deletions parameters/path_parameters_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2348,3 +2348,42 @@ 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)

assert.NotPanics(t, func() {
Comment thread
daveshanley marked this conversation as resolved.
Outdated
_, _ = v.ValidatePathParamsWithPathItem(req, pathItem, "/test/path/{param}")
})

// Also cover the case where the submitted path has fewer segments than
// the spec path, which would otherwise index submittedSegments out of bounds.
shortReq, _ := http.NewRequest(http.MethodGet, "https://example.com/test/path", nil)
assert.NotPanics(t, func() {
_, _ = v.ValidatePathParamsWithPathItem(shortReq, pathItem, "/test/path/{param}")
})
}
Loading