From 712ee6cdbce16f2b9ca1aacd6894b53a638653e2 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Thu, 20 Nov 2025 14:48:39 -0800 Subject: [PATCH 1/5] initial change --- .../azure-appconfiguration/CHANGELOG.md | 2 + .../_azure_appconfiguration_client.py | 7 +- .../appconfiguration/_query_param_policy.py | 60 ++++++ .../_azure_appconfiguration_client_async.py | 7 +- .../tests/test_query_param_policy.py | 180 ++++++++++++++++++ 5 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py create mode 100644 sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py diff --git a/sdk/appconfiguration/azure-appconfiguration/CHANGELOG.md b/sdk/appconfiguration/azure-appconfiguration/CHANGELOG.md index 80682b60a427..eea7e12f5ee6 100644 --- a/sdk/appconfiguration/azure-appconfiguration/CHANGELOG.md +++ b/sdk/appconfiguration/azure-appconfiguration/CHANGELOG.md @@ -4,6 +4,8 @@ ### Features Added +- Added query parameter normalization to support Azure Front Door as a CDN. Query parameter keys are now converted to lowercase and sorted alphabetically. + ### Breaking Changes ### Bugs Fixed diff --git a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py index 538b507114fc..a41658122ce2 100644 --- a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py +++ b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py @@ -16,6 +16,7 @@ from azure.core.rest import HttpRequest, HttpResponse from ._azure_appconfiguration_error import ResourceReadOnlyError from ._azure_appconfiguration_requests import AppConfigRequestsCredentialsPolicy +from ._query_param_policy import QueryParamPolicy from ._generated import AzureAppConfigurationClient as AzureAppConfigurationClientGenerated from ._generated.models import ( SnapshotStatus, @@ -65,6 +66,7 @@ def __init__(self, base_url: str, credential: TokenCredential, **kwargs: Any) -> credential_scopes = [f"{base_url.strip('/')}/.default"] self._sync_token_policy = SyncTokenPolicy() + self._query_param_policy = QueryParamPolicy() if isinstance(credential, AzureKeyCredential): id_credential = kwargs.pop("id_credential") @@ -85,7 +87,10 @@ def __init__(self, base_url: str, credential: TokenCredential, **kwargs: Any) -> ) # mypy doesn't compare the credential type hint with the API surface in patch.py self._impl = AzureAppConfigurationClientGenerated( - base_url, credential, per_call_policies=self._sync_token_policy, **kwargs # type: ignore[arg-type] + base_url, + credential, + per_call_policies=[self._query_param_policy, self._sync_token_policy], + **kwargs # type: ignore[arg-type] ) @classmethod diff --git a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py new file mode 100644 index 000000000000..2bd8271075d2 --- /dev/null +++ b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py @@ -0,0 +1,60 @@ +# ------------------------------------------------------------------------ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# ------------------------------------------------------------------------- + +from urllib.parse import urlparse, urlunparse, parse_qsl, urlencode +from azure.core.pipeline.policies import HTTPPolicy + + +class QueryParamPolicy(HTTPPolicy): + """Policy to normalize query parameters by converting keys to lowercase and sorting alphabetically. + + This policy ensures query parameter keys are converted to lowercase and sorted alphabetically + to support Azure Front Door as a CDN. + """ + + def send(self, request): + """Normalize query parameters before sending the request. + + :param request: The pipeline request object + :type request: ~azure.core.pipeline.PipelineRequest + :return: The pipeline response + :rtype: ~azure.core.pipeline.PipelineResponse + """ + try: + # Parse the current URL + parsed_url = urlparse(request.http_request.url) + + if parsed_url.query: + # Parse query parameters + query_params = parse_qsl(parsed_url.query, keep_blank_values=True) + + # Convert keys to lowercase and sort alphabetically + normalized_params = sorted( + [(key.lower(), value) for key, value in query_params], + key=lambda x: x[0] + ) + + # Rebuild the query string + new_query = urlencode(normalized_params) + + # Rebuild the URL with normalized query parameters + new_url = urlunparse(( + parsed_url.scheme, + parsed_url.netloc, + parsed_url.path, + parsed_url.params, + new_query, + parsed_url.fragment + )) + + # Update the request URL + request.http_request.url = new_url + except Exception: # pylint: disable=broad-except + # If URL normalization fails, continue with the original URL + # This ensures the policy doesn't break existing functionality + pass + + return self.next.send(request) diff --git a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py index d10e46951958..123c0c9bf6d9 100644 --- a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py +++ b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py @@ -19,6 +19,7 @@ from ._sync_token_async import AsyncSyncTokenPolicy from .._azure_appconfiguration_error import ResourceReadOnlyError from .._azure_appconfiguration_requests import AppConfigRequestsCredentialsPolicy +from .._query_param_policy import QueryParamPolicy from .._generated.aio import AzureAppConfigurationClient as AzureAppConfigurationClientGenerated from .._generated.models import ( SnapshotStatus, @@ -69,6 +70,7 @@ def __init__(self, base_url: str, credential: AsyncTokenCredential, **kwargs: An credential_scopes = [f"{base_url.strip('/')}/.default"] self._sync_token_policy = AsyncSyncTokenPolicy() + self._query_param_policy = QueryParamPolicy() if isinstance(credential, AzureKeyCredential): id_credential = kwargs.pop("id_credential") @@ -89,7 +91,10 @@ def __init__(self, base_url: str, credential: AsyncTokenCredential, **kwargs: An ) # mypy doesn't compare the credential type hint with the API surface in patch.py self._impl = AzureAppConfigurationClientGenerated( - base_url, credential, per_call_policies=self._sync_token_policy, **kwargs # type: ignore[arg-type] + base_url, + credential, + per_call_policies=[self._query_param_policy, self._sync_token_policy], + **kwargs # type: ignore[arg-type] ) @classmethod diff --git a/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py b/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py new file mode 100644 index 000000000000..54ac1dd34ac5 --- /dev/null +++ b/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py @@ -0,0 +1,180 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# -------------------------------------------------------------------------- + +from azure.core.pipeline.transport import HttpRequest +from azure.core.pipeline import PipelineRequest +from azure.appconfiguration._query_param_policy import QueryParamPolicy + + +def test_query_parameters_are_sorted_alphabetically(): + """Test that query parameters are sorted alphabetically.""" + original_url = "https://test.azconfig.io/kv?zebra=value1&alpha=value2&beta=value3" + expected_url = "https://test.azconfig.io/kv?alpha=value2&beta=value3&zebra=value1" + + request = HttpRequest("GET", original_url) + pipeline_request = PipelineRequest(request, None) + + query_param_policy = QueryParamPolicy() + + # Create a mock next policy that captures the request + class MockNext: + def __init__(self): + self.captured_url = None + + def send(self, request): + self.captured_url = request.http_request.url + return None + + mock_next = MockNext() + query_param_policy.next = mock_next + + query_param_policy.send(pipeline_request) + + assert mock_next.captured_url == expected_url, \ + f"Query parameters should be sorted alphabetically. Expected: {expected_url}, Got: {mock_next.captured_url}" + + +def test_query_parameter_keys_are_converted_to_lowercase(): + """Test that query parameter keys are converted to lowercase.""" + original_url = "https://test.azconfig.io/kv?SELECT=field1&FILTER=condition&orderBy=field2" + expected_url = "https://test.azconfig.io/kv?filter=condition&orderby=field2&select=field1" + + request = HttpRequest("GET", original_url) + pipeline_request = PipelineRequest(request, None) + + query_param_policy = QueryParamPolicy() + + # Create a mock next policy + class MockNext: + def __init__(self): + self.captured_url = None + + def send(self, request): + self.captured_url = request.http_request.url + return None + + mock_next = MockNext() + query_param_policy.next = mock_next + + query_param_policy.send(pipeline_request) + + assert mock_next.captured_url == expected_url, \ + f"Query parameter keys should be lowercase and sorted. Expected: {expected_url}, Got: {mock_next.captured_url}" + + +def test_query_parameters_with_multiple_values(): + """Test that query parameters with multiple values are handled correctly.""" + original_url = "https://test.azconfig.io/kv?key=value1&key=value2&alpha=test" + expected_url = "https://test.azconfig.io/kv?alpha=test&key=value1&key=value2" + + request = HttpRequest("GET", original_url) + pipeline_request = PipelineRequest(request, None) + + query_param_policy = QueryParamPolicy() + + # Create a mock next policy + class MockNext: + def __init__(self): + self.captured_url = None + + def send(self, request): + self.captured_url = request.http_request.url + return None + + mock_next = MockNext() + query_param_policy.next = mock_next + + query_param_policy.send(pipeline_request) + + assert mock_next.captured_url == expected_url, \ + f"Multiple values for same key should be preserved. Expected: {expected_url}, Got: {mock_next.captured_url}" + + +def test_query_parameters_with_special_characters(): + """Test that query parameters with special characters are preserved.""" + original_url = "https://test.azconfig.io/kv?filter=name%20eq%20%27test%27&select=*" + expected_url = "https://test.azconfig.io/kv?filter=name+eq+%27test%27&select=%2A" + + request = HttpRequest("GET", original_url) + pipeline_request = PipelineRequest(request, None) + + query_param_policy = QueryParamPolicy() + + # Create a mock next policy + class MockNext: + def __init__(self): + self.captured_url = None + + def send(self, request): + self.captured_url = request.http_request.url + return None + + mock_next = MockNext() + query_param_policy.next = mock_next + + query_param_policy.send(pipeline_request) + + # Note: URL encoding may change the format slightly, but the query params should still be sorted + assert "filter=" in mock_next.captured_url + assert "select=" in mock_next.captured_url + # The params should be in alphabetical order + filter_pos = mock_next.captured_url.index("filter=") + select_pos = mock_next.captured_url.index("select=") + assert filter_pos < select_pos, "Parameters should be sorted alphabetically" + + +def test_no_query_parameters(): + """Test that URLs without query parameters are not modified.""" + original_url = "https://test.azconfig.io/kv" + expected_url = "https://test.azconfig.io/kv" + + request = HttpRequest("GET", original_url) + pipeline_request = PipelineRequest(request, None) + + query_param_policy = QueryParamPolicy() + + # Create a mock next policy + class MockNext: + def __init__(self): + self.captured_url = None + + def send(self, request): + self.captured_url = request.http_request.url + return None + + mock_next = MockNext() + query_param_policy.next = mock_next + + query_param_policy.send(pipeline_request) + + assert mock_next.captured_url == expected_url + + +def test_empty_query_parameter_values(): + """Test that empty query parameter values are preserved.""" + original_url = "https://test.azconfig.io/kv?zebra=&alpha=value&beta=" + expected_url = "https://test.azconfig.io/kv?alpha=value&beta=&zebra=" + + request = HttpRequest("GET", original_url) + pipeline_request = PipelineRequest(request, None) + + query_param_policy = QueryParamPolicy() + + # Create a mock next policy + class MockNext: + def __init__(self): + self.captured_url = None + + def send(self, request): + self.captured_url = request.http_request.url + return None + + mock_next = MockNext() + query_param_policy.next = mock_next + + query_param_policy.send(pipeline_request) + + assert mock_next.captured_url == expected_url From ab4518566b9fd43bf6bfe55339fe0799b28632f7 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 21 Nov 2025 12:55:49 -0800 Subject: [PATCH 2/5] fixes + tests --- .../_azure_appconfiguration_client.py | 2 +- .../appconfiguration/_query_param_policy.py | 70 ++-- .../_azure_appconfiguration_client_async.py | 2 +- .../tests/test_query_param_policy.py | 323 +++++++++++------- 4 files changed, 254 insertions(+), 143 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py index a41658122ce2..eb9b0fb1f0d1 100644 --- a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py +++ b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py @@ -90,7 +90,7 @@ def __init__(self, base_url: str, credential: TokenCredential, **kwargs: Any) -> base_url, credential, per_call_policies=[self._query_param_policy, self._sync_token_policy], - **kwargs # type: ignore[arg-type] + **kwargs, # type: ignore[arg-type] ) @classmethod diff --git a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py index 2bd8271075d2..a1865ae0da50 100644 --- a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py +++ b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py @@ -3,21 +3,21 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # ------------------------------------------------------------------------- - -from urllib.parse import urlparse, urlunparse, parse_qsl, urlencode +from collections import defaultdict +from urllib.parse import urlparse, urlunparse, parse_qsl, urlencode, quote from azure.core.pipeline.policies import HTTPPolicy class QueryParamPolicy(HTTPPolicy): """Policy to normalize query parameters by converting keys to lowercase and sorting alphabetically. - + This policy ensures query parameter keys are converted to lowercase and sorted alphabetically to support Azure Front Door as a CDN. """ def send(self, request): """Normalize query parameters before sending the request. - + :param request: The pipeline request object :type request: ~azure.core.pipeline.PipelineRequest :return: The pipeline response @@ -26,35 +26,55 @@ def send(self, request): try: # Parse the current URL parsed_url = urlparse(request.http_request.url) - + if parsed_url.query: # Parse query parameters query_params = parse_qsl(parsed_url.query, keep_blank_values=True) - - # Convert keys to lowercase and sort alphabetically - normalized_params = sorted( - [(key.lower(), value) for key, value in query_params], - key=lambda x: x[0] - ) - - # Rebuild the query string - new_query = urlencode(normalized_params) - + + # Convert keys to lowercase, drop empty keys + lowered_params = [(key.lower(), value) for key, value in query_params if key] + + # Sort all params by key, and for duplicate keys, non-empty values lexicographically, empty values last + + grouped = defaultdict(list) + for k, v in lowered_params: + grouped[k].append(v) + normalized_params = [] + for k in sorted(grouped.keys()): + values = grouped[k] + if len(values) > 1: + # Empty values last, space values second to last, other non-empty values sorted + # lexicographically first + def sort_key(v): + if v == "": + return (2, "") # empty string last + if v == " ": + return (1, v) # space second to last + return (0, v) # other non-empty values first + + values = sorted(values, key=sort_key) + normalized_params.extend([(k, v) for v in values]) + + # Rebuild the query string, encoding spaces as %20 instead of + + new_query = urlencode(normalized_params, quote_via=quote) + # Rebuild the URL with normalized query parameters - new_url = urlunparse(( - parsed_url.scheme, - parsed_url.netloc, - parsed_url.path, - parsed_url.params, - new_query, - parsed_url.fragment - )) - + new_url = urlunparse( + ( + parsed_url.scheme, + parsed_url.netloc, + parsed_url.path, + parsed_url.params, + new_query, + parsed_url.fragment, + ) + ) + # Update the request URL request.http_request.url = new_url except Exception: # pylint: disable=broad-except # If URL normalization fails, continue with the original URL # This ensures the policy doesn't break existing functionality pass - + return self.next.send(request) diff --git a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py index 123c0c9bf6d9..3634d51694f9 100644 --- a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py +++ b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py @@ -94,7 +94,7 @@ def __init__(self, base_url: str, credential: AsyncTokenCredential, **kwargs: An base_url, credential, per_call_policies=[self._query_param_policy, self._sync_token_policy], - **kwargs # type: ignore[arg-type] + **kwargs, # type: ignore[arg-type] ) @classmethod diff --git a/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py b/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py index 54ac1dd34ac5..634c6af3c4a5 100644 --- a/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py +++ b/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py @@ -4,120 +4,73 @@ # license information. # -------------------------------------------------------------------------- + from azure.core.pipeline.transport import HttpRequest from azure.core.pipeline import PipelineRequest from azure.appconfiguration._query_param_policy import QueryParamPolicy +import pytest + +TEST_URL = "https://example.com" def test_query_parameters_are_sorted_alphabetically(): """Test that query parameters are sorted alphabetically.""" - original_url = "https://test.azconfig.io/kv?zebra=value1&alpha=value2&beta=value3" - expected_url = "https://test.azconfig.io/kv?alpha=value2&beta=value3&zebra=value1" - - request = HttpRequest("GET", original_url) - pipeline_request = PipelineRequest(request, None) - - query_param_policy = QueryParamPolicy() - - # Create a mock next policy that captures the request - class MockNext: - def __init__(self): - self.captured_url = None - - def send(self, request): - self.captured_url = request.http_request.url - return None - - mock_next = MockNext() - query_param_policy.next = mock_next - - query_param_policy.send(pipeline_request) - - assert mock_next.captured_url == expected_url, \ - f"Query parameters should be sorted alphabetically. Expected: {expected_url}, Got: {mock_next.captured_url}" + original_url = "?zebra=value1&alpha=value2&beta=value3" + expected_url = "?alpha=value2&beta=value3&zebra=value1" + + run_query_param_policy_test(original_url, expected_url) def test_query_parameter_keys_are_converted_to_lowercase(): """Test that query parameter keys are converted to lowercase.""" - original_url = "https://test.azconfig.io/kv?SELECT=field1&FILTER=condition&orderBy=field2" - expected_url = "https://test.azconfig.io/kv?filter=condition&orderby=field2&select=field1" - - request = HttpRequest("GET", original_url) - pipeline_request = PipelineRequest(request, None) - - query_param_policy = QueryParamPolicy() - - # Create a mock next policy - class MockNext: - def __init__(self): - self.captured_url = None - - def send(self, request): - self.captured_url = request.http_request.url - return None - - mock_next = MockNext() - query_param_policy.next = mock_next - - query_param_policy.send(pipeline_request) - - assert mock_next.captured_url == expected_url, \ - f"Query parameter keys should be lowercase and sorted. Expected: {expected_url}, Got: {mock_next.captured_url}" + original_url = "?SELECT=field1&FILTER=condition&orderBy=field2" + expected_url = "?filter=condition&orderby=field2&select=field1" + + run_query_param_policy_test(original_url, expected_url) + + +def test_query_parameter_o_data_parameters(): + """Test that OData query parameters are handled correctly.""" + original_url = "?$Select=name%2Cvalue&$Filter=startsWith%28key%2C%27test%27%29&api-version=1.0" + expected_url = "?%24filter=startsWith%28key%2C%27test%27%29&%24select=name%2Cvalue&api-version=1.0" + + run_query_param_policy_test(original_url, expected_url) def test_query_parameters_with_multiple_values(): """Test that query parameters with multiple values are handled correctly.""" - original_url = "https://test.azconfig.io/kv?key=value1&key=value2&alpha=test" - expected_url = "https://test.azconfig.io/kv?alpha=test&key=value1&key=value2" - - request = HttpRequest("GET", original_url) - pipeline_request = PipelineRequest(request, None) - - query_param_policy = QueryParamPolicy() - - # Create a mock next policy - class MockNext: - def __init__(self): - self.captured_url = None - - def send(self, request): - self.captured_url = request.http_request.url - return None - - mock_next = MockNext() - query_param_policy.next = mock_next - - query_param_policy.send(pipeline_request) - - assert mock_next.captured_url == expected_url, \ - f"Multiple values for same key should be preserved. Expected: {expected_url}, Got: {mock_next.captured_url}" + original_url = "?key=value1&key=value2&alpha=test" + expected_url = "?alpha=test&key=value1&key=value2" + + run_query_param_policy_test(original_url, expected_url) def test_query_parameters_with_special_characters(): """Test that query parameters with special characters are preserved.""" - original_url = "https://test.azconfig.io/kv?filter=name%20eq%20%27test%27&select=*" - expected_url = "https://test.azconfig.io/kv?filter=name+eq+%27test%27&select=%2A" - + original_url = "?filter=name%20eq%20%27test%27&select=*" + expected_url = "?filter=name%20eq%20%27test%27&select=%2A" + request = HttpRequest("GET", original_url) pipeline_request = PipelineRequest(request, None) - + query_param_policy = QueryParamPolicy() - + # Create a mock next policy class MockNext: def __init__(self): self.captured_url = None - + def send(self, request): self.captured_url = request.http_request.url return None - + mock_next = MockNext() query_param_policy.next = mock_next - + query_param_policy.send(pipeline_request) - + # Note: URL encoding may change the format slightly, but the query params should still be sorted + assert mock_next.captured_url is not None, "No URL was captured by the mock policy." assert "filter=" in mock_next.captured_url assert "select=" in mock_next.captured_url # The params should be in alphabetical order @@ -128,53 +81,191 @@ def send(self, request): def test_no_query_parameters(): """Test that URLs without query parameters are not modified.""" - original_url = "https://test.azconfig.io/kv" - expected_url = "https://test.azconfig.io/kv" - - request = HttpRequest("GET", original_url) - pipeline_request = PipelineRequest(request, None) - - query_param_policy = QueryParamPolicy() - - # Create a mock next policy - class MockNext: - def __init__(self): - self.captured_url = None - - def send(self, request): - self.captured_url = request.http_request.url - return None - - mock_next = MockNext() - query_param_policy.next = mock_next - - query_param_policy.send(pipeline_request) - - assert mock_next.captured_url == expected_url + original_url = "" + expected_url = "" + + run_query_param_policy_test(original_url, expected_url) def test_empty_query_parameter_values(): """Test that empty query parameter values are preserved.""" - original_url = "https://test.azconfig.io/kv?zebra=&alpha=value&beta=" - expected_url = "https://test.azconfig.io/kv?alpha=value&beta=&zebra=" - - request = HttpRequest("GET", original_url) + original_url = "?zebra=&alpha=value&beta=" + expected_url = "?alpha=value&beta=&zebra=" + + run_query_param_policy_test(original_url, expected_url) + + +def test_query_parameter_with_key_only(): + """Test that query parameters with only a key and no value are preserved and sorted.""" + original_url = "?zebra&alpha=value&beta" + expected_url = "?alpha=value&beta=&zebra=" + + run_query_param_policy_test(original_url, expected_url) + + +def test_query_parameter_with_empty_key_is_first(): + """Test that query parameters with empty keys are first.""" + original_url = "?alpha=value2&=value1" + expected_url = "?alpha=value2" + + run_query_param_policy_test(original_url, expected_url) + + +@pytest.mark.parametrize( + "original_url,expected_url", + [ + ("?key=%20value%20&alpha=%20%20", "?alpha=%20%20&key=%20value%20"), + ("?key=hello%20world&alpha=foo%20bar", "?alpha=foo%20bar&key=hello%20world"), + ], +) +def test_query_param_policy_whitespace(original_url, expected_url): + run_query_param_policy_test(original_url, expected_url) + + +# Unicode and encoded values +@pytest.mark.parametrize( + "original_url,expected_url", + [ + ("?key=%E2%9C%93&alpha=%C3%A9", "?alpha=%C3%A9&key=%E2%9C%93"), + ("?key=val1&key=\u2713", "?key=val1&key=%E2%9C%93"), + ("?key=val1&key=é", "?key=val1&key=%C3%A9"), + ], +) +def test_query_param_policy_unicode(original_url, expected_url): + run_query_param_policy_test(original_url, expected_url) + + +# Multiple values for same key +@pytest.mark.parametrize( + "original_url,expected_url", + [ + ("?key=val1&key=val2&key=val3", "?key=val1&key=val2&key=val3"), + ], +) +def test_query_param_policy_multiple_values(original_url, expected_url): + run_query_param_policy_test(original_url, expected_url) + + +def test_query_parameter_values_are_preserved(): + """Test that query parameter values are preserved correctly.""" + original_url = "?key1=Value%20With%20Spaces&key2=SimpleValue&key3=" + expected_url = "?key1=Value%20With%20Spaces&key2=SimpleValue&key3=" + + run_query_param_policy_test(original_url, expected_url) + + +def test_query_parameters_with_same_key_are_preserved(): + """Test that query parameters with the same key are preserved in order.""" + original_url = "?key=val1&key=val2&key=val3" + expected_url = "?key=val1&key=val2&key=val3" + + run_query_param_policy_test(original_url, expected_url) + + +# Empty and malformed cases +@pytest.mark.parametrize( + "original_url,expected_url", + [ + ("?key=val1&&key=val2", "?key=val1&key=val2"), + ("?key=val1&=val2", "?key=val1"), + ("?key=val1&key=", "?key=val1&key="), + ("?key=val1&key", "?key=val1&key="), + ("?key=val1&key= ", "?key=val1&key=%20"), + ("?key=val1&key=%20", "?key=val1&key=%20"), + ("?key=val1&key=%E2%9C%93", "?key=val1&key=%E2%9C%93"), + ], +) +def test_query_param_policy_empty_and_malformed(original_url, expected_url): + run_query_param_policy_test(original_url, expected_url) + + +def test_comprehensive_query_parameter_normalization(): + original_url = ( + "?$TOP=10&API-Version=2023-10-01&$select=key,value&label=prod&$filter=startsWith(key,'app')&maxItems=100" + ) + expected_url = "?%24filter=startsWith%28key%2C%27app%27%29&%24select=key%2Cvalue&%24top=10&api-version=2023-10-01&label=prod&maxitems=100" + + run_query_param_policy_test(original_url, expected_url) + + +def test_multiple_tags_parameters(): + """Test that multiple tags parameters are handled correctly.""" + original_url = "?api-version=2023-11-01&key=*&label=dev&tags=environment%3Ddev&tags=team%3Dfrontend" + expected_url = "?api-version=2023-11-01&key=%2A&label=dev&tags=environment%3Ddev&tags=team%3Dfrontend" + + run_query_param_policy_test(original_url, expected_url) + + +def test_tags_parameters_with_complex_values(): + """Test that tags parameters with complex values are handled correctly.""" + original_url = "?tags=environment%3Dproduction&tags=team%3Dbackend&api-version=2023-11-01" + expected_url = "?api-version=2023-11-01&tags=environment%3Dproduction&tags=team%3Dbackend" + + run_query_param_policy_test(original_url, expected_url) + + +def test_tags_parameters_mixed_with_other_parameters(): + """Test that tags parameters mixed with other parameters are handled correctly.""" + original_url = "?$select=key,value&tags=feature%3Dauth&label=*&api-version=2023-11-01&$filter=startsWith(key,'app')&tags=env%3Dtest" + expected_url = "?%24filter=startsWith%28key%2C%27app%27%29&%24select=key%2Cvalue&api-version=2023-11-01&label=%2A&tags=env%3Dtest&tags=feature%3Dauth" + + run_query_param_policy_test(original_url, expected_url) + + +def test_tags_parameters_with_special_characters(): + """Test that tags parameters with special characters are handled correctly.""" + original_url = "?TAGS=Priority%3DHigh&api-version=2023-11-01&Tags=Status%3DActive" + expected_url = "?api-version=2023-11-01&tags=Priority%3DHigh&tags=Status%3DActive" + + run_query_param_policy_test(original_url, expected_url) + + +def test_key_and_label_filters_with_ampersand_character(): + """Test that key and label filters with ampersand characters are handled correctly.""" + original_url = "?key=app%26config&label=prod%26test&api-version=2023-11-01" + expected_url = "?api-version=2023-11-01&key=app%26config&label=prod%26test" + + run_query_param_policy_test(original_url, expected_url) + + +def test_key_and_label_filters_with_space_character(): + """Test that key and label filters with space characters are handled correctly.""" + original_url = "?key=app%20config&label=dev%20environment&api-version=2023-11-01" + expected_url = "?api-version=2023-11-01&key=app%20config&label=dev%20environment" + + run_query_param_policy_test(original_url, expected_url) + + +def test_key_and_label_filters_with_hash_character(): + """Test that key and label filters with hash characters are handled correctly.""" + original_url = "?key=app%23config&label=version%23v1&api-version=2023-11-01" + expected_url = "?api-version=2023-11-01&key=app%23config&label=version%23v1" + + run_query_param_policy_test(original_url, expected_url) + + +def test_key_and_label_filters_with_mixed_special_characters(): + """Test that key and label filters with mixed special characters are handled correctly.""" + original_url = "?key=app%26config%20test%23v1&label=prod%20%26%20test%23env&api-version=2023-11-01" + expected_url = "?api-version=2023-11-01&key=app%26config%20test%23v1&label=prod%20%26%20test%23env" + + run_query_param_policy_test(original_url, expected_url) + + +def run_query_param_policy_test(original_url, expected_url): + request = HttpRequest("GET", TEST_URL + original_url) pipeline_request = PipelineRequest(request, None) - query_param_policy = QueryParamPolicy() - - # Create a mock next policy + class MockNext: def __init__(self): self.captured_url = None - + def send(self, request): self.captured_url = request.http_request.url return None - + mock_next = MockNext() query_param_policy.next = mock_next - query_param_policy.send(pipeline_request) - - assert mock_next.captured_url == expected_url + assert mock_next.captured_url == TEST_URL + expected_url From 6376c03a0cdc885802dc140440055105e06904cf Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 21 Nov 2025 13:09:54 -0800 Subject: [PATCH 3/5] Update _query_param_policy.py --- .../appconfiguration/_query_param_policy.py | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py index a1865ae0da50..4991267d2611 100644 --- a/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py +++ b/sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_query_param_policy.py @@ -45,14 +45,7 @@ def send(self, request): if len(values) > 1: # Empty values last, space values second to last, other non-empty values sorted # lexicographically first - def sort_key(v): - if v == "": - return (2, "") # empty string last - if v == " ": - return (1, v) # space second to last - return (0, v) # other non-empty values first - - values = sorted(values, key=sort_key) + values = sorted(values, key=self.__sort_key) normalized_params.extend([(k, v) for v in values]) # Rebuild the query string, encoding spaces as %20 instead of + @@ -72,9 +65,17 @@ def sort_key(v): # Update the request URL request.http_request.url = new_url - except Exception: # pylint: disable=broad-except - # If URL normalization fails, continue with the original URL + except (ValueError, TypeError): + # If URL normalization fails due to parsing or encoding errors, continue with the original URL # This ensures the policy doesn't break existing functionality pass return self.next.send(request) + + @staticmethod + def __sort_key(v: str): + if v == "": + return (2, "") # empty string last + if v == " ": + return (1, v) # space second to last + return (0, v) # other non-empty values first \ No newline at end of file From 517d073c06a2edffebeb0c0a32be7e9647008296 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 21 Nov 2025 13:16:25 -0800 Subject: [PATCH 4/5] Update assets.json --- sdk/appconfiguration/azure-appconfiguration/assets.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/appconfiguration/azure-appconfiguration/assets.json b/sdk/appconfiguration/azure-appconfiguration/assets.json index e706599ea118..313681ddacd7 100644 --- a/sdk/appconfiguration/azure-appconfiguration/assets.json +++ b/sdk/appconfiguration/azure-appconfiguration/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "python", "TagPrefix": "python/appconfiguration/azure-appconfiguration", - "Tag": "python/appconfiguration/azure-appconfiguration_710e235678" + "Tag": "python/appconfiguration/azure-appconfiguration_91b992c0d1" } From 76465baac1bff205fbacf574bca8e2b7a83c0dec Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Fri, 21 Nov 2025 16:48:52 -0800 Subject: [PATCH 5/5] Update test_query_param_policy.py --- .../azure-appconfiguration/tests/test_query_param_policy.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py b/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py index 634c6af3c4a5..7f19376b58ec 100644 --- a/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py +++ b/sdk/appconfiguration/azure-appconfiguration/tests/test_query_param_policy.py @@ -190,8 +190,9 @@ def test_comprehensive_query_parameter_normalization(): def test_multiple_tags_parameters(): """Test that multiple tags parameters are handled correctly.""" - original_url = "?api-version=2023-11-01&key=*&label=dev&tags=environment%3Ddev&tags=team%3Dfrontend" - expected_url = "?api-version=2023-11-01&key=%2A&label=dev&tags=environment%3Ddev&tags=team%3Dfrontend" + # cspell thinks the url encoding is part of the word + original_url = "?api-version=2023-11-01&key=*&label=dev&tags=environment%3Ddev&tags=team%3Dfrontend" # cspell:ignore + expected_url = "?api-version=2023-11-01&key=%2A&label=dev&tags=environment%3Ddev&tags=team%3Dfrontend" # cspell:ignore run_query_param_policy_test(original_url, expected_url)