fix(http): improve parameters name decoding precision and span lifecycle

packages/http/httpx/kiota_http/middleware/parameters_name_decoding_handler.py

@@ -35,29 +35,34 @@
         Returns:
             Response: The response object.
         """
-        current_options = self._get_current_options(request)
+        if request is None:
+            raise TypeError("request cannot be null")
+
         span = self._create_observability_span(request, "ParametersNameDecodingHandler_send")
-        if current_options.enabled:
-            span.set_attribute(PARAMETERS_NAME_DECODING_KEY, current_options.enabled)
-        span.end()
-
-        query_params = request.url.query.decode('utf-8')
-        if all(
-            [
-                current_options,
-                current_options.enabled,
-                "%" in query_params,
-                current_options.characters_to_decode,
-            ]
-        ):
-            original_url: str = str(request.url)
-            decoded_query_parameters_string = self.decode_uri_encoded_string(
-                query_params, current_options.characters_to_decode
-            )
-            new_url = original_url.replace(query_params, decoded_query_parameters_string)
-            request.url = httpx.URL(new_url)
-        response = await super().send(request, transport)
-        return response
+        try:
+            current_options = self._get_current_options(request)
+            if current_options.enabled:
+                span.set_attribute(PARAMETERS_NAME_DECODING_KEY, current_options.enabled)
+
+            query_params = request.url.query.decode("utf-8")
+            if all(
+                [
+                    current_options,
+                    current_options.enabled,
+                    "%" in query_params,
+                    current_options.characters_to_decode,
+                ]
+            ):
+                decoded_query_parameters_string = self.decode_uri_encoded_string(
+                    query_params, current_options.characters_to_decode
+                )
+                request.url = request.url.copy_with(
+                    query=decoded_query_parameters_string.encode("utf-8")
+                )
+            response = await super().send(request, transport)
+            return response
+        finally:
+            span.end()
 
     def _get_current_options(self, request: httpx.Request) -> ParametersNameDecodingHandlerOption:
         """Returns the options to use for the request.Overrides default options if
@@ -77,22 +82,35 @@
             return current_options
         return self.options
 
     def decode_uri_encoded_string(self, original: str, characters_to_decode: list[str]) -> str:
-        """Decodes a uri encoded url string"""
+        """Decodes a uri encoded query string, decoding characters only in parameter names."""
         if not original or not characters_to_decode:
             return original
-        symbols_to_replace = [
-            (f"%{ord(x):X}", x) for x in characters_to_decode if f"%{ord(x):X}" in original
-        ]
-
-        encoded_parameter_values = [
-            part.split('=')[0] for part in original.split('&') if '%' in part
-        ]
-
-        for parameter in encoded_parameter_values:
-            for symbol_to_replace in symbols_to_replace:
-                if symbol_to_replace[0] in parameter:
-                    new_parameter = parameter.replace(symbol_to_replace[0], symbol_to_replace[1])
-                    original = original.replace(parameter, new_parameter)
 
-        return original
+        symbols_to_replace: list[tuple[str, str]] = []
+        for x in characters_to_decode:
+            code_upper = f"%{ord(x):02X}"
+            code_lower = f"%{ord(x):02x}"
+            symbols_to_replace.append((code_upper, x))
+            if code_upper != code_lower:
+                symbols_to_replace.append((code_lower, x))
+
+        decoded_parts: list[str] = []
+        for part in original.split("&"):
+            if not part:
+                decoded_parts.append(part)
+                continue
+            if "=" in part:
+                param_name, param_value = part.split("=", 1)
+                for code, symbol in symbols_to_replace:
+                    if code in param_name:
+                        param_name = param_name.replace(code, symbol)
+                decoded_parts.append(f"{param_name}={param_value}")
+            else:
+                param_name = part
+                for code, symbol in symbols_to_replace:
+                    if code in param_name:
+                        param_name = param_name.replace(code, symbol)
+                decoded_parts.append(param_name)
+
+        return "&".join(decoded_parts)

packages/http/httpx/tests/middleware_tests/test_parameters_name_decoding_handler.py

@@ -1,12 +1,28 @@
-import httpx
 import pytest
 
-from kiota_http.middleware import ParametersNameDecodingHandler
+import httpx
 from kiota_http.middleware.options import ParametersNameDecodingHandlerOption
+from kiota_http.middleware.parameters_name_decoding_handler import (
+    PARAMETERS_NAME_DECODING_KEY,
+    ParametersNameDecodingHandler,
+)
 
 OPTION_KEY = "ParametersNameDecodingHandlerOption"
 
 
+class MockSpan:
+
+    def __init__(self):
+        self.end_called = 0
+        self.attributes = {}
+
+    def set_attribute(self, key, value):
+        self.attributes[key] = value
+
+    def end(self):
+        self.end_called += 1
+
+
 def test_no_config():
     """
     Test that default values are used if no custom confguration is passed
@@ -80,3 +96,101 @@ def request_handler(request: httpx.Request):
 def test_default_characters_to_decode_are_not_shared():
     ParametersNameDecodingHandlerOption().characters_to_decode.append("+")
     assert ParametersNameDecodingHandlerOption().characters_to_decode == [".", "-", "~", "$"]
+
+
+@pytest.mark.asyncio
+async def test_send_request_none_raises_type_error():
+    handler = ParametersNameDecodingHandler()
+    with pytest.raises(TypeError, match="request cannot be null"):
+        await handler.send(None, None)
+
+
+@pytest.mark.asyncio
+async def test_decoding_preserves_values_matching_parameter_name():
+
+    def request_handler(request: httpx.Request):
+        return httpx.Response(200, json={"text": "Hello, world!"})
+
+    handler = ParametersNameDecodingHandler()
+    request = httpx.Request('GET', "https://example.com/api?query=%24select&%24select=name")
+    mock_transport = httpx.MockTransport(request_handler)
+    resp = await handler.send(request, mock_transport)
+    assert str(resp.request.url) == "https://example.com/api?query=%24select&$select=name"
+
+
+@pytest.mark.asyncio
+async def test_decodes_lowercase_percent_encoding():
+
+    def request_handler(request: httpx.Request):
+        return httpx.Response(200, json={"text": "Hello, world!"})
+
+    handler = ParametersNameDecodingHandler()
+    request = httpx.Request(
+        'GET', "https://example.com/api?api%2dversion=1&param%2evalue=2&flag%7etest=3"
+    )
+    mock_transport = httpx.MockTransport(request_handler)
+    resp = await handler.send(request, mock_transport)
+    assert str(
+        resp.request.url
+    ) == "https://example.com/api?api-version=1&param.value=2&flag~test=3"
+
+
+@pytest.mark.asyncio
+async def test_decoding_preserves_url_path():
+
+    def request_handler(request: httpx.Request):
+        return httpx.Response(200, json={"text": "Hello, world!"})
+
+    handler = ParametersNameDecodingHandler()
+    request = httpx.Request('GET', "https://example.com/api/%24select=name/items?%24select=name")
+    mock_transport = httpx.MockTransport(request_handler)
+    resp = await handler.send(request, mock_transport)
+    assert str(resp.request.url) == "https://example.com/api/%24select=name/items?$select=name"
+    assert b"/api/%24select=name/items" in resp.request.url.raw_path
+    assert resp.request.url.query == b"$select=name"
+
+
+@pytest.mark.asyncio
+async def test_span_lifecycle_active_during_send_and_ends_on_success(monkeypatch):
+    mock_span = MockSpan()
+    handler = ParametersNameDecodingHandler()
+    monkeypatch.setattr(handler, "_create_observability_span", lambda *_: mock_span)
+
+    transport_called = False
+
+    def request_handler(request: httpx.Request):
+        nonlocal transport_called
+        transport_called = True
+        assert mock_span.end_called == 0
+        return httpx.Response(200, json={"text": "Hello, world!"})
+
+    request = httpx.Request("GET", "https://example.com/api?%24select=name")
+    resp = await handler.send(request, httpx.MockTransport(request_handler))
+
+    assert transport_called is True
+    assert mock_span.end_called == 1
+    assert mock_span.attributes.get(PARAMETERS_NAME_DECODING_KEY) is True
+    assert str(resp.request.url) == "https://example.com/api?$select=name"
+
+
+@pytest.mark.asyncio
+async def test_span_lifecycle_ends_and_exception_propagates_on_transport_failure(monkeypatch):
+    mock_span = MockSpan()
+    handler = ParametersNameDecodingHandler()
+    monkeypatch.setattr(handler, "_create_observability_span", lambda *_: mock_span)
+
+    transport_called = False
+
+    def request_handler(request: httpx.Request):
+        nonlocal transport_called
+        transport_called = True
+        assert mock_span.end_called == 0
+        raise httpx.ReadError("transport connection interrupted")
+
+    request = httpx.Request("GET", "https://example.com/api?%24select=name")
+    transport = httpx.MockTransport(request_handler)
+    with pytest.raises(httpx.ReadError, match="transport connection interrupted"):
+        await handler.send(request, transport)
+
+    assert transport_called is True
+    assert mock_span.end_called == 1