Skip to content

fix(proxy): preserve multipart/form-data boundary in passthrough endpoints #23338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Sameerlite merged 2 commits into from
Mar 11, 2026
Merged

fix(proxy): preserve multipart/form-data boundary in passthrough endpoints #23338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2fca11
fix(proxy): preserve multipart/form-data boundary in passthrough endp…
Sameerlite Mar 11, 2026
8e32919
Merge branch 'main' into litellm_fix_multipart_passthrough
Sameerlite Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 28 additions & 31 deletions litellm/proxy/pass_through_endpoints/pass_through_endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,9 @@ async def non_streaming_http_request_handler(
headers=headers,
params=requested_query_params,
)
elif HttpPassThroughEndpointHelpers.is_multipart(request) is True:
elif HttpPassThroughEndpointHelpers.is_multipart(request) is True and not _parsed_body:
# Only use multipart handler if we don't have a parsed body
# (parsed body means it was JSON despite multipart content-type header)
return await HttpPassThroughEndpointHelpers.make_multipart_http_request(
request=request,
async_client=async_client,
Expand Down Expand Up @@ -677,8 +679,15 @@ async def pass_through_request( # noqa: PLR0915
str(url)
)

# Skip body parsing for multipart requests - make_multipart_http_request will handle it
# But if custom_body is provided (e.g., JSON parsed despite multipart content-type), use it
is_multipart = HttpPassThroughEndpointHelpers.is_multipart(request) and not custom_body

if custom_body:
_parsed_body = custom_body
elif is_multipart:
# Don't parse multipart body here - it will be handled by make_multipart_http_request
_parsed_body = {}
Comment on lines +688 to +690

@greptile-apps greptile-apps Bot Mar 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty dict sentinel for multipart is fragile and leaks into the downstream request

Setting _parsed_body = {} as a sentinel value for "don't send a body" is unreliable because:

  1. Lines 707–713 will inject metadata.guardrails into this dict when guardrails are configured.
  2. Line 736 always injects litellm_logging_obj, making the dict non-empty.
  3. If the not _parsed_body check in non_streaming_http_request_handler is ever bypassed (see the comment on line 736), these internal keys will be forwarded to the upstream service as JSON.

A safer approach is to keep _parsed_body = None for multipart and propagate an explicit boolean:

elif is_multipart:
    _parsed_body = None   # leave body untouched; make_multipart_http_request reads it

Then guard the litellm_logging_obj injection against None, and pass an explicit _is_multipart flag to non_streaming_http_request_handler rather than relying on the emptiness of _parsed_body.

else:
_parsed_body = await _read_request_body(request)
verbose_proxy_logger.debug(
Expand Down Expand Up @@ -1043,30 +1052,22 @@ async def _parse_request_data_by_content_type(
# Handle requests with no body (e.g., DELETE requests)
pass
elif "multipart/form-data" in content_type:
# ✅ Handle multipart form-data
form = await request.form()
if "query_params" in form:
form_value = form["query_params"]
if isinstance(form_value, str):
try:
query_params_data = json.loads(form_value)
except Exception:
query_params_data = form_value
else:
query_params_data = form_value

if "custom_body" in form:
form_value = form["custom_body"]
if isinstance(form_value, str):
try:
custom_body_data = json.loads(form_value)
except Exception:
custom_body_data = form_value
else:
custom_body_data = form_value

if "file" in form:
file_data = form["file"] # this is a Starlette UploadFile object
# ✅ 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()
# 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
except (json.JSONDecodeError, Exception):
# Not JSON - this is actual multipart data
# Skip parsing here to avoid consuming the request body stream
# make_multipart_http_request will handle it
pass
Comment on lines +1066 to +1070

@greptile-apps greptile-apps Bot Mar 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overly-broad exception clause silences unexpected errors

except (json.JSONDecodeError, Exception) is redundant — Exception already catches json.JSONDecodeError — and it swallows every possible exception (e.g. MemoryError, RuntimeError, network-level errors on request.body()). Any real error reading the body will silently be treated as "not JSON, skip", making failures invisible.

Suggested change
except (json.JSONDecodeError, Exception):
# Not JSON - this is actual multipart data
# Skip parsing here to avoid consuming the request body stream
# make_multipart_http_request will handle it
pass
except (json.JSONDecodeError, ValueError):
# Not JSON - this is actual multipart data
# Skip parsing here to avoid consuming the request body stream
# make_multipart_http_request will handle it
pass

Comment on lines +1055 to +1070

@greptile-apps greptile-apps Bot Mar 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempting request.json() on real multipart bodies is semantically wrong and still consumes the body

The comment says "Try to parse as JSON first (handles misconfigured clients sending JSON with multipart content-type)", but this approach has two problems:

  1. Stream consumption: request.json() calls await request.body() internally. Even though Starlette caches the raw bytes, the call still reads the entire body into memory before make_multipart_http_request can stream it via request.form(). For large file uploads this causes a full in-memory copy of the file regardless of the code path taken.

  2. Incorrect fallback assignment: When JSON parsing succeeds, line 1064 sets custom_body_data = body (the entire parsed JSON object). This means any JSON request arriving with a multipart/form-data content-type header will have its entire body treated as custom_body, overriding whatever the caller intended. The original code isolated specific keys (query_params, custom_body, stream); this new path does not.

A safer approach: only run the JSON-fallback path if the body starts with { or [ (fast check without full parse), or require that clients send the correct content-type rather than trying to paper over misconfigured clients in the proxy layer.


elif "application/x-www-form-urlencoded" in content_type:
# ✅ Handle URL-encoded form data
Expand Down Expand Up @@ -1132,7 +1133,6 @@ async def endpoint_func( # type: ignore
fastapi_response: Response,
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
subpath: str = "", # captures sub-paths when include_subpath=True
custom_body: Optional[dict] = None,
):
from litellm.proxy.pass_through_endpoints.pass_through_endpoints import (
InitPassThroughEndpointHelpers,
Expand Down Expand Up @@ -1208,12 +1208,9 @@ async def endpoint_func( # type: ignore
)
if query_params:
final_query_params.update(query_params)
# When a caller (e.g. bedrock_proxy_route) supplies a pre-built
# body, use it instead of the body parsed from the raw request.
# Use the body parsed from the raw request
final_custom_body: Optional[dict] = None
if custom_body is not None:
final_custom_body = custom_body
elif isinstance(custom_body_data, dict):
if isinstance(custom_body_data, dict):
final_custom_body = custom_body_data

return await pass_through_request( # type: ignore
Expand Down
104 changes: 104 additions & 0 deletions tests/test_litellm/proxy/pass_through_endpoints/test_pass_through_endpoints.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2369,3 +2369,107 @@ def test_get_registered_pass_through_route_with_custom_root():

# Clean up
_registered_pass_through_routes.clear()


def test_mapped_pass_through_routes_with_server_root_path():
"""
Mapped passthrough routes (vertex_ai, bedrock, etc) should match
even when SERVER_ROOT_PATH is set and the incoming route is prefixed.

Regression test for https://github.com/BerriAI/litellm/issues/22272
"""
from litellm.proxy.pass_through_endpoints.pass_through_endpoints import (
InitPassThroughEndpointHelpers,
)

with patch(
"litellm.proxy.pass_through_endpoints.pass_through_endpoints.get_server_root_path"
) as mock_get_root:
mock_get_root.return_value = "/litellm"

# prefixed route should match mapped routes like /vertex_ai
assert (
InitPassThroughEndpointHelpers.is_registered_pass_through_route(
"/litellm/vertex_ai/v1/projects/foo"
)
is True
)
assert (
InitPassThroughEndpointHelpers.is_registered_pass_through_route(
"/litellm/bedrock/model/invoke"
)
is True
)

# bare route without prefix should not match when root is set
assert (
InitPassThroughEndpointHelpers.is_registered_pass_through_route(
"/vertex_ai/v1/projects/foo"
)
is False
)
Comment on lines +2390 to +2410

@greptile-apps greptile-apps Bot Mar 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test assertions are inverted — test will fail CI

The assertions in test_mapped_pass_through_routes_with_server_root_path are testing behaviour that doesn't exist in the current implementation of is_registered_pass_through_route.

Looking at is_registered_pass_through_route (line 2061–2063 in the production file):

for mapped_route in LiteLLMRoutes.mapped_pass_through_routes.value:
    if route.startswith(mapped_route):   # NO root-path prefix applied here
        return True

The mapped-route check uses raw startswith(mapped_route) without ever calling _build_full_path_with_root. This means:

  • "/litellm/vertex_ai/v1/projects/foo".startswith("/vertex_ai") → False — but the test asserts True
  • "/litellm/bedrock/model/invoke".startswith("/bedrock") → False — but the test asserts True
  • "/vertex_ai/v1/projects/foo".startswith("/vertex_ai") → True — but the test asserts False

All three assertions are wrong relative to the actual implementation. The test would fail on every CI run.

The fix would need to either:

  1. Update is_registered_pass_through_route to prefix mapped routes with _build_full_path_with_root (implementing the intended behaviour), or
  2. Correct the assertions to match what the code actually does (if the test was written against the wrong spec).



@pytest.mark.asyncio
async def test_multipart_passthrough_preserves_boundary():
"""
Test that multipart/form-data requests through passthrough preserve the boundary
and can be correctly parsed by the upstream server.

Regression test for multipart boundary stripping issue.
"""
from io import BytesIO

# Mock the httpx request to verify files are passed correctly
mock_response = MagicMock()
mock_response.status_code = 200
mock_response.headers = httpx.Headers({"content-type": "application/json"})
mock_response.aread = AsyncMock(return_value=b'{"filename": "test.txt", "size": 17}')
mock_response.text = '{"filename": "test.txt", "size": 17}'

async def mock_httpx_request(method, url, **kwargs):
# Verify that files parameter is passed (not json)
assert "files" in kwargs, "Files should be passed for multipart requests"
assert "file" in kwargs["files"], "File field should be in files dict"

# Verify content-type is NOT in headers (httpx will set it with correct boundary)
headers = kwargs.get("headers", {})
assert "content-type" not in headers, "content-type should be removed for multipart"

filename, content, content_type = kwargs["files"]["file"]
assert filename == "test.txt"
assert content == b"test file content"
assert content_type == "text/plain"

return mock_response

async_client = MagicMock()
async_client.request = AsyncMock(side_effect=mock_httpx_request)

# Create mock request
request = MagicMock(spec=Request)
request.method = "POST"
Comment on lines +2447 to +2451

@greptile-apps greptile-apps Bot Mar 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

content-type assertion passes trivially

The test passes headers={} (empty dict) into make_multipart_http_request. Inside the implementation, headers_copy.pop("content-type", None) is a no-op on an empty dict, so the assertion assert "content-type" not in headers will always be True regardless of whether the stripping logic is actually working.

To give this assertion real coverage, the headers argument should be populated with a "content-type" key before it is passed in, so the test actually verifies that the header is removed by make_multipart_http_request.

request.headers = Headers({"content-type": "multipart/form-data; boundary=test123"})

# Mock form data
file_content = b"test file content"
file = BytesIO(file_content)
headers = Headers({"content-type": "text/plain"})
upload_file = UploadFile(file=file, filename="test.txt", headers=headers)
upload_file.read = AsyncMock(return_value=file_content)

form_data = {"file": upload_file}
request.form = AsyncMock(return_value=form_data)

# Test the multipart handler directly
response = await HttpPassThroughEndpointHelpers.make_multipart_http_request(
request=request,
async_client=async_client,
url=httpx.URL("http://test.com/upload"),
headers={},
requested_query_params=None,
)

# Verify the response
assert response.status_code == 200
Comment on lines +2464 to +2474

@greptile-apps greptile-apps Bot Mar 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test bypasses the broken code path — pass_through_request flow is untested

The test calls make_multipart_http_request directly, skipping the entire pass_through_request → non_streaming_http_request_handler pipeline where the routing bug lives. A test that exercises only the leaf function will pass even when the top-level fix is broken (as is the case here).

To provide real regression coverage, the test should either:

  1. Call pass_through_request end-to-end (mocking out proxy_logging_obj, user_api_key_auth, etc.), or
  2. At minimum, exercise non_streaming_http_request_handler with a _parsed_body that has already been mutated the way pass_through_request mutates it (i.e. with litellm_logging_obj present), and assert that make_multipart_http_request is still dispatched.

async_client.request.assert_called_once()
Loading