-
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
+345
−8
Merged
Changes from 2 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
Some comments aren't visible on the classic Files Changed page.
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
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.
44 changes: 44 additions & 0 deletions
44
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,44 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import type { PipelinePolicy } from "@azure/core-rest-pipeline"; | ||
|
|
||
| /** | ||
| * A policy that normalizes query parameters for stable, canonical request URLs. | ||
| * Behavior: | ||
| * 1. All query parameter names are converted to lowercase (canonical form). | ||
| * 2. Parameters are sorted lexicographically by their lowercase names. | ||
| * 3. Relative order of duplicate names (ignoring case) is preserved (stable sort guarantee). | ||
| * | ||
| * This improves determinism for recordings and avoids casing-related cache misses. | ||
| * NOTE: Only enable if the target service treats parameter names case-insensitively. | ||
| * @internal | ||
| */ | ||
| export function urlQueryParamNormalizationPolicy(): PipelinePolicy { | ||
| return { | ||
| name: "urlQueryParamNormalizationPolicy", | ||
| async sendRequest(request, next) { | ||
| const qIndex = request.url.indexOf("?"); | ||
| if (qIndex === -1) { | ||
| return next(request); | ||
| } | ||
|
|
||
| const base = request.url.substring(0, qIndex); | ||
| const queryString = request.url.substring(qIndex + 1); | ||
| const params = new URLSearchParams(queryString); | ||
zhiyuanliang-ms marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const collected: Array<{ name: string; value: string; lower: string }> = []; | ||
| for (const [name, value] of params.entries()) { | ||
| collected.push({ name, value, lower: name.toLowerCase() }); | ||
| } | ||
| if (collected.length > 1) { | ||
| collected.sort((a, b) => (a.lower < b.lower ? -1 : a.lower > b.lower ? 1 : 0)); | ||
| } | ||
| const normalized = new URLSearchParams(); | ||
zhiyuanliang-ms marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| for (const p of collected) { | ||
| normalized.append(p.lower, p.value); | ||
| } | ||
| Object.assign(request, { url: `${base}?${normalized.toString()}` }); | ||
zhiyuanliang-ms marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
zhiyuanliang-ms marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return next(request); | ||
| }, | ||
| }; | ||
| } | ||
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
32 changes: 32 additions & 0 deletions
32
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,32 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import { describe, it, expect } from "vitest"; | ||
| import { urlQueryParamNormalizationPolicy } 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 = urlQueryParamNormalizationPolicy(); | ||
| const request = createPipelineRequest({ | ||
| url: "https://example.azconfig.io/kv?api-version=2023-11-01&After=abcdefg&key=*&label=dev&$Select=key", | ||
| }); | ||
| const response = await policy.sendRequest(request, mockNext()); | ||
| const finalUrl = response.headers.get("url-lookup")!; | ||
| console.log(finalUrl); | ||
zhiyuanliang-ms marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| expect( | ||
| finalUrl.endsWith("?%24select=key&after=abcdefg&api-version=2023-11-01&key=*&label=dev"), | ||
| ).toBe(true); | ||
| }); | ||
| }); | ||
159 changes: 159 additions & 0 deletions
159
sdk/appconfiguration/app-configuration/test/public/queryParam.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,159 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
zhiyuanliang-ms marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Licensed under the MIT License. | ||
|
|
||
| import type { | ||
| AppConfigurationClient | ||
| } from "../../src/index.js"; | ||
| import type { Recorder } from "@azure-tools/test-recorder"; | ||
| import { isLiveMode} from "@azure-tools/test-recorder"; | ||
| import type { PipelinePolicy } from "@azure/core-rest-pipeline"; | ||
| import { | ||
| createAppConfigurationClientForTests, | ||
| startRecorder, | ||
| } from "./utils/testHelpers.js"; | ||
| import { describe, it, assert, beforeEach, afterEach } from "vitest"; | ||
|
|
||
| describe("request url query parameters", () => { | ||
| let recorder: Recorder; | ||
|
|
||
| beforeEach(async (ctx) => { | ||
| recorder = await startRecorder(ctx); | ||
| }); | ||
|
|
||
| afterEach(async () => { | ||
| await recorder.stop(); | ||
| }); | ||
|
|
||
| describe("normalize query parameters", () => { | ||
| it("sort query params in alphabetical order", async () => { | ||
| const key = recorder.variable( | ||
| "sortQueryParams", | ||
| `sortQueryParams${Math.floor(Math.random() * 1000)}`, | ||
| ); | ||
|
|
||
| const { getCapturedUrl, client } = createClientWithUrlCapturePolicy(); | ||
|
|
||
| await client.addConfigurationSetting({ key, label: "dev", value: "some value" }); | ||
|
|
||
| const configurationSetting = await client.getConfigurationSetting( | ||
| { key, label: "dev" }, | ||
| { fields: ["key"] }, | ||
| ); | ||
|
|
||
| assert.ok( | ||
| getCapturedUrl(), | ||
| "Expected to have captured a request URL for getConfigurationSetting", | ||
| ); | ||
| // Regex enforces exact ordering of query params: $select (or %24select), api-version, label | ||
| let queryOrderRegex = /\?(?:\$|%24)select=key&api-version=[^&]+&label=dev$/; | ||
| assert.match( | ||
| getCapturedUrl()!, | ||
| queryOrderRegex, | ||
| `Query parameters not in expected order or values. URL: ${getCapturedUrl()}`, | ||
| ); | ||
|
|
||
| assert.equal(configurationSetting.key, key); | ||
|
|
||
| const listResult = client.listConfigurationSettings({ keyFilter: "*", labelFilter: "dev" }); | ||
|
|
||
| for await (const _ of listResult.byPage()) { | ||
| // do nothing, just drain the iterator | ||
| } | ||
|
|
||
| // Regex enforces exact ordering of query params: api-version, key, label | ||
| queryOrderRegex = /\?api-version=[^&]+&key=\*&label=dev$/; | ||
| console.log("Captured URL for listConfigurationSettings:", getCapturedUrl()); | ||
zhiyuanliang-ms marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| assert.match( | ||
| getCapturedUrl()!, | ||
| queryOrderRegex, | ||
| `Query parameters not in expected order or values. URL: ${getCapturedUrl()}`, | ||
| ); | ||
|
|
||
| await client.deleteConfigurationSetting({ key, label: "dev" }); | ||
| }); | ||
|
|
||
| // This occasionally hits 429 error (throttling) since we are making 100s of requests in the test to create, get and delete keys. | ||
| // To avoid hitting the service with too many requests, skipping the test in live. | ||
| // More details at https://github.com/Azure/azure-sdk-for-js/issues/16743 | ||
| // | ||
| // Remove the following line if you want to hit the live service. | ||
| it("sort query params in alphabetical order - continuation token", /* { skip: isLiveMode() }, */ async () => { | ||
| const key = recorder.variable( | ||
| "sortQueryParamsMultiplePages", | ||
| `sortQueryParamsMultiplePages${Math.floor(Math.random() * 1000)}`, | ||
| ); | ||
|
|
||
| const { getCapturedUrl, client } = createClientWithUrlCapturePolicy(); | ||
|
|
||
| // this number is arbitrarily chosen to match the size of a page + 1 | ||
| const expectedNumberOfLabels = 101; | ||
|
|
||
| let addSettingPromises = []; | ||
|
|
||
| for (let i = 0; i < expectedNumberOfLabels; i++) { | ||
| addSettingPromises.push( | ||
| client.addConfigurationSetting({ | ||
| key, | ||
| value: `the value for ${i}`, | ||
| label: i.toString(), | ||
| }), | ||
| ); | ||
|
|
||
| if (i !== 0 && i % 2 === 0) { | ||
| await Promise.all(addSettingPromises); | ||
| addSettingPromises = []; | ||
| } | ||
| } | ||
|
|
||
| const listResult = client.listConfigurationSettings({ | ||
| keyFilter: key, | ||
| }); | ||
|
|
||
| for await (const _ of listResult.byPage()) { | ||
| // do nothing, just drain the iterator | ||
| } | ||
|
|
||
| // Regex enforces exact ordering of query params for continuation page: after, api-version, key | ||
| // Note that only the request for the second page has the 'after' query param | ||
| const queryOrderRegex = new RegExp( | ||
| `\\?after=[^&]+&api-version=[^&]+&key=[^&]+$`, | ||
| ); | ||
|
|
||
| assert.match( | ||
| getCapturedUrl()!, | ||
| queryOrderRegex, | ||
| `Query parameters not in expected order or values. URL: ${getCapturedUrl()}`, | ||
| ); | ||
|
|
||
| for (let i = 0; i < expectedNumberOfLabels; i++) { | ||
| await client.deleteConfigurationSetting({ key, label: i.toString() }); | ||
| } | ||
| }); | ||
|
|
||
| function createClientWithUrlCapturePolicy(): { | ||
| getCapturedUrl: () => string | undefined; | ||
| client: AppConfigurationClient; | ||
| } { | ||
| let capturedUrl: string | undefined; | ||
| const urlCapturePolicy: PipelinePolicy = { | ||
| name: "UrlCapturePolicy", | ||
| async sendRequest(request, next) { | ||
| capturedUrl = request.url; | ||
| return next(request); | ||
| }, | ||
| }; | ||
|
|
||
| const client = createAppConfigurationClientForTests( | ||
| recorder.configureClientOptions({ | ||
| additionalPolicies: [ | ||
| { | ||
| policy: urlCapturePolicy, | ||
| position: "perRetry", | ||
| }, | ||
| ], | ||
| }), | ||
| ); | ||
| return { getCapturedUrl: () => capturedUrl, client }; | ||
| } | ||
| }); | ||
| }); | ||
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.