mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(proxy): read a non-object JSON body as no fields instead of crashing
A body that is valid JSON but not an object (`[]`, `123`, `"s"`, `true`,
`null`) parsed into a list or scalar that `_read_request_body` returned
under its `-> dict` annotation. Auth reads the body before any route does
and treats it as a mapping, so it raised AttributeError in
`pre_db_read_auth_checks`, then again inside
`_handle_authentication_error` while turning the first one into a
response. With the handler itself broken the caller got a bare 500 and no
hint of what was wrong, plus a prefilled "file a bug report" link for
their own mistake.
Honour the annotation: a non-object body reads as `{}`, the same as an
empty body, since every field a caller looks for is a key and a
non-object body carries none of them. The route's own validation then
returns its usual 400.
Coerce rather than reject, because a passthrough body is the caller's own
provider payload and need not be an object, as
`is_passthrough_request_streaming` documents. Raw forwarding re-reads
`request.body()`, so the real payload still reaches the provider
untouched.
Guard the second crash site too: `_parse_request_data_by_content_type`
reads the body itself and looked for envelope fields on whatever
`request.json()` returned.
Fixes #43711
This commit is contained in:
parent
684a1edd44
commit
e459979400
5 changed files with 183 additions and 15 deletions
|
|
@ -127,7 +127,14 @@ async def _read_request_body(request: Request | None) -> dict:
|
|||
- request: The request object to read the body from
|
||||
|
||||
Returns:
|
||||
- dict: Parsed request data as a dictionary or an empty dictionary if parsing fails
|
||||
- dict: Parsed request data as a dictionary. A body that is valid JSON but not an
|
||||
object reads as ``{}``, the same as an empty body: every field a caller can look
|
||||
for is a key, so a non-object body carries none of them. Honouring the annotation
|
||||
matters because auth reads the body before any route does, and both read it with
|
||||
``.get(...)``, so returning a list here surfaced as AttributeError -> 500 (#43711)
|
||||
|
||||
Raises:
|
||||
- ProxyException: 400, when the body is present but malformed
|
||||
"""
|
||||
try:
|
||||
if request is None:
|
||||
|
|
@ -208,9 +215,11 @@ async def _read_request_body(request: Request | None) -> dict:
|
|||
code=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
|
||||
# Cache the parsed result
|
||||
_safe_set_request_parsed_body(request=request, parsed_body=parsed_body)
|
||||
return parsed_body
|
||||
# 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
|
||||
|
||||
except (json.JSONDecodeError, orjson.JSONDecodeError, ProxyException) as e:
|
||||
# Re-raise ProxyException as-is
|
||||
|
|
|
|||
|
|
@ -1914,6 +1914,13 @@ class _PassThroughRequestEnvelope(TypedDict, total=False):
|
|||
stream: bool | None
|
||||
|
||||
|
||||
def _passthrough_envelope(body: object) -> _PassThroughRequestEnvelope | None:
|
||||
"""``request.json()`` returns any JSON kind, and a passthrough body is the caller's own
|
||||
provider payload, so a list or scalar is legitimate here and simply carries no envelope
|
||||
fields. Reading it as one raised AttributeError -> 500 (#43711)."""
|
||||
return body if isinstance(body, dict) else None
|
||||
|
||||
|
||||
async def _parse_request_data_by_content_type(
|
||||
request: Request,
|
||||
) -> tuple[object, object, None, bool | None]:
|
||||
|
|
@ -1935,10 +1942,11 @@ async def _parse_request_data_by_content_type(
|
|||
if "application/json" in content_type:
|
||||
# ✅ Handle JSON
|
||||
try:
|
||||
body: _PassThroughRequestEnvelope = await request.json()
|
||||
query_params_data = body.get("query_params")
|
||||
custom_body_data = body.get("custom_body")
|
||||
stream = body.get("stream")
|
||||
body: _PassThroughRequestEnvelope | None = _passthrough_envelope(await request.json())
|
||||
if body is not None:
|
||||
query_params_data = body.get("query_params")
|
||||
custom_body_data = body.get("custom_body")
|
||||
stream = body.get("stream")
|
||||
except json.JSONDecodeError:
|
||||
# Handle requests with no body (e.g., DELETE requests)
|
||||
pass
|
||||
|
|
@ -1946,14 +1954,15 @@ async def _parse_request_data_by_content_type(
|
|||
# ✅ Try to parse as JSON first (handles misconfigured clients sending JSON with multipart content-type)
|
||||
# If that fails, skip parsing - pass_through_request will handle actual multipart
|
||||
try:
|
||||
body = await request.json()
|
||||
body = _passthrough_envelope(await request.json())
|
||||
# Successfully parsed as JSON - treat as JSON body
|
||||
query_params_data = body.get("query_params")
|
||||
custom_body_data = body.get("custom_body")
|
||||
stream = body.get("stream")
|
||||
# If custom_body is not set, use the entire body
|
||||
if custom_body_data is None and body:
|
||||
custom_body_data = body
|
||||
if body is not None:
|
||||
query_params_data = body.get("query_params")
|
||||
custom_body_data = body.get("custom_body")
|
||||
stream = body.get("stream")
|
||||
# If custom_body is not set, use the entire body
|
||||
if custom_body_data is None and body:
|
||||
custom_body_data = body
|
||||
except (json.JSONDecodeError, Exception):
|
||||
# Not JSON - this is actual multipart data
|
||||
# Skip parsing here to avoid consuming the request body stream
|
||||
|
|
|
|||
|
|
@ -53,6 +53,7 @@ from litellm.proxy.auth.user_api_key_auth import (
|
|||
_ensure_parent_otel_span_on_request_state,
|
||||
_PendingAutoRegister,
|
||||
_matches_routing_override,
|
||||
_read_request_body_deferring_parse_failure,
|
||||
_reserve_budget_after_common_checks,
|
||||
_route_requires_auth_despite_public,
|
||||
_routing_selector_matches_claim,
|
||||
|
|
@ -6419,6 +6420,43 @@ async def test_user_api_key_auth_authenticates_before_raising_malformed_body_err
|
|||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
pytest.param(b"[]", id="array"),
|
||||
pytest.param(b'[{"role": "user"}]', id="array-of-objects"),
|
||||
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_auth_reads_a_non_object_body_as_no_fields(body: bytes):
|
||||
"""Auth reads the body before any route does and then treats it as a mapping, in
|
||||
``pre_db_read_auth_checks`` and again in ``populate_request_with_path_params``. A body
|
||||
that parsed to a list/int/str raised AttributeError in both, and in the handler meant
|
||||
to turn the first one into a response, so the caller got a bare 500 (#43711)."""
|
||||
from fastapi import Request
|
||||
from starlette.datastructures import URL
|
||||
|
||||
request = Request(
|
||||
scope={
|
||||
"type": "http",
|
||||
"headers": [(b"content-type", b"application/json")],
|
||||
"method": "POST",
|
||||
"path_params": {"vector_store_id": "vs-1"},
|
||||
}
|
||||
)
|
||||
request._url = URL(url="/v1/vector_stores/vs-1/search")
|
||||
request._body = body
|
||||
|
||||
request_data, parse_exception = await _read_request_body_deferring_parse_failure(request=request)
|
||||
|
||||
assert parse_exception is None
|
||||
assert request_data == {"vector_store_id": "vs-1", "vector_store_ids": ["vs-1"]}
|
||||
|
||||
|
||||
async def _run_auth_with_malformed_body(post_call_failure_hook):
|
||||
"""Drive ``user_api_key_auth`` for an authenticated caller whose body never parses,
|
||||
with ``proxy_logging_obj.post_call_failure_hook`` swapped for the passed double.
|
||||
|
|
|
|||
|
|
@ -504,6 +504,66 @@ def _make_json_request(body: bytes) -> MagicMock:
|
|||
return mock_request
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
pytest.param(b"[]", id="empty-array"),
|
||||
pytest.param(b'[{"model": "gpt-4o"}]', id="array-of-objects"),
|
||||
pytest.param(b"123", id="integer"),
|
||||
pytest.param(b"1.5", id="float"),
|
||||
pytest.param(b'"gpt-4o"', id="string"),
|
||||
pytest.param(b"true", id="boolean"),
|
||||
pytest.param(b"null", id="null"),
|
||||
],
|
||||
)
|
||||
async def test_non_object_json_body_reads_as_no_fields(body: bytes):
|
||||
"""
|
||||
``orjson.loads`` returns whatever JSON kind it found, so this ``-> dict`` used to hand
|
||||
back a list/int/str. Auth reads the body before any route does, and both read it with
|
||||
``.get(...)``, so that surfaced as AttributeError -> 500 (#43711). Every field a caller
|
||||
looks for is a key, so a non-object body carries none of them and reads as ``{}``.
|
||||
"""
|
||||
assert await _read_request_body(_make_json_request(body)) == {}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_object_body_is_not_cached_for_later_readers():
|
||||
"""The coercion only helps if a second read, or a route reading the cache after auth,
|
||||
cannot pull the list back out and call ``.get()`` on it."""
|
||||
request = _starlette_request(b"[1, 2, 3]", "application/json")
|
||||
|
||||
assert await _read_request_body(request) == {}
|
||||
assert _safe_get_request_parsed_body(request=request) == {}
|
||||
assert await _read_request_body(request) == {}
|
||||
|
||||
|
||||
@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"}]'
|
||||
request = _starlette_request(body, "application/json")
|
||||
|
||||
assert await _read_request_body(request) == {}
|
||||
assert await read_raw_json_body(request) == body
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body, expected",
|
||||
[
|
||||
pytest.param(b"{}", {}, id="empty-object"),
|
||||
pytest.param(b"", {}, id="empty-body"),
|
||||
pytest.param(b'{"model": "gpt-4o", "n": [1, 2]}', {"model": "gpt-4o", "n": [1, 2]}, id="object"),
|
||||
],
|
||||
)
|
||||
async def test_object_bodies_are_untouched(body: bytes, expected: dict):
|
||||
"""The coercion must only ever fire on a non-object body: an object's own list values
|
||||
stay exactly as the caller sent them."""
|
||||
assert await _read_request_body(_make_json_request(body)) == expected
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_surrogate_repair_skipped_above_size_limit(monkeypatch):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ from litellm.proxy.pass_through_endpoints.pass_through_endpoints import (
|
|||
LITELLM_PASS_THROUGH_CUSTOM_BODY_STATE_KEY,
|
||||
HttpPassThroughEndpointHelpers,
|
||||
InitPassThroughEndpointHelpers,
|
||||
_parse_request_data_by_content_type,
|
||||
_registered_pass_through_routes,
|
||||
_truncate_upstream_error_body,
|
||||
_with_trace_context,
|
||||
|
|
@ -7622,3 +7623,54 @@ def test_passthrough_attributes_a_cli_session_to_its_alias_not_the_login_token()
|
|||
metadata = kwargs["litellm_params"]["metadata"]
|
||||
assert metadata["user_api_key"] == "cli-session-alice"
|
||||
assert _get_spend_logs_metadata(metadata)["user_api_key"] == "cli-session-alice"
|
||||
|
||||
|
||||
def _json_request(body: bytes, content_type: str = "application/json") -> Request:
|
||||
async def receive():
|
||||
return {"type": "http.request", "body": body, "more_body": False}
|
||||
|
||||
return Request(
|
||||
{
|
||||
"type": "http",
|
||||
"method": "POST",
|
||||
"path": "/anthropic/v1/messages",
|
||||
"headers": [(b"content-type", content_type.encode())],
|
||||
"query_string": b"",
|
||||
},
|
||||
receive,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"body",
|
||||
[
|
||||
pytest.param(b"[]", id="empty-array"),
|
||||
pytest.param(b'[{"role": "user", "content": "hi"}]', id="array-of-objects"),
|
||||
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_passthrough_body_carries_no_envelope_fields(body: bytes):
|
||||
"""A passthrough body is the caller's own provider payload, so it need not be a JSON
|
||||
object. Reading a list or scalar as the query_params/custom_body envelope raised
|
||||
AttributeError and the caller got a bare 500 (#43711)."""
|
||||
query_params_data, custom_body_data, file_data, stream = await _parse_request_data_by_content_type(
|
||||
_json_request(body)
|
||||
)
|
||||
|
||||
assert (query_params_data, custom_body_data, file_data, stream) == (None, None, None, None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_object_passthrough_body_still_yields_its_envelope_fields():
|
||||
"""The guard must only fire on a non-object body: a real envelope is unaffected."""
|
||||
body = json.dumps({"query_params": {"alt": "sse"}, "custom_body": {"model": "x"}, "stream": True}).encode()
|
||||
|
||||
query_params_data, custom_body_data, _, stream = await _parse_request_data_by_content_type(_json_request(body))
|
||||
|
||||
assert query_params_data == {"alt": "sse"}
|
||||
assert custom_body_data == {"model": "x"}
|
||||
assert stream is True
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue