Fix double encoding of URI template parameter names for list/associative-array values - #5280
Open
iscai-msft wants to merge 3 commits into
Open
Fix double encoding of URI template parameter names for list/associative-array values#5280iscai-msft wants to merge 3 commits into
iscai-msft wants to merge 3 commits into
Conversation
…ive-array values The scalar-value path in expandUrlTemplate already preserved a pre-encoded variable name, but the array/list path (getExpandedValue) and the non-expanded/associative-array path (getNonExpandedValue) still called encodeComponent/encodeURIComponent on varName, causing double encoding (e.g. "%24Select" becoming "%2524Select"). Fixes Azure#5278 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
microsoft-github-policy-service
Bot
added
the
emitter:typescript
Contributor
📦 Package size report✅ No notable package size changes compared to the base branch. 13 package(s) with no notable change
Packed = gzipped |
Contributor
|
You can try these changes here
|
Maor Leger (maorleger)
approved these changes
Aug 21, 2026
Maor Leger (maorleger)
left a comment
Maor Leger (maorleger)
left a comment
Member
There was a problem hiding this comment.
The fix LGTM but I'd love to get confirmation from the emitter folks
Thanks for putting that together!
| }); | ||
| assert("{?%24Select*}", "?name=value"); | ||
| assert("{?%24Select}", "?%24Select=name,value"); | ||
| }); |
There was a problem hiding this comment.
Is it worth adding a test that combines encoded name with values requiring encoding?
Example (I think):
const assert = createAssertion({
"%24Select": ["a b", "x=y", "*"],
});
assert(
"{?%24Select*}",
"?%24Select=a%20b&%24Select=x%3Dy&%24Select=%2A",
);Or something similar? just a test that shows that we continue to encode the values even when preserving the already-encoded name.
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
2 participants
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.
Summary
Fixes #5278
Query parameter names are pre-encoded by the emitter (e.g.
$Select→%24Select) before being passed as URI template variable names. The scalar-value expansion path inexpandUrlTemplatealready accounted for this and skipped re-encoding the variable name, but the array-expansion path (getExpandedValue) and the associative-array/list path (getNonExpandedValue) still calledencodeURIComponent/encodeComponentonvarName, causing it to be encoded twice (e.g.%24Selectbecoming%2524Select).Fix
Removed the redundant re-encoding of
varNamein bothgetExpandedValueandgetNonExpandedValueinpackages/typespec-ts/static/static-helpers/urlTemplate.ts, matching the existing (correct) behavior of the scalar-value path.Testing
Added repro tests in
packages/typespec-ts/test/modular-unit/static/url-template.test.tscovering scalar, list, and associative-array query parameter values with a pre-encoded parameter name (%24Select). Verified the new list/associative-array tests fail onmainwith the exact reported symptom (%2524Select) and pass with this fix. All 67 tests in the file pass.Also ran
pnpm linton the touched package (clean).