fix(proxy): decode multipart user_config JSON#26764
Conversation
|
Kcstring seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Greptile SummaryThis PR fixes multipart form-data handling by adding a Confidence Score: 4/5Safe to merge; changes are well-scoped and backwards-compatible for valid inputs. All findings are P2. The logic is correct for the happy path and the refactor of the existing No files require special attention; the two changed files are straightforward.
|
| Filename | Overview |
|---|---|
| litellm/proxy/common_utils/http_parsing_utils.py | Adds _parse_form_json_field helper and applies it to user_config and tags form fields; refactors existing metadata handling to use the same helper. Minor edge-case risk: malformed JSON that starts with {/[ propagates a json.JSONDecodeError rather than falling back gracefully. |
| tests/test_litellm/proxy/common_utils/test_multipart_json_form_fields.py | New test file covering happy-path JSON decoding for user_config/tags and preservation of plain-string tags. Missing coverage for the error path (malformed JSON-shaped values). |
Reviews (1): Last reviewed commit: "fix(proxy): decode multipart user_config..." | Re-trigger Greptile
| if require_json or value.lstrip().startswith(("{", "[")): | ||
| parsed_body[field_name] = json.loads(value) |
There was a problem hiding this comment.
json.JSONDecodeError for malformed-but-JSON-shaped values
If user_config or tags starts with { or [ but is not valid JSON, json.loads raises json.JSONDecodeError. This propagates out of _read_request_body via the outer re-raise handler, which turns the entire request into a 500/error response instead of a helpful 400 message. Before this PR these fields were simply left as-is, so the failure mode is new. Consider wrapping the parse in a try/except and either falling back to the original string value or raising a clear ProxyException.
if require_json or value.lstrip().startswith("{", "[")):
try:
parsed_body[field_name] = json.loads(value)
except json.JSONDecodeError:
if require_json:
raise
# leave the original string value for non-required fields| @pytest.mark.asyncio | ||
| async def test_form_data_with_plain_string_tags_is_left_unchanged(): | ||
| mock_request = MagicMock() | ||
| test_data = {"model": "whisper-1", "tags": "production"} | ||
|
|
||
| mock_request.form = AsyncMock(return_value=test_data) | ||
| mock_request.headers = {"content-type": "multipart/form-data"} | ||
| mock_request.scope = {} | ||
| mock_request.state._cached_headers = None | ||
|
|
||
| result = await _read_request_body(mock_request) | ||
|
|
||
| assert result["tags"] == "production" |
There was a problem hiding this comment.
There is no test for the case where user_config or tags starts with {/[ but contains invalid JSON. Adding such a test would lock in the intended error behaviour (whether that is a raised exception or a graceful fallback) and prevent future regressions.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…(5 PRs) - BerriAI/litellm#26764 merge-after-nits: multipart user_config/tags JSON decode - BerriAI/litellm#26769 request-changes: FuturMix add bundles unrelated deletions - google-gemini/gemini-cli#26184 merge-after-nits: deleteSession fail-loud on missing file - QwenLM/qwen-code#3737 merge-after-nits: drop strip-thoughts helpers, preserve reasoning_content - aaif-goose/goose#8781 needs-discussion: ACP per-connection inline→spawn ordering question
Fixes #26707
Summary
user_configform fields in multipart requests before routing.tagsarrays from multipart requests.tagsvalues unchanged to avoid breaking existing form clients.Verification
python3 -m pytest tests/test_litellm/proxy/common_utils/test_multipart_json_form_fields.pypython3 -m pytest tests/test_litellm/proxy/common_utils/test_http_parsing_utils.py -k "form_data_with_json_metadata or form_data_with_invalid_json_metadata"python3 -m pytest tests/test_litellm/proxy/common_utils/test_http_parsing_utils.py tests/test_litellm/proxy/common_utils/test_multipart_json_form_fields.pyruff check litellm/proxy/common_utils/http_parsing_utils.py tests/test_litellm/proxy/common_utils/test_multipart_json_form_fields.pygit diff --check