mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-02 07:13:02 +00:00
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
This commit is contained in:
parent
02a6a96dec
commit
ecc977177d
4 changed files with 109 additions and 21 deletions
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue