[Repo Assist] Fix $0 in path parameter values treated as regex back-reference#310
Merged
sergey-tihon merged 3 commits intomasterfrom Mar 10, 2026
Conversation
Regex.Replace uses $0, $1, $& etc. as back-references in the replacement string. When a path parameter value contained a literal dollar sign (e.g. "$0something"), Regex.Replace would substitute it with the matched text instead of the literal value. Fix: escape $ characters in the replacement string by replacing "$" with "$$" (the .NET replacement-string escape for a literal dollar sign). This applies to both the v2 SwaggerClientProvider and v3 OpenApiClientProvider. Also adds: - EchoPathController in the test server that returns its string path parameter - A regression test in SpecialCasesControllers.Tests.fs that passes "$0something" through a path parameter and asserts the value is echoed back unchanged - global.json rollForward updated from minor/10.0.103 to latestPatch/10.0.100 so the project builds with any 10.0.1xx SDK available in CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a path-parameter substitution bug in the generated clients where literal $ characters in path values were interpreted as Regex.Replace back-references (e.g., $0, $&) instead of being passed through literally. This impacts both the Swagger v2 and OpenAPI v3 client providers.
Changes:
- Escape
$inRegex.Replacereplacement strings for path parameter substitution (v2 + v3 operation compilers). - Add a test-server controller that echoes a string path parameter and a v3 regression test covering
$0something. - Update
global.jsonSDK pin/roll-forward behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Swashbuckle.WebApi.Server/Controllers/SpecialCasesControllers.fs | Adds EchoPathController endpoint used to validate literal $ round-tripping through path params. |
| tests/SwaggerProvider.ProviderTests/v3/Swashbuckle.SpecialCasesControllers.Tests.fs | Adds regression test for $ in path params (issue #141). |
| src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs | Escapes $ in path-parameter replacement to avoid regex replacement back-references. |
| src/SwaggerProvider.DesignTime/v2/OperationCompiler.fs | Same $ escaping fix for the v2 code path. |
| global.json | Changes SDK version/rollForward settings to use 10.0.100 + latestPatch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Member
Author
|
@copilot merge master |
Contributor
|
@sergey-tihon I've opened a new pull request, #311, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Fix Option types in form data not being unwrapped (issue #214) (#298) RuntimeHelpers.getPropertyValues now strips the FSharpOption<T> wrapper before returning property values. Previously, a property set to Some(true) was serialised as the string "Some(true)" in multipart/form-data and application/x-www-form-urlencoded bodies instead of the bare "True". None values were already handled (null). Also relax global.json SDK version constraint from 10.0.103 to 10.0.100 with rollForward:latestPatch so the project builds with any 10.0.1xx SDK available locally or in CI. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add concurrency group to CI workflow to cancel redundant builds (#309) When multiple commits are pushed to the same PR branch, GitHub Actions queues a new build for each push. Without a concurrency group, the older builds keep running even though they're now stale. Adding cancel-in-progress: true cancels the in-progress build when a newer commit arrives on the same ref, reducing CI minutes consumed and giving faster feedback on the latest commit. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * [Repo Assist] Add unit tests for RuntimeHelpers module (#295) * Add unit tests for RuntimeHelpers module RuntimeHelpers.fs handles all parameter serialization and HTTP request building for generated API clients, but previously had zero dedicated unit tests. This PR adds 44 focused tests covering: - toParam: DateTime/DateTimeOffset ISO 8601 formatting, null handling - toQueryParams: all supported array/option/scalar types, DateTime ISO 8601 serialization, None filtering in optional arrays - combineUrl: trailing/leading slash normalisation - createHttpRequest: HTTP method, path, query params, null filtering - fillHeaders: header addition, null-value skip, Content-Type silence - toStringContent/toTextContent/toStreamContent: content-type headers, error on non-stream input Closes no specific issue; improves baseline test coverage for the runtime serialization layer (Task 9 - Testing Improvements). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * ci: trigger checks * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types (#304) * Initial plan * Add missing toQueryParams test cases: float32/double/byte arrays and Option array types Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Merge master and fix CI: add RuntimeHelpers unit tests (#305) * Fix Option types in form data not being unwrapped (issue #214) (#298) RuntimeHelpers.getPropertyValues now strips the FSharpOption<T> wrapper before returning property values. Previously, a property set to Some(true) was serialised as the string "Some(true)" in multipart/form-data and application/x-www-form-urlencoded bodies instead of the bare "True". None values were already handled (null). Also relax global.json SDK version constraint from 10.0.103 to 10.0.100 with rollForward:latestPatch so the project builds with any 10.0.1xx SDK available locally or in CI. Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Initial plan * Fix Build and Test workflow to run on all pull requests, not just PRs targeting master Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Remove unreachable null check from `toQueryParams` fallthrough case (#306) * Initial plan * Fix toQueryParams: remove unreachable null check from _ case Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Make null handling explicit in `toQueryParams` and fix misleading test comment (#307) * Initial plan * Add explicit null guard in toQueryParams and fix test comment Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> * Apply Fantomas formatting to RuntimeHelpers.fs Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com> Co-authored-by: Don Syme <dsyme@users.noreply.github.com> * Initial plan --------- Co-authored-by: Don Syme <dsyme@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sergey Tihon <sergey.tihon@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: sergey-tihon <1197905+sergey-tihon@users.noreply.github.com>
…-param-0274620a2548eff5
sergey-tihon
deleted the
repo-assist/fix-issue-141-dollar-sign-path-param-0274620a2548eff5
branch
7 tasks
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.
Regex.Replace uses $0,$1, $ & etc. as back-references in the replacement string. When a path parameter value contained a literal dollar sign (e.g. "$0something"), Regex.Replace would substitute it with the matched text instead of the literal value.
Fix: escape $ characters in the replacement string by replacing "$" with "$$" (the .NET replacement-string escape for a literal dollar sign). This applies to both the v2 SwaggerClientProvider and v3 OpenApiClientProvider.
Also adds:
Closes #141 and #302