[App Configuration] - Add query param policy #53137
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new query parameter normalization pipeline policy to the Azure App Configuration SDK. The policy ensures query parameters are lowercase and sorted alphabetically to support Azure Front Door as a CDN.
- Implements a new
QueryParamPolicythat transforms query parameters to lowercase and sorts them alphabetically - Adds comprehensive test coverage for various query parameter scenarios including duplicates and encoded values
- Removes the unused
ApiVersionPolicyclass as part of cleanup
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 |
|---|---|
QueryParamPolicy.cs |
New pipeline policy that normalizes query parameters by converting to lowercase and sorting alphabetically |
QueryParamPolicyTests.cs |
Comprehensive test suite covering lowercase conversion, sorting, duplicate handling, and value preservation |
ConfigurationClient.cs |
Integration of the new QueryParamPolicy into the HTTP pipeline |
ApiVersionPolicy.cs |
Removal of unused ApiVersionPolicy class |
assets.json |
Version tag update for the changes |
sdk/appconfiguration/Azure.Data.AppConfiguration/src/QueryParamPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/QueryParamPolicy.cs
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/Azure.Data.AppConfiguration/src/QueryParamPolicy.cs
Show resolved
Hide resolved
| queryString = queryString.Substring(1); | ||
| } | ||
|
|
||
| // Separate out any hash fragment so we can keep it verbatim |
There was a problem hiding this comment.
I think we should do queryString = Uri.Query + Uri.Fragment.
That saves us from parsing and reconstructing the fragment part.
There was a problem hiding this comment.
There is no Request.Uri.Fragment https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/src/RequestUriBuilder.cs
I need to convert it to Uri first:
Uri originalUrl = message.Request.Uri.ToUri();|
|
||
| string baseUrl = beforeHash.Substring(0, beforeHash.IndexOf('?')); | ||
|
|
||
| // We don't use HttpUtility.ParseQueryString as it will process the values |
There was a problem hiding this comment.
We can still use HttpUtility.ParseQueryString and access the duplicate query params by calling NameValueCollection.GetValues on the parsed values.
Eg:
var url = new Uri("https://www.contoso.com/kv?key=Foo&label=My#Label&tags=t2=v2&tags=t1=v1");
var queryString = url.Query + url.Fragment;
var paramsCollection = HttpUtility.ParseQueryString(queryString);
foreach (var key in paramsCollection.AllKeys)
{
var values = paramsCollection.GetValues(key);
foreach (var val in values)
{
Console.WriteLine($"{key} = {val}");
}
}
Output:
key = Foo
label = My#Label
tags = t2=v2
tags = t1=v1
There was a problem hiding this comment.
Should we distinguish these two cases ?key= and ?key? If yes, then we must parse the query by ourselves. Any url parsing tool will treat ?key as key: "", so when we reconstruct the query, we will add the =
| { | ||
| public class QueryParamPolicyTests : SyncAsyncPolicyTestBase | ||
| { | ||
| public QueryParamPolicyTests(bool isAsync) : base(isAsync) { } |
There was a problem hiding this comment.
I like it - nice and simple tests. Can you add more test cases?
Mix and match with $select, null values, different endpoints (snapshot case), etc
Why this PR?
Adds a new pipeline to update the query params to make sure they are all lowercase and in alphabetical order. This is required for support of Azure Front Door as a CDN.