Repository navigation
Restrict :param path params to the start of a path segment - #1839
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: d7ac54c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
All reported issues were addressed across 3 files
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
Member
|
Thank you |
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
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.
The parser preserves literal colon suffixes in generated URLs and recognizes colon parameters only at the start of a path segment.
Problem
/v1/items:searchproduced a requiredsearch: stringargument and the URL/v1/items${search}./v1/items/{itemId}:archivehad the same extra-argument and suffix-loss problem.Solution
src/schema-routes/schema-routes.tsuses the same segment boundary for parameter detection and replacement.A literal suffix cannot capture a later parameter's substitution.
The parser consumes a legacy trailing colon only before
/or the route end, so/:itemId:archiveretains its suffix.The parser retains
/users/:userIdand brace parameters for compatibility with existing clients.Full Express route grammar remains outside scope: embedded colon syntax becomes literal path text.
Verification
The inline OpenAPI regression test in
tests/spec/issue-1833/basic.test.tsfails on the baseline and passes with this fix.The package test suite passes.
Closes #1833.
Summary by cubic
Fixes routes with AIP-136 custom methods (like
/v1/items:searchor/v1/items/{itemId}:archive) generating a bogus requiredstringargument and dropping the literal:suffixfrom the request URL.:paramNameis now parsed as an Express-style path parameter only at the start of a path segment, so/users/:userIdkeeps working while mid-segment colons stay part of the literal path. The fix also keeps a legacy trailing-colon param form (/users/:id:) substitutable before/or the route end, and handles a colon param at the route start without a leading slash (as produced by hooks likeonPreBuildRoutePath).Added regression tests in
tests/spec/issue-1833/basic.test.tscovering the custom-method suffixes, the trailing-colon form, and the no-leading-slash case; they fail on the baseline and pass with the fix.Written for commit d7ac54c. Summary will update on new commits.