From ecc977177d135e717b4e646520e6eb09321e2e24 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 30 Oct 2026 21:22:51 -0700 Subject: [PATCH] fix(http): keep GET URL query strings intact so presigned image links stop failing GET requests rebuilt the URL query from a parsed dict, which dropped blank values, collapsed repeated keys and re-encoded escapes. That broke signed S3 and GCS URLs (SignatureDoesNotMatch). The query is now sent as given and caller params are only appended when the URL lacks that key. Fixes #47837 --- litellm/llms/custom_httpx/http_handler.py | 43 +++++++++-------- litellm/llms/custom_httpx/llm_http_handler.py | 4 +- .../llms/custom_httpx/test_http_handler.py | 48 +++++++++++++++++++ .../custom_httpx/test_llm_http_handler.py | 35 ++++++++++++++ 4 files changed, 109 insertions(+), 21 deletions(-) diff --git a/litellm/llms/custom_httpx/http_handler.py b/litellm/llms/custom_httpx/http_handler.py index 5620971a8a2..b1bf2481dd1 100644 --- a/litellm/llms/custom_httpx/http_handler.py +++ b/litellm/llms/custom_httpx/http_handler.py @@ -13,6 +13,7 @@ from http.cookiejar import CookieJar, DefaultCookiePolicy from io import BytesIO from types import MappingProxyType from typing import TYPE_CHECKING, Any, ClassVar, Final, NoReturn, Optional, TypeAlias, TypedDict, TypeVar +from urllib.parse import parse_qsl import certifi import httpx @@ -535,6 +536,17 @@ def _safe_get_response_text(response: httpx.Response) -> str: return "" +def url_with_query_params(url: str, params: httpx.QueryParams) -> httpx.URL: + """Keep the URL's query bytes as sent and append only params whose keys the URL lacks, so signed URLs stay valid.""" + target: Final = httpx.URL(url) + raw_query: Final = target.query.decode("ascii") + url_keys: Final = frozenset(key for key, _ in parse_qsl(raw_query, keep_blank_values=True)) + extra: Final = str(httpx.QueryParams([(key, value) for key, value in params.multi_items() if key not in url_keys])) + if not extra: + return target + return target.copy_with(query="&".join(part for part in (raw_query, extra) if part).encode("ascii")) + + def header_value(headers: Mapping[str, str], name: str) -> str | None: """Read one header as ``str | None``; ``httpx.Headers.get`` itself is typed ``Any``.""" return headers.get(name) @@ -821,18 +833,17 @@ class AsyncHTTPHandler: ): # Set follow_redirects to UseClientDefault if None _follow_redirects: Final = follow_redirects if follow_redirects is not None else USE_CLIENT_DEFAULT - query: Final = httpx.QueryParams(params).merge(HTTPHandler.extract_query_params(url)) + request_url: Final = url_with_query_params(url, httpx.QueryParams(params)) request_headers: Final = httpx.Headers(headers) if stream: return await self._send_streaming_get( - url, params=query, headers=request_headers, follow_redirects=follow_redirects, timeout=timeout + request_url, headers=request_headers, follow_redirects=follow_redirects, timeout=timeout ) if max_response_bytes is not None: return await self._get_with_response_limit( - url, - params=query, + request_url, headers=request_headers, max_bytes=max_response_bytes, follow_redirects=self.client.follow_redirects if follow_redirects is None else follow_redirects, @@ -840,8 +851,7 @@ class AsyncHTTPHandler: ) response: Final = await self.client.get( - url, - params=query, + request_url, headers=request_headers, follow_redirects=_follow_redirects, timeout=timeout if timeout is not None else USE_CLIENT_DEFAULT, @@ -850,16 +860,15 @@ class AsyncHTTPHandler: async def _send_streaming_get( self, - url: str, + url: httpx.URL, *, - params: httpx.QueryParams, headers: httpx.Headers, follow_redirects: bool | None, timeout: float | httpx.Timeout | None, ) -> httpx.Response: start_time: Final = time.time() request: Final = self.client.build_request( - "GET", url, params=params, headers=headers, timeout=USE_CLIENT_DEFAULT if timeout is None else timeout + "GET", url, headers=headers, timeout=USE_CLIENT_DEFAULT if timeout is None else timeout ) try: response: Final = await self.client.send( @@ -878,9 +887,8 @@ class AsyncHTTPHandler: async def _get_with_response_limit( self, - url: str, + url: httpx.URL, *, - params: httpx.QueryParams, headers: httpx.Headers, timeout: httpx.Timeout, max_bytes: int, @@ -890,7 +898,6 @@ class AsyncHTTPHandler: "GET", url, headers=MappingProxyType({**headers, "accept-encoding": "identity"}), - params=params, timeout=timeout, ) response: Final = await self.client.send(request, stream=True, follow_redirects=False) @@ -1609,16 +1616,15 @@ class HTTPHandler: ): # Set follow_redirects to UseClientDefault if None _follow_redirects: Final = follow_redirects if follow_redirects is not None else USE_CLIENT_DEFAULT - query: Final = httpx.QueryParams(params).merge(self.extract_query_params(url)) + request_url: Final = url_with_query_params(url, httpx.QueryParams(params)) if stream: return self._send_streaming_get( - url, params=query, headers=headers, follow_redirects=follow_redirects, timeout=timeout + request_url, headers=headers, follow_redirects=follow_redirects, timeout=timeout ) response: Final = self.client.get( - url, - params=query, + request_url, headers=headers, follow_redirects=_follow_redirects, timeout=timeout if timeout is not None else USE_CLIENT_DEFAULT, @@ -1628,16 +1634,15 @@ class HTTPHandler: def _send_streaming_get( self, - url: str, + url: httpx.URL, *, - params: httpx.QueryParams, headers: Mapping[str, str] | None, follow_redirects: bool | None, timeout: float | httpx.Timeout | None, ) -> httpx.Response: start_time: Final = time.time() request: Final = self.client.build_request( - "GET", url, params=params, headers=headers, timeout=USE_CLIENT_DEFAULT if timeout is None else timeout + "GET", url, headers=headers, timeout=USE_CLIENT_DEFAULT if timeout is None else timeout ) try: response: Final = self.client.send( diff --git a/litellm/llms/custom_httpx/llm_http_handler.py b/litellm/llms/custom_httpx/llm_http_handler.py index 9a2b352dd86..db9adb51520 100644 --- a/litellm/llms/custom_httpx/llm_http_handler.py +++ b/litellm/llms/custom_httpx/llm_http_handler.py @@ -122,6 +122,7 @@ from litellm.llms.custom_httpx.http_handler import ( get_async_httpx_client, get_httpx_client, header_value, + url_with_query_params, ) from litellm.llms.custom_httpx.sse_lines import aiter_sse_lines, iter_sse_lines from litellm.responses.streaming_iterator import ( @@ -5804,9 +5805,8 @@ class BaseLLMHTTPHandler: request: Final = async_httpx_client.client.build_request( "GET", - prepared.url, + url_with_query_params(prepared.url, httpx.QueryParams(prepared.params)), headers=prepared.headers, - params=httpx.QueryParams(HTTPHandler.extract_query_params(prepared.url)).merge(prepared.params), timeout=USE_CLIENT_DEFAULT if timeout is None else httpx.Timeout(timeout), ) try: diff --git a/tests/unit/llms/custom_httpx/test_http_handler.py b/tests/unit/llms/custom_httpx/test_http_handler.py index 0163cb9b26d..963dd1bdc09 100644 --- a/tests/unit/llms/custom_httpx/test_http_handler.py +++ b/tests/unit/llms/custom_httpx/test_http_handler.py @@ -711,6 +711,54 @@ async def test_async_get_forwards_per_request_timeout(): await handler.close() + +SIGNED_URL_QUERY: Final = b"X-Amz-Signature=abc&ResponseContentLanguage=&p=a%20b&r=1&r=2&token&t=%7E&s=a%2fb&u=a/b:c" +SIGNED_URL: Final = "https://bucket.example/photo.png?" + SIGNED_URL_QUERY.decode() + +QUERY_MERGE_CASES: Final = ( + pytest.param(SIGNED_URL, None, SIGNED_URL_QUERY, id="no-params-keeps-signed-query"), + pytest.param(SIGNED_URL, {}, SIGNED_URL_QUERY, id="empty-params-keeps-signed-query"), + pytest.param(SIGNED_URL, {"limit": 5}, SIGNED_URL_QUERY + b"&limit=5", id="params-append-after-signed-query"), + pytest.param( + "https://example.com/files?api-version=1", + {"api-version": "2", "after": "x"}, + b"api-version=1&after=x", + id="url-value-wins-on-clash", + ), + pytest.param("https://example.com/files?a=", {"a": "1"}, b"a=", id="blank-url-value-still-wins"), + pytest.param("https://example.com/files", {"after": "x/y"}, b"after=x%2Fy", id="params-only"), +) + + +def _echo_query_transport() -> httpx.MockTransport: + return httpx.MockTransport(lambda request: httpx.Response(200, request=request, content=request.url.query)) + + +@pytest.mark.parametrize("url,params,expected_query", QUERY_MERGE_CASES) +@pytest.mark.parametrize("stream", [False, True]) +def test_sync_get_sends_url_query_bytes_unchanged(url, params, expected_query, stream): + handler = HTTPHandler(client=httpx.Client(transport=_echo_query_transport())) + try: + response = handler.get(url, params=params, stream=stream) + assert response.read() == expected_query + finally: + handler.client.close() + + +@pytest.mark.parametrize("url,params,expected_query", QUERY_MERGE_CASES) +@pytest.mark.parametrize( + "get_kwargs", [{}, {"stream": True}, {"max_response_bytes": 1000}], ids=["plain", "stream", "bounded"] +) +async def test_async_get_sends_url_query_bytes_unchanged(url, params, expected_query, get_kwargs): + handler = AsyncHTTPHandler() + await handler.client.aclose() + handler.client = httpx.AsyncClient(transport=_echo_query_transport()) + try: + response = await handler.get(url, params=params, **get_kwargs) + assert await response.aread() == expected_query + finally: + await handler.client.aclose() + class TestDefaultCachedClientTimeoutHonorsRequestTimeout: """Cached default httpx clients must fall back to an explicit litellm.request_timeout. diff --git a/tests/unit/llms/custom_httpx/test_llm_http_handler.py b/tests/unit/llms/custom_httpx/test_llm_http_handler.py index 23d7feb2557..75752d6c1b2 100644 --- a/tests/unit/llms/custom_httpx/test_llm_http_handler.py +++ b/tests/unit/llms/custom_httpx/test_llm_http_handler.py @@ -26,6 +26,7 @@ from litellm.integrations.code_interpreter_interception.handler import ( ) from litellm.integrations.custom_logger import CustomLogger from litellm.integrations.websearch_interception.handler import WebSearchInterceptionLogger +from litellm.llms.anthropic.files.transformation import AnthropicFilesConfig from litellm.llms.azure.responses.transformation import AzureOpenAIResponsesAPIConfig from litellm.llms.azure.vector_stores.transformation import AzureOpenAIVectorStoreConfig from litellm.llms.base_llm.audio_transcription.transformation import ( @@ -3201,6 +3202,40 @@ def test_sync_retrieve_file_content_raises_on_http_error(): assert exc_info.value.status_code == 404 +SIGNED_DOWNLOAD_QUERY: Final = "X-Amz-Signature=abc&ResponseContentLanguage=&p=a%20b&r=1&r=2&token&t=%7E&s=a%2fb" + + +class _SignedDownloadFilesConfig(AnthropicFilesConfig): + def transform_file_content_request(self, file_content_request, optional_params, litellm_params): + return f"https://downloads.example/{file_content_request['file_id']}?{SIGNED_DOWNLOAD_QUERY}", { + "token": "from-params", + "extra": "1", + } + + +async def test_streaming_file_content_keeps_signed_url_query_and_appends_new_params(): + async_client = AsyncHTTPHandler() + await async_client.client.aclose() + async_client.client = httpx.AsyncClient( + transport=httpx.MockTransport(lambda request: httpx.Response(200, request=request, content=request.url.query)) + ) + try: + result = await BaseLLMHTTPHandler().async_retrieve_file_content_streaming( + file_content_request={"file_id": "file-abc"}, + provider_config=_SignedDownloadFilesConfig(), + litellm_params={"api_key": "sk-test"}, + headers={}, + logging_obj=Mock(), + chunk_size=1024, + client=async_client, + ) + body = b"".join([chunk async for chunk in result.stream_iterator]) + finally: + await async_client.client.aclose() + + assert body == f"{SIGNED_DOWNLOAD_QUERY}&extra=1".encode() + + @pytest.mark.asyncio async def test_handle_error_surfaces_provider_error_raised_while_aiohttp_streams_the_body( monkeypatch: pytest.MonkeyPatch,