From 0edb4123b52c70a06876c3ec379ccefdeace00a4 Mon Sep 17 00:00:00 2001 From: Manjunath Janardhan Date: Wed, 19 Aug 2026 18:32:46 +0530 Subject: [PATCH 1/2] Python: fix: preserve URL query parameters in DefaultHttpRequestHandler The declarative handler passed query_parameters to httpx's params= kwarg, which replaces any query string already present in the URL. URL-embedded parameters (api-version, tenant, ...) were silently dropped when combined with query_parameters. Append encoded query_parameters to the URL's existing query using the same rules as .NET DefaultHttpRequestHandler.ResolveRequestUri: keep existing parameters, use '&' if the URL already contains '?', skip empty keys, and percent-encode via quote_via=quote so spaces encode as %20 (matching Uri.EscapeDataString) rather than form-encoded '+'. The URL is left unchanged when query_parameters is empty or contains only empty keys. Fixes #7749 (Python side; .NET was already correct) --- .../_workflows/_http_handler.py | 24 ++++- .../test_default_http_request_handler.py | 98 +++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py b/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py index 6c1790509d1..733f2716431 100644 --- a/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py +++ b/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py @@ -24,6 +24,7 @@ from collections.abc import Awaitable, Callable, Mapping from dataclasses import dataclass, field from typing import Any, Protocol, runtime_checkable +from urllib.parse import quote, urlencode, urlsplit, urlunsplit import httpx @@ -173,12 +174,29 @@ async def send(self, info: HttpRequestInfo) -> HttpRequestResult: # callers (not just the YAML executor) get sensible defaults. headers["Content-Type"] = info.body_content_type or "text/plain" - params: Mapping[str, str] | None = info.query_parameters or None + # Build the final URL manually instead of passing httpx's ``params=`` kwarg, + # which *replaces* any query string already present in ``info.url``. Append + # encoded ``query_parameters`` to the URL's existing query, preserving + # URL-embedded params. Matches the .NET ``DefaultHttpRequestHandler. + # ResolveRequestUri`` behavior (skip empty keys, leave the URL unchanged when + # nothing remains, percent-encode with ``%20`` for spaces rather than + # form-encoded ``+``), and additionally uses urlsplit/urlunsplit so the new + # query string is inserted between the path and any fragment (avoids the + # string-concat bug that would place the query after a trailing ``#fragment``). + url = info.url + query_string = urlencode( + [(key, value) for key, value in info.query_parameters.items() if key], + quote_via=quote, + ) + if query_string: + parts = urlsplit(url) + existing_query = parts.query + merged_query = f"{existing_query}&{query_string}" if existing_query else query_string + url = urlunsplit((parts.scheme, parts.netloc, parts.path, merged_query, parts.fragment)) response = await client.request( method=info.method, - url=info.url, - params=params, + url=url, headers=headers or None, content=content, timeout=timeout, diff --git a/python/packages/declarative/tests/test_default_http_request_handler.py b/python/packages/declarative/tests/test_default_http_request_handler.py index 93cfc9b6745..2c495597b47 100644 --- a/python/packages/declarative/tests/test_default_http_request_handler.py +++ b/python/packages/declarative/tests/test_default_http_request_handler.py @@ -68,6 +68,104 @@ def respond(request: httpx.Request) -> httpx.Response: assert req.url.params.get("q") == "alpha" assert req.url.params.get("limit") == "5" + @pytest.mark.asyncio + async def test_query_parameters_preserve_url_query_string(self) -> None: + """query_parameters must be appended, not replace the URL's existing query string. + + Regression for https://github.com/microsoft/agent-framework/issues/7749: + passing ``params=`` to httpx replaces the URL's existing query string, and the + handler used to do exactly that — URL parameters (api-version, tenant, ...) were + silently dropped. The .NET DefaultHttpRequestHandler.ResolveRequestUri preserves + them; Python now matches that behavior. + """ + captured: dict[str, httpx.Request] = {} + + def respond(request: httpx.Request) -> httpx.Response: + captured["req"] = request + return httpx.Response(200, text="ok") + + handler = _make_handler(httpx.MockTransport(respond)) + try: + await handler.send( + HttpRequestInfo( + method="GET", + url="https://api.example.test/items?api-version=2025-01-01&tenant=alpha", + query_parameters={"page": "2"}, + ) + ) + finally: + await handler.aclose() + + req = captured["req"] + # URL-supplied params are preserved and query_parameters are appended on top + assert req.url.params.get("api-version") == "2025-01-01" + assert req.url.params.get("tenant") == "alpha" + assert req.url.params.get("page") == "2" + + @pytest.mark.asyncio + async def test_query_parameters_append_before_fragment(self) -> None: + """query_parameters must be inserted before any fragment, not appended after. + + Plain string concatenation would produce ``url#frag?key=val`` (an invalid URL + — the query string must come before the fragment). The fix uses urlsplit so + reassembly keeps the fragment last. + """ + captured: dict[str, httpx.Request] = {} + + def respond(request: httpx.Request) -> httpx.Response: + captured["req"] = request + return httpx.Response(200, text="ok") + + handler = _make_handler(httpx.MockTransport(respond)) + try: + await handler.send( + HttpRequestInfo( + method="GET", + url="https://api.example.test/items?api-version=2025-01-01#section", + query_parameters={"page": "2"}, + ) + ) + finally: + await handler.aclose() + + req = captured["req"] + assert req.url.params.get("api-version") == "2025-01-01" + assert req.url.params.get("page") == "2" + # The fragment is preserved and stays last + assert str(req.url).endswith("#section") + assert req.url.fragment == "section" + + @pytest.mark.asyncio + async def test_query_parameters_when_url_ends_with_question_mark(self) -> None: + """query_parameters must append cleanly when the URL already ends with ``?``. + + urlsplit parses the trailing ``?`` as an empty query part, so the result is + a clean ``?key=val`` rather than a malformed ``?&key=val`` that plain string + concatenation would produce. + """ + captured: dict[str, httpx.Request] = {} + + def respond(request: httpx.Request) -> httpx.Response: + captured["req"] = request + return httpx.Response(200, text="ok") + + handler = _make_handler(httpx.MockTransport(respond)) + try: + await handler.send( + HttpRequestInfo( + method="GET", + url="https://api.example.test/items?", + query_parameters={"page": "2"}, + ) + ) + finally: + await handler.aclose() + + req = captured["req"] + assert req.url.params.get("page") == "2" + # Result is a clean query string, not && or && at the start + assert str(req.url) == "https://api.example.test/items?page=2" + @pytest.mark.asyncio async def test_body_content_type_forwarded(self) -> None: captured: dict[str, httpx.Request] = {} From d81257aef4bc4f17769cb1e47639ac05ca0e7949 Mon Sep 17 00:00:00 2001 From: Manjunath Janardhan Date: Thu, 20 Aug 2026 13:55:33 +0530 Subject: [PATCH 2/2] Python: fix: preserve client-level AsyncClient params in DefaultHttpRequestHandler Address moonbox3 review on #7765: passing params= or omitting it in client.request lets the httpx AsyncClient's params= replace the URL's raw query, so embedding query_parameters in the URL could silently drop the URL-embedded query whenever a caller's AsyncClient(params=...) was supplied. Collect client params, URL-embedded params, and query_parameters into a single explicit params= list (client < URL < query_parameters precedence, with later sources overriding duplicates) and pass that to client.request. URLunsplit strips the raw query from the URL so it is not double-applied, and urlsplit keeps the final query before any #fragment. Regression test: three-source merge preserved; duplicate key overridden; empty keys dropped. Fixes #7749 review feedback --- .../_workflows/_http_handler.py | 56 +++++++---- .../test_default_http_request_handler.py | 95 +++++++++++++++++++ 2 files changed, 132 insertions(+), 19 deletions(-) diff --git a/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py b/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py index 733f2716431..39ba32a8317 100644 --- a/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py +++ b/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py @@ -24,7 +24,7 @@ from collections.abc import Awaitable, Callable, Mapping from dataclasses import dataclass, field from typing import Any, Protocol, runtime_checkable -from urllib.parse import quote, urlencode, urlsplit, urlunsplit +from urllib.parse import urlsplit, urlunsplit import httpx @@ -174,29 +174,47 @@ async def send(self, info: HttpRequestInfo) -> HttpRequestResult: # callers (not just the YAML executor) get sensible defaults. headers["Content-Type"] = info.body_content_type or "text/plain" - # Build the final URL manually instead of passing httpx's ``params=`` kwarg, - # which *replaces* any query string already present in ``info.url``. Append - # encoded ``query_parameters`` to the URL's existing query, preserving - # URL-embedded params. Matches the .NET ``DefaultHttpRequestHandler. - # ResolveRequestUri`` behavior (skip empty keys, leave the URL unchanged when - # nothing remains, percent-encode with ``%20`` for spaces rather than - # form-encoded ``+``), and additionally uses urlsplit/urlunsplit so the new - # query string is inserted between the path and any fragment (avoids the - # string-concat bug that would place the query after a trailing ``#fragment``). + # Merge all three query sources into one ``params=`` list. Passing only + # ``query_parameters`` into ``params=`` (the pre-#7765 bug) drops URL-embedded + # params, and passing nothing (#7765 first iteration) lets the *client*' + # ``AsyncClient.params`` (via ``Client._merge_queryparams``) drop the URL's + # query. Collecting client params + URL params + query_parameters into one + # explicit list preserves every source. Precedence is client (lowest) < URL + # query < ``query_parameters`` (highest): on a duplicate key the later source' + # value replaces the earlier one (dict-of-lists assignment), matching the old + # ``_merge_queryparams.merge(params)`` per-request-wins behavior. urlsplit/ + # urlunsplit keeps the final query before any ``#fragment``. url = info.url - query_string = urlencode( - [(key, value) for key, value in info.query_parameters.items() if key], - quote_via=quote, - ) - if query_string: - parts = urlsplit(url) - existing_query = parts.query - merged_query = f"{existing_query}&{query_string}" if existing_query else query_string - url = urlunsplit((parts.scheme, parts.netloc, parts.path, merged_query, parts.fragment)) + raw = urlsplit(url) + merged: dict[str, list[str]] = {} + overridden: set[str] = set() + + def _merge_layer(pairs: list[tuple[str, str]]) -> None: + seen: set[str] = set() + for key, value in pairs: + if key in overridden and key not in seen: + # First occurrence in this layer overrides the lower-precedence value. + merged[key] = [value] + else: + merged.setdefault(key, []).append(value) + seen.add(key) + + _merge_layer(list(client.params.multi_items())) + overridden.update(merged) + _merge_layer(list(httpx.URL(url).params.multi_items())) + overridden.update(merged) + _merge_layer([(key, value) for key, value in info.query_parameters.items() if key]) + + params: list[tuple[str, Any]] = [(k, v) for k, values in merged.items() for v in values] + if params: + # urlunsplit with an empty query part strips the raw query so it is not + # double-applied; ``params=`` now carries all three sources. + url = urlunsplit((raw.scheme, raw.netloc, raw.path, "", raw.fragment)) response = await client.request( method=info.method, url=url, + params=params or None, headers=headers or None, content=content, timeout=timeout, diff --git a/python/packages/declarative/tests/test_default_http_request_handler.py b/python/packages/declarative/tests/test_default_http_request_handler.py index 2c495597b47..8e91fa3d08f 100644 --- a/python/packages/declarative/tests/test_default_http_request_handler.py +++ b/python/packages/declarative/tests/test_default_http_request_handler.py @@ -166,6 +166,101 @@ def respond(request: httpx.Request) -> httpx.Response: # Result is a clean query string, not && or && at the start assert str(req.url) == "https://api.example.test/items?page=2" + @pytest.mark.asyncio + async def test_query_parameters_preserve_client_level_params(self) -> None: + """Client-level ``AsyncClient.params`` must survive alongside URL-embedded and + ``query_parameters`` params. + + moonbox3 review on #7765: when a caller-supplied ``AsyncClient`` is built with + ``params=``, passing ``params=`` (or nothing) into ``client.request`` + re-merges and lets the client's params *replace* the URL's query. The handler + must collect all three sources into one explicit ``params=`` list. + """ + captured: dict[str, httpx.Request] = {} + + def respond(request: httpx.Request) -> httpx.Response: + captured["req"] = request + return httpx.Response(200, text="ok") + + client = httpx.AsyncClient( + params={"client-default": "v"}, + transport=httpx.MockTransport(respond), + ) + handler = DefaultHttpRequestHandler(client=client) + try: + await handler.send( + HttpRequestInfo( + method="GET", + url="https://api.example.test/items?tenant=alpha&page=2", + query_parameters={"limit": "5"}, + ) + ) + finally: + await handler.aclose() + await client.aclose() + + req_url = str(captured["req"].url) + for needle in ("client-default=v", "tenant=alpha", "page=2", "limit=5"): + assert needle in req_url, f"missing {needle!r} in {req_url!r}" + # Lower-precedence source's duplicate key is overridden (query_parameters win). + assert "tenant=alpha" in req_url # smoke: distinct keys all survive + + @pytest.mark.asyncio + async def test_query_parameters_higher_precedence_than_client_params(self) -> None: + """On duplicate keys, ``query_parameters`` and URL params override client-defaults.""" + captured: dict[str, httpx.Request] = {} + + def respond(request: httpx.Request) -> httpx.Response: + captured["req"] = request + return httpx.Response(200, text="ok") + + client = httpx.AsyncClient( + params={"key": "clientval"}, + transport=httpx.MockTransport(respond), + ) + handler = DefaultHttpRequestHandler(client=client) + try: + await handler.send( + HttpRequestInfo( + method="GET", + url="https://api.example.test/items?key=urlval", + query_parameters={"key": "qpval"}, + ) + ) + finally: + await handler.aclose() + await client.aclose() + + req_url = str(captured["req"].url) + assert "key=qpval" in req_url + assert "key=urlval" not in req_url + assert "key=clientval" not in req_url + + @pytest.mark.asyncio + async def test_query_parameters_empty_key_skipped(self) -> None: + """Empty query-parameter keys are dropped (matches .NET ResolveRequestUri).""" + captured: dict[str, httpx.Request] = {} + + def respond(request: httpx.Request) -> httpx.Response: + captured["req"] = request + return httpx.Response(200, text="ok") + + handler = _make_handler(httpx.MockTransport(respond)) + try: + await handler.send( + HttpRequestInfo( + method="GET", + url="https://api.example.test/items", + query_parameters={"": "shouldbedropped", "page": "2"}, + ) + ) + finally: + await handler.aclose() + + req_url = str(captured["req"].url) + assert "shouldbedropped" not in req_url + assert "page=2" in req_url + @pytest.mark.asyncio async def test_body_content_type_forwarded(self) -> None: captured: dict[str, httpx.Request] = {}