Anchor trailing-slash stdhttp routes with {$} to match OpenAPI semantics#2460
Merged
Conversation
Closes: #2065 A ServeMux pattern ending in '/' matches the whole subtree below it, while an OpenAPI path ending in '/' means exactly that path. Registering spec paths as subtree patterns both mismatched the spec's semantics and panicked at registration when two such patterns overlapped ambiguously (e.g. "/api/test/{id}/test2/" and "/api/test/test3/", which both match "/api/test/test3/test2/" with neither more specific). SwaggerUriToStdHttpUri now appends the {$} end-of-URL anchor to any path ending in '/', generalizing the existing special case for the root path. The anchor is added after parameter-name sanitization so the '$' is not mistaken for a parameter. Behavior change for existing specs with trailing-slash paths: such routes no longer act as catch-all subtrees, and requests to deeper paths now return 404, matching what the spec declares. ServeMux still redirects a request missing the trailing slash to the canonical path (with a 307), which the new test leaf pins alongside the issue's panic repro. Since we are changing behavior in the direction of correctness, I am not flagifying this. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mromaszewicz
force-pushed
the
fix/issue-2065
branch
from
July 13, 2026 01:07
c43c795 to
b5c4fa1
Compare
Contributor
Greptile SummaryThis PR fixes issue #2065 by generalizing the existing root-path
Confidence Score: 5/5Safe to merge; the change is a targeted, well-tested correction to stdhttp route registration semantics with no unrelated diff churn. The core fix in utils.go is minimal and correct. The generated fixture confirms the right patterns are emitted. Tests cover the panic repro, exact routing, sub-path 404, and redirect behavior. No unrelated template changes; no other backends touched. No files require special attention beyond server_test.go, which is missing the //go:build go1.22 build constraint already noted in a prior review thread.
|
| Filename | Overview |
|---|---|
| pkg/codegen/utils.go | Generalizes the root-path {$} special case into a universal trailing-slash anchor for all paths, correctly placed after parameter sanitization so $ is never mistaken for a wildcard name. |
| pkg/codegen/utils_test.go | Adds three unit-test cases covering the new {$}-anchoring logic for plain, parameterized, and literal trailing-slash paths. |
| internal/test/servers/routers/trailing_slash/server_test.go | Integration test covering the panic-repro case from issue #2065, exact-match routing, 404 for sub-paths, and trailing-slash redirect; missing //go:build go1.22 build constraint (already flagged in prior review thread). |
| internal/test/servers/routers/trailing_slash/trailing_slash.gen.go | Generated file correctly registers routes with {$} anchors; //go:build go1.22 tag present; size proportional to the change. |
| internal/test/servers/routers/trailing_slash/spec.yaml | New spec with the exact two overlapping paths from issue #2065, placed correctly under servers/routers/ with provenance comment. |
Reviews (2): Last reviewed commit: "Anchor trailing-slash stdhttp routes wit..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes: #2065
A ServeMux pattern ending in '/' matches the whole subtree below it, while an OpenAPI path ending in '/' means exactly that path. Registering spec paths as subtree patterns both mismatched the spec's semantics and panicked at registration when two such patterns overlapped ambiguously (e.g. "/api/test/{id}/test2/" and "/api/test/test3/", which both match "/api/test/test3/test2/" with neither more specific).
SwaggerUriToStdHttpUri now appends the {$} end-of-URL anchor to any path ending in '/', generalizing the existing special case for the root path. The anchor is added after parameter-name sanitization so the '$' is not mistaken for a parameter.
Behavior change for existing specs with trailing-slash paths: such routes no longer act as catch-all subtrees, and requests to deeper paths now return 404, matching what the spec declares. ServeMux still redirects a request missing the trailing slash to the canonical path (with a 307), which the new test leaf pins alongside the issue's panic repro.
Since we are changing behavior in the direction of correctness, I am not flagifying this.