mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(proxy): forward a non-object passthrough body as the bytes it arrived as
Coercing a non-object body to `{}` fixed the crash but broke what the
provider receives. `pass_through_request` builds the upstream request as
`json=_parsed_body` unless `state_raw_body` is set, and only SigV4
signers set that, so every ordinary `/anthropic` or `/openai` passthrough
sent `{}` in place of the caller's own payload. Silent data loss is worse
than the 500 it replaced.
Reuse the exact-bytes path that already exists for SigV4 rather than
adding a second mechanism: `_read_request_body` marks the request when it
coerces, `non_object_raw_body` returns the original bytes for exactly
those requests, and `pass_through_request` feeds them into
`state_raw_body`, which both the streaming and non-streaming branches
already honour.
Only a coerced body takes this path. An object body still goes through
`json=_parsed_body`, so hook mutations reach the provider, and a
SigV4 body keeps its own bytes because the fallback runs only when
`state_raw_body` is still None.
The earlier manual check could not have caught this: Anthropic answers
`model: Field required` for both `[]` and `{}`. The new test captures the
request actually built for the provider and asserts the outbound bytes.
This commit is contained in:
parent
e459979400
commit
2666539d0c
4 changed files with 149 additions and 9 deletions
|
|
@ -24,6 +24,8 @@ _FORM_CONTENT_TYPES: Final[frozenset[str]] = frozenset({"application/x-www-form-
|
|||
|
||||
_ANNOTATION_QUALIFIERS: Final[frozenset[object]] = frozenset({Annotated, NotRequired, ReadOnly, Required})
|
||||
|
||||
_NON_OBJECT_BODY_SCOPE_KEY: Final[str] = "litellm_non_object_json_body"
|
||||
|
||||
|
||||
def _normalize_media_type(content_type: str) -> str:
|
||||
"""Return the bare media type per RFC 7231: strip params, trim, lowercase."""
|
||||
|
|
@ -215,11 +217,15 @@ async def _read_request_body(request: Request | None) -> dict:
|
|||
code=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
|
||||
# Cache the parsed result. Raw passthrough forwarding re-reads ``request.body()``,
|
||||
# so dropping a non-object body here never changes what reaches the provider.
|
||||
object_body: Final[dict] = parsed_body if isinstance(parsed_body, dict) else {}
|
||||
_safe_set_request_parsed_body(request=request, parsed_body=object_body)
|
||||
return object_body
|
||||
if isinstance(parsed_body, dict):
|
||||
_safe_set_request_parsed_body(request=request, parsed_body=parsed_body)
|
||||
return parsed_body
|
||||
|
||||
# Anything that forwards the parsed body upstream would now send an empty object in
|
||||
# place of the caller's payload, so mark the request for ``non_object_raw_body``.
|
||||
_mark_non_object_body(request=request)
|
||||
_safe_set_request_parsed_body(request=request, parsed_body={})
|
||||
return {}
|
||||
|
||||
except (json.JSONDecodeError, orjson.JSONDecodeError, ProxyException) as e:
|
||||
# Re-raise ProxyException as-is
|
||||
|
|
@ -239,6 +245,32 @@ def is_opaque_audio_pass_through_request(route: str, content_type: str) -> bool:
|
|||
)
|
||||
|
||||
|
||||
def _mark_non_object_body(request: Request | None) -> None:
|
||||
try:
|
||||
if request is not None:
|
||||
request.scope[_NON_OBJECT_BODY_SCOPE_KEY] = True
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.debug("Unexpected error marking non-object request body - %s", e)
|
||||
|
||||
|
||||
async def non_object_raw_body(request: Request | None) -> bytes | None:
|
||||
"""The bytes of a body that is valid JSON but not an object, else ``None``.
|
||||
|
||||
``_read_request_body`` reads such a body as ``{}`` so every caller that treats it as a
|
||||
mapping is safe, which means forwarding the parsed body would send an empty object in
|
||||
place of the caller's payload. Passthrough sends these bytes instead.
|
||||
"""
|
||||
if request is None:
|
||||
return None
|
||||
scope: Final[object] = getattr(request, "scope", None)
|
||||
if not isinstance(scope, Mapping) or scope.get(_NON_OBJECT_BODY_SCOPE_KEY) is not True:
|
||||
return None
|
||||
try:
|
||||
return await request.body()
|
||||
except RuntimeError:
|
||||
return None
|
||||
|
||||
|
||||
async def read_raw_json_body(request: Request | None) -> bytes | None:
|
||||
if request is None or _safe_get_request_parsed_body(request=request) is None:
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -86,6 +86,7 @@ from litellm.proxy.common_utils.error_body_call_id import JSON_OBJECT, error_bod
|
|||
from litellm.proxy.common_utils.http_parsing_utils import (
|
||||
_read_request_body,
|
||||
_safe_get_request_headers,
|
||||
non_object_raw_body,
|
||||
)
|
||||
from litellm.proxy.common_utils.openai_error_payload import (
|
||||
LITELLM_CALL_ID_HEADER,
|
||||
|
|
@ -1120,6 +1121,10 @@ async def pass_through_request(
|
|||
_parsed_body = {}
|
||||
else:
|
||||
_parsed_body = await _read_request_body(request)
|
||||
if state_raw_body is None:
|
||||
# A non-object JSON body reads as ``{}``, so forwarding the parsed body would
|
||||
# send an empty object in place of the caller's own provider payload.
|
||||
state_raw_body = await non_object_raw_body(request)
|
||||
verbose_proxy_logger.debug(
|
||||
"Pass through endpoint sending request to \nURL %s\nheaders: %s\nbody: %s\n",
|
||||
url,
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ from litellm.proxy.common_utils.http_parsing_utils import (
|
|||
get_tags_from_request_body,
|
||||
numeric_form_fields,
|
||||
populate_request_with_path_params,
|
||||
non_object_raw_body,
|
||||
read_raw_json_body,
|
||||
)
|
||||
|
||||
|
|
@ -539,16 +540,53 @@ async def test_non_object_body_is_not_cached_for_later_readers():
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_object_body_still_reaches_the_provider_verbatim():
|
||||
"""Passthrough forwards the raw bytes, not the parsed body, so coercing the parsed view
|
||||
must not change what a provider actually receives."""
|
||||
body = b'[{"role": "user", "content": "hi"}]'
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
pytest.param(b'[{"role": "user", "content": "hi"}]', id="array"),
|
||||
pytest.param(b"123", id="number"),
|
||||
pytest.param(b'"gpt-4o"', id="string"),
|
||||
pytest.param(b"true", id="boolean"),
|
||||
pytest.param(b"null", id="null"),
|
||||
],
|
||||
)
|
||||
async def test_non_object_body_is_recoverable_verbatim_for_forwarding(body: bytes):
|
||||
"""Passthrough forwards ``_parsed_body`` as JSON unless it is handed exact bytes, so the
|
||||
coerced ``{}`` would reach the provider in place of the caller's payload. The original
|
||||
bytes must stay recoverable."""
|
||||
request = _starlette_request(body, "application/json")
|
||||
|
||||
assert await _read_request_body(request) == {}
|
||||
assert await non_object_raw_body(request) == body
|
||||
assert await read_raw_json_body(request) == body
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
pytest.param(b'{"model": "gpt-4o"}', id="object"),
|
||||
pytest.param(b"{}", id="empty-object"),
|
||||
pytest.param(b"", id="empty-body"),
|
||||
],
|
||||
)
|
||||
async def test_object_body_is_not_offered_for_raw_forwarding(body: bytes):
|
||||
"""Only a coerced body may bypass ``json=_parsed_body``: an object body must keep going
|
||||
through the parsed path, so hooks that mutate it are still what gets sent."""
|
||||
request = _starlette_request(body, "application/json")
|
||||
|
||||
assert isinstance(await _read_request_body(request), dict)
|
||||
assert await non_object_raw_body(request) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_raw_forwarding_before_the_body_has_been_read():
|
||||
"""Nothing may be marked for raw forwarding until a read has established the body is
|
||||
actually a non-object."""
|
||||
assert await non_object_raw_body(_starlette_request(b"[]", "application/json")) is None
|
||||
assert await non_object_raw_body(None) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body, expected",
|
||||
|
|
|
|||
|
|
@ -7674,3 +7674,68 @@ async def test_object_passthrough_body_still_yields_its_envelope_fields():
|
|||
assert query_params_data == {"alt": "sse"}
|
||||
assert custom_body_data == {"model": "x"}
|
||||
assert stream is True
|
||||
|
||||
|
||||
async def _capture_upstream_request(body: bytes) -> httpx.Request:
|
||||
"""Drive ``pass_through_request`` for ``body`` and return the request it built for the
|
||||
provider, by letting a real httpx client encode it and failing the send."""
|
||||
captured: list[httpx.Request] = [] # mutable-ok: the send double records what it was given
|
||||
|
||||
async def _send(req, **kwargs):
|
||||
captured.append(req)
|
||||
raise httpx.HTTPError("stop after capture")
|
||||
|
||||
real_client = httpx.AsyncClient()
|
||||
real_client.send = _send
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.proxy_logging_obj") as mock_proxy_logging,
|
||||
patch(
|
||||
"litellm.proxy.pass_through_endpoints.pass_through_endpoints.get_async_httpx_client"
|
||||
) as mock_get_client,
|
||||
):
|
||||
mock_proxy_logging.pre_call_hook = AsyncMock(side_effect=lambda **kwargs: kwargs["data"])
|
||||
mock_get_client.return_value = SimpleNamespace(client=real_client)
|
||||
|
||||
# The failure handler runs on a MagicMock proxy_logging_obj and dies with TypeError,
|
||||
# the same way test_pass_through_request_uses_resolved_timeout relies on.
|
||||
with pytest.raises(TypeError):
|
||||
await pass_through_request(
|
||||
request=_json_request(body),
|
||||
target="http://upstream.test/v1/messages",
|
||||
custom_headers={},
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="sk-test"),
|
||||
)
|
||||
|
||||
assert captured, "pass_through_request never built an upstream request"
|
||||
return captured[0]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
pytest.param(b'[{"role": "user", "content": "hi"}]', id="array"),
|
||||
pytest.param(b"[1, 2, 3]", id="array-of-numbers"),
|
||||
pytest.param(b"123", id="number"),
|
||||
pytest.param(b'"claude-sonnet-4-5"', id="string"),
|
||||
pytest.param(b"true", id="boolean"),
|
||||
pytest.param(b"null", id="null"),
|
||||
],
|
||||
)
|
||||
async def test_non_object_body_is_forwarded_to_the_provider_verbatim(body: bytes):
|
||||
"""A passthrough body is the caller's own provider payload. The parsed view of a non-object
|
||||
body is ``{}``, and the forwarding path sends the parsed body as JSON, so without the raw
|
||||
bytes the provider silently received ``{}`` in place of what the caller sent."""
|
||||
upstream = await _capture_upstream_request(body)
|
||||
|
||||
assert upstream.content == body
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_object_body_is_still_forwarded_from_the_parsed_view():
|
||||
"""Hooks mutate the parsed body and those mutations must still reach the provider, so an
|
||||
object body must not take the raw-bytes path."""
|
||||
upstream = await _capture_upstream_request(b'{"model": "claude-sonnet-4-5"}')
|
||||
|
||||
assert json.loads(upstream.content)["model"] == "claude-sonnet-4-5"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue