diff --git a/plugins/communication_protocols/http/src/utcp_http/_security.py b/plugins/communication_protocols/http/src/utcp_http/_security.py index 0cc5791..fa70578 100644 --- a/plugins/communication_protocols/http/src/utcp_http/_security.py +++ b/plugins/communication_protocols/http/src/utcp_http/_security.py @@ -9,6 +9,7 @@ from __future__ import annotations +import json import re import socket from contextlib import asynccontextmanager @@ -648,3 +649,67 @@ def reject_remote_loopback_tool_urls( "allowed to route the client-credentials POST at the agent's " "own loopback interface." ) + + +def encode_query_params(params: Dict[str, Any]) -> Dict[str, Any]: + """Encode query parameter values for HTTP/SSE/Streamable HTTP requests. + + - Drops None values so optional unset arguments are omitted from query strings. + - Preserves primitive values (str, int, float). + - Serializes booleans as 'true'/'false'. + - Serializes sets into JSON arrays with sorted elements for deterministic output. + - Serializes dictionaries as compact JSON objects with string-normalized keys. + - Preserves top-level lists and tuples so aiohttp generates repeated query + parameters (?key=v1&key=v2), while recursively encoding nested sequences + or objects within list items into JSON (omitting None items). + - Non-JSON types (like sets or custom objects) and non-string mapping keys + nested inside dicts or lists are safely normalized before dumping. + """ + def _normalize(obj: Any) -> Any: + if isinstance(obj, dict): + normalized: Dict[str, Any] = {} + for k, v in obj.items(): + key_str = str(k) + if key_str in normalized: + raise ValueError( + f"Query parameter dictionary key collision: conflicting key {k!r} maps to existing {key_str!r}" + ) + normalized[key_str] = _normalize(v) + return normalized + if isinstance(obj, (set, frozenset)): + return [_normalize(x) for x in sorted(obj, key=str)] + if isinstance(obj, (list, tuple)): + return [_normalize(x) for x in obj] + return obj + + def _encode_item(item: Any) -> Optional[Any]: + if item is None: + return None + if isinstance(item, bool): + return "true" if item else "false" + if isinstance(item, (int, float, str)): + return item + if isinstance(item, (set, frozenset)): + return json.dumps([_normalize(x) for x in sorted(item, key=str)], default=str, separators=(",", ":")) + if isinstance(item, (dict, list, tuple)): + return json.dumps(_normalize(item), default=str, separators=(",", ":")) + return str(item) + + result: Dict[str, Any] = {} + for k, v in params.items(): + if v is None: + continue + key_str = str(k) + if isinstance(v, (list, tuple)): + encoded_items = [_encode_item(sub) for sub in v if sub is not None] + result[key_str] = encoded_items + else: + encoded_val = _encode_item(v) + if encoded_val is not None: + result[key_str] = encoded_val + + return result + + + + diff --git a/plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py b/plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py index 141a979..fc1bc9c 100644 --- a/plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py +++ b/plugins/communication_protocols/http/src/utcp_http/http_communication_protocol.py @@ -38,6 +38,7 @@ ensure_loopback_was_declared, safe_request_with_redirects, reject_remote_loopback_tool_urls, + encode_query_params, ) from utcp_http._errors import raise_for_status_with_body import logging @@ -332,7 +333,7 @@ async def call_tool(self, caller, tool_name: str, tool_args: Dict[str, Any], too ensure_loopback_was_declared(tool_call_template.url, url, context="tool invocation") # The rest of the arguments are query parameters - query_params = remaining_args + query_params = encode_query_params(remaining_args) # Handle authentication auth, cookies, auth_header_names = self._apply_auth(tool_call_template, request_headers, query_params) diff --git a/plugins/communication_protocols/http/src/utcp_http/sse_communication_protocol.py b/plugins/communication_protocols/http/src/utcp_http/sse_communication_protocol.py index fa4ca72..d96e0a3 100644 --- a/plugins/communication_protocols/http/src/utcp_http/sse_communication_protocol.py +++ b/plugins/communication_protocols/http/src/utcp_http/sse_communication_protocol.py @@ -24,6 +24,7 @@ ensure_loopback_was_declared, safe_request_with_redirects, reject_remote_loopback_tool_urls, + encode_query_params, ) import traceback import logging @@ -246,7 +247,7 @@ async def call_tool_streaming(self, caller, tool_name: str, tool_args: Dict[str, ensure_loopback_was_declared(tool_call_template.url, url, context="tool invocation") # The rest of the arguments are query parameters - query_params = remaining_args + query_params = encode_query_params(remaining_args) # Handle authentication # ``auth_header_names`` unused in the streaming path because diff --git a/plugins/communication_protocols/http/src/utcp_http/streamable_http_communication_protocol.py b/plugins/communication_protocols/http/src/utcp_http/streamable_http_communication_protocol.py index c25b43d..10f287f 100644 --- a/plugins/communication_protocols/http/src/utcp_http/streamable_http_communication_protocol.py +++ b/plugins/communication_protocols/http/src/utcp_http/streamable_http_communication_protocol.py @@ -21,6 +21,7 @@ ensure_loopback_was_declared, safe_request_with_redirects, reject_remote_loopback_tool_urls, + encode_query_params, ) import logging @@ -251,7 +252,7 @@ async def call_tool_streaming(self, caller, tool_name: str, tool_args: Dict[str, ensure_loopback_was_declared(tool_call_template.url, url, context="tool invocation") # The rest of the arguments are query parameters - query_params = remaining_args + query_params = encode_query_params(remaining_args) # Handle authentication # ``auth_header_names`` unused here -- streaming handshake uses diff --git a/plugins/communication_protocols/http/tests/test_http_communication_protocol.py b/plugins/communication_protocols/http/tests/test_http_communication_protocol.py index 4504e2c..c9e7869 100644 --- a/plugins/communication_protocols/http/tests/test_http_communication_protocol.py +++ b/plugins/communication_protocols/http/tests/test_http_communication_protocol.py @@ -163,6 +163,15 @@ async def forbidden_huge_handler(request): return web.Response(status=403, text="x" * (1024 * 1024)) app.router.add_route('*', '/forbidden-object-then-message', forbidden_object_then_message_handler) + + # Echo query parameters endpoint for testing serialization + async def echo_query_handler(request): + return web.json_response({ + "query": dict(request.query), + "multi_query": {k: request.query.getall(k) for k in request.query.keys()}, + }) + + app.router.add_get('/echo-query', echo_query_handler) app.router.add_route('*', '/forbidden-huge', forbidden_huge_handler) # No Content-Type at all, so no charset to decode with. @@ -887,3 +896,53 @@ async def test_deeply_nested_error_body_still_raises_client_response_error(http_ def test_error_detail_collapses_control_characters(): from utcp_http._errors import error_detail_from_body assert error_detail_from_body('{"error": "line one\\nline two\\u001b[31m"}') == "line one line two [31m" + + +@pytest.mark.asyncio +async def test_call_tool_encodes_object_and_list_query_parameters(http_transport, aiohttp_client, app): + """Verify dicts, sets, nested sequences, and primitives are properly encoded, and None is dropped (#107).""" + client = await aiohttp_client(app) + call_template = HttpCallTemplate( + name="echo_tool", + url=f"http://localhost:{client.port}/echo-query", + http_method="GET", + ) + tool_args = { + "filter": {"status": "open", "nested": {100: "numeric_key"}, "tags": {"python", "async"}}, + "tags": ["alpha", "beta"], + "active": True, + "count": 42, + "categories": {"electronics", "books"}, + "matrix": [[1, 2], [3, 4]], + "optional_unset": None, + } + result = await http_transport.call_tool(None, "echo_tool", tool_args, call_template) + assert result["query"]["filter"] == '{"status":"open","nested":{"100":"numeric_key"},"tags":["async","python"]}' + assert result["query"]["active"] == "true" + assert result["query"]["count"] == "42" + assert result["query"]["categories"] == '["books","electronics"]' + assert "optional_unset" not in result["query"] + assert result["multi_query"]["tags"] == ["alpha", "beta"] + assert result["multi_query"]["matrix"] == ["[1,2]", "[3,4]"] + + +@pytest.mark.asyncio +async def test_call_tool_detects_nested_dictionary_key_collision(http_transport, aiohttp_client, app): + """Verify that distinct dictionary keys that map to the same string raise ValueError instead of silently overwriting.""" + client = await aiohttp_client(app) + call_template = HttpCallTemplate( + name="echo_tool", + url=f"http://localhost:{client.port}/echo-query", + http_method="GET", + ) + tool_args = { + "filter": {100: "first", "100": "second"} + } + with pytest.raises(ValueError, match="Query parameter dictionary key collision"): + await http_transport.call_tool(None, "echo_tool", tool_args, call_template) + + + + + +