-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[Azure App Configuration] - Normalize the query parameters in request url #35962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
zhiyuanliang-ms
merged 25 commits into
Azure:main
from
zhiyuanliang-ms:zhiyuanliang/sort-query-param
Oct 26, 2025
Merged
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
7b0cfd7
add policy
zhiyuanliang-ms 1ffceb0
add testcase
zhiyuanliang-ms bb14d10
remove debug code
zhiyuanliang-ms d81b70a
Merge branch 'main' of https://github.com/zhiyuanliang-ms/azure-sdk-f…
zhiyuanliang-ms 042e19f
update recording
zhiyuanliang-ms a3c0992
fix format
zhiyuanliang-ms b0f8ce2
update
zhiyuanliang-ms aca2dd9
Merge branch 'main' of https://github.com/zhiyuanliang-ms/azure-sdk-f…
zhiyuanliang-ms 387d730
add test for tag query
zhiyuanliang-ms 1bd5a4e
update
zhiyuanliang-ms 141e4f7
Merge branch 'main' of https://github.com/zhiyuanliang-ms/azure-sdk-f…
zhiyuanliang-ms 857c849
update
zhiyuanliang-ms 6376b8c
update
zhiyuanliang-ms a68e541
update
zhiyuanliang-ms 3f06358
update
zhiyuanliang-ms ee92713
Merge branch 'main' of https://github.com/zhiyuanliang-ms/azure-sdk-f…
zhiyuanliang-ms b461666
update
zhiyuanliang-ms dcf63a6
update test
zhiyuanliang-ms ccbb155
update test
zhiyuanliang-ms 5218baf
handle corner case that query doesn't have =
zhiyuanliang-ms 4b538f5
handle corner case that query is empty
zhiyuanliang-ms 43994c3
update testcase
zhiyuanliang-ms 445920a
update testcase
zhiyuanliang-ms 1fdc43a
update testcase
zhiyuanliang-ms e35413a
update
zhiyuanliang-ms File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
2 changes: 1 addition & 1 deletion
2
sdk/appconfiguration/app-configuration/src/generated/src/models/parameters.ts
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
70 changes: 70 additions & 0 deletions
70
sdk/appconfiguration/app-configuration/src/internal/queryParamPolicy.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import type { | ||
| PipelinePolicy, | ||
| PipelineRequest, | ||
| PipelineResponse, | ||
| SendRequest, | ||
| } from "@azure/core-rest-pipeline"; | ||
|
|
||
| /** | ||
| * Creates a PipelinePolicy that normalizes query parameters: | ||
| * - Lowercase names | ||
| * - Sort by lowercase name | ||
| * - Preserve the relative order of duplicates | ||
| */ | ||
| export function queryParamPolicy(): PipelinePolicy { | ||
| return { | ||
| name: "queryParamPolicy", | ||
| async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> { | ||
| try { | ||
| const originalUrl: string = request.url; | ||
| const url = new URL(originalUrl); | ||
|
|
||
| if (url.search === "") { | ||
| return next(request); | ||
| } | ||
|
|
||
| const params: ParamEntry[] = []; | ||
| for (const entry of url.search.substring(1).split("&")) { | ||
| if (entry === "") { | ||
| continue; | ||
| } | ||
| const [name, value] = entry.split("=", 2); | ||
jimmyca15 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| params.push({ lowercaseName: name.toLowerCase(), value: value ?? "" }); | ||
| } | ||
|
|
||
| // Modern JavaScript Array.prototype.sort is stable | ||
| // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#sort_stability | ||
| params.sort((a, b) => { | ||
| if (a.lowercaseName < b.lowercaseName) { | ||
| return -1; | ||
| } else if (a.lowercaseName > b.lowercaseName) { | ||
| return 1; | ||
| } | ||
| return 0; | ||
| }); | ||
|
|
||
| const newSearchParams = params | ||
| .map(({ lowercaseName, value }) => `${lowercaseName}=${value}`) | ||
| .join("&"); | ||
|
|
||
| const newUrl = url.origin + url.pathname + "?" + newSearchParams + url.hash; | ||
jimmyca15 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (newUrl !== originalUrl) { | ||
| request.url = newUrl; | ||
| } | ||
| } catch { | ||
| // If anything goes wrong, fall back to sending the original request. | ||
| console.log("Failed to normalize query parameters."); | ||
zhiyuanliang-ms marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| return next(request); | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| interface ParamEntry { | ||
| lowercaseName: string; | ||
| value: string; | ||
| } | ||
File renamed without changes.
2 changes: 1 addition & 1 deletion
2
sdk/appconfiguration/app-configuration/test/internal/node/http.spec.ts
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
83 changes: 83 additions & 0 deletions
83
sdk/appconfiguration/app-configuration/test/internal/queryParamPolicy.spec.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import { describe, it, expect } from "vitest"; | ||
| import { queryParamPolicy } from "../../src/internal/queryParamPolicy.js"; | ||
| import { createPipelineRequest, createHttpHeaders } from "@azure/core-rest-pipeline"; | ||
| import type { PipelineRequest, PipelineResponse } from "@azure/core-rest-pipeline"; | ||
|
|
||
| function mockNext(returnStatus: number = 200) { | ||
| return async (request: PipelineRequest): Promise<PipelineResponse> => { | ||
| return { | ||
| request, | ||
| headers: createHttpHeaders({ "url-lookup": request.url }), | ||
| status: returnStatus, | ||
| } as PipelineResponse; | ||
| }; | ||
| } | ||
|
|
||
| describe("urlQueryParamsNormalizationPolicy", () => { | ||
| it("normalizes query parameters", async () => { | ||
| const policy = queryParamPolicy(); | ||
| const request = createPipelineRequest({ | ||
| url: "https://example.azconfig.io/kv?api-version=2023-11-01&After=abcdefg&tags=tag3%3Dvalue3&key=*&label=dev&$Select=key&tags=tag2%3Dvalue2&tags=tag1%3Dvalue1", | ||
| }); | ||
| const response = await policy.sendRequest(request, mockNext()); | ||
| const finalUrl = response.headers.get("url-lookup")!; | ||
| expect( | ||
| finalUrl.endsWith( | ||
| "?$select=key&after=abcdefg&api-version=2023-11-01&key=*&label=dev&tags=tag3%3Dvalue3&tags=tag2%3Dvalue2&tags=tag1%3Dvalue1", | ||
| ), | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it("keeps original encoded parameter value", async () => { | ||
| const policy = queryParamPolicy(); | ||
| const request = createPipelineRequest({ | ||
| url: "https://example.azconfig.io/kv?key=%25%20%2B&label=%00", | ||
| }); | ||
| const response = await policy.sendRequest(request, mockNext()); | ||
| const finalUrl = response.headers.get("url-lookup")!; | ||
| expect(finalUrl.endsWith("?key=%25%20%2B&label=%00")).toBe(true); | ||
| }); | ||
|
|
||
| it("keeps original order of query parameters", async () => { | ||
| const policy = queryParamPolicy(); | ||
| const request = createPipelineRequest({ | ||
| url: "https://example.azconfig.io/kv?tags=tag2&api-version=2023-11-01&tags=tag1", | ||
| }); | ||
| const response = await policy.sendRequest(request, mockNext()); | ||
| const finalUrl = response.headers.get("url-lookup")!; | ||
| expect(finalUrl.endsWith("?api-version=2023-11-01&tags=tag2&tags=tag1")).toBe(true); | ||
| }); | ||
|
|
||
| it("keeps empty parameter value", async () => { | ||
| const policy = queryParamPolicy(); | ||
| const request = createPipelineRequest({ | ||
| url: "https://example.azconfig.io/kv?key=&api-version=2023-11-01&key1&=", | ||
| }); | ||
| const response = await policy.sendRequest(request, mockNext()); | ||
| const finalUrl = response.headers.get("url-lookup")!; | ||
| expect(finalUrl.endsWith("?=&api-version=2023-11-01&key=&key1=")).toBe(true); | ||
| }); | ||
|
|
||
| it("removes redundant &", async () => { | ||
| const policy = queryParamPolicy(); | ||
| const request = createPipelineRequest({ | ||
| url: "https://example.azconfig.io/kv?b=2&&a=1", | ||
| }); | ||
| const response = await policy.sendRequest(request, mockNext()); | ||
| const finalUrl = response.headers.get("url-lookup")!; | ||
| expect(finalUrl.endsWith("?a=1&b=2")).toBe(true); | ||
| }); | ||
|
|
||
| it("skips when no query parameters are present", async () => { | ||
| const policy = queryParamPolicy(); | ||
| const request = createPipelineRequest({ | ||
| url: "https://example.azconfig.io/kv?", | ||
| }); | ||
| const response = await policy.sendRequest(request, mockNext()); | ||
| const finalUrl = response.headers.get("url-lookup")!; | ||
| expect(finalUrl.endsWith("/kv?")).toBe(true); | ||
| }); | ||
| }); |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.