Skip to content

fix(httpx): sanitize unread multipart headers in masked errors - #26633

Open
samrusani wants to merge 2 commits into
BerriAI:litellm_internal_stagingfrom
samrusani:fix/issue-26552-image-edit-mask-stream
Open

fix(httpx): sanitize unread multipart headers in masked errors#26633
samrusani wants to merge 2 commits into
BerriAI:litellm_internal_stagingfrom
samrusani:fix/issue-26552-image-edit-mask-stream

Conversation

@samrusani

@samrusani samrusani commented Apr 27, 2026

Copy link
Copy Markdown

Relevant issues

Follow-up to #26718 and #26552.

#26718 fixed the RequestNotRead exception when a masked HTTP error is rebuilt from an unread streaming request. This PR addresses the remaining request-metadata inconsistency in that fallback.

Pre-Submission checklist

  • I have added meaningful tests
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 on the current rebased head (the earlier review scored 5/5; a new review was requested after rebasing)

Screenshots / Proof of Fix

For an unread multipart image-edit request, the masked error wrapper must replace the unavailable body with b"". Before this follow-up, the rebuilt request retained the original multipart Content-Type, Content-Length, and transfer metadata, so the diagnostic request described a multipart payload that was no longer attached.

The regression test constructs a real httpx multipart request with both image[] and mask streams and verifies that the rebuilt masked request:

  • removes the secret from the URL
  • has an empty body
  • reports Content-Length: 0
  • does not retain multipart Content-Type or transfer metadata

No real provider call is required for this error-path test.

Type

Bug Fix

Test

Changes

  • Extract masked-request reconstruction into a focused helper.
  • When request content is unread, use an empty body and remove entity headers that would describe the discarded multipart stream.
  • Preserve existing behavior for readable request bodies and URL masking.
  • Add focused multipart image-edit coverage for proxy image + mask handling and proxy-style BytesIO masks.

Tests

uv run --no-sync pytest tests/test_litellm/llms/custom_httpx/test_credential_leak_prevention.py tests/test_litellm/llms/openai/test_openai_image_edit_transformation.py -q
# 39 passed

uv run --no-sync pytest tests/test_litellm/proxy/image_endpoints/test_azure_routes.py -q
# 3 passed

uv run --no-sync ruff format --check litellm/llms/custom_httpx/http_handler.py tests/test_litellm/llms/custom_httpx/test_credential_leak_prevention.py tests/test_litellm/llms/openai/test_openai_image_edit_transformation.py tests/test_litellm/proxy/image_endpoints/test_azure_routes.py
# passed

uv run --no-sync ruff check litellm/llms/custom_httpx/http_handler.py tests/test_litellm/llms/custom_httpx/test_credential_leak_prevention.py tests/test_litellm/llms/openai/test_openai_image_edit_transformation.py tests/test_litellm/proxy/image_endpoints/test_azure_routes.py
# passed

git diff --check origin/litellm_internal_staging...HEAD
# passed

Current GitHub checks pass except the repository-wide OSV scan, which reports existing vulnerabilities in lockfiles this PR does not modify.

Scope

This intentionally does not change provider request construction, retries, transport behavior, or image-edit feature support.

@samrusani

Copy link
Copy Markdown
Author

@greptileai

@greptile-apps

greptile-apps Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts the masked-request reconstruction in MaskedHTTPStatusError into a dedicated _build_masked_request helper, and fixes a metadata inconsistency where the rebuilt request retained stale multipart Content-Type, Content-Length, and transfer headers even after the body was replaced with b"" in the RequestNotRead fallback.

  • _build_masked_request now pops content-length, transfer-encoding, and content-type before building the empty-body request, so the diagnostic request no longer claims to carry a multipart payload it does not have.
  • New focused tests verify the multipart unread-body path (test_handles_unread_multipart_request_content), proxy BytesIO mask handling, and the proxy /v1/images/edits route with both image and mask fields — all using mocks, no real network calls.

Confidence Score: 5/5

  • This PR is safe to merge. The change is tightly scoped to the error-path request reconstruction and does not touch provider logic, retries, or transport behavior.
  • The fix is small and self-contained: a helper replaces an inline block, the header-stripping logic is straightforward and directly verified by new assertions, and all existing tests are preserved unchanged. The readable-body code path is unaffected. No production call path changes.
  • No files require special attention.

Important Files Changed

Filename Overview
litellm/llms/custom_httpx/http_handler.py Extracts masked request construction into _build_masked_request; correctly pops content-length, transfer-encoding, and content-type in the RequestNotRead fallback so the rebuilt request no longer describes a multipart payload that was never attached.
tests/test_litellm/llms/custom_httpx/test_credential_leak_prevention.py Adds test_handles_unread_multipart_request_content that constructs a real multipart httpx request and asserts the masked rebuild strips the URL secret, zeroes the body, sets content-length: 0, and removes content-type. All assertions match the new helper's behaviour.
tests/test_litellm/llms/openai/test_openai_image_edit_transformation.py Adds test_transform_image_edit_request_with_bytesio_mask_list covering proxy-style BytesIO mask handling; removes unused image_edit and BufferedReader imports. No mock weakening detected.
tests/test_litellm/proxy/image_endpoints/test_azure_routes.py Adds test_image_edit_route_with_mask exercising the proxy /v1/images/edits route with both image and mask files; uses TestClient + mocked litellm.aimage_edit — no real network calls.

Reviews (9): Last reviewed commit: "fix(httpx): avoid masked request content..." | Re-trigger Greptile

Comment thread litellm/llms/custom_httpx/http_handler.py Outdated
@codecov

codecov Bot commented Apr 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@oss-pr-review-agent-shin
oss-pr-review-agent-shin Bot changed the base branch from litellm_internal_staging to shin_agent_oss_staging_05_11_2026 May 11, 2026 17:41
@oss-pr-review-agent-shin
oss-pr-review-agent-shin Bot changed the base branch from shin_agent_oss_staging_05_11_2026 to litellm_internal_staging May 11, 2026 17:41

Copy link
Copy Markdown
Author

@greptileai

@samrusani

Copy link
Copy Markdown
Author

Rebased this PR onto current litellm_internal_staging and resolved the custom HTTPX masking conflict while keeping the scope unchanged.

Validation run on the rebased head:

  • uv run --no-sync pytest tests/test_litellm/llms/custom_httpx/test_credential_leak_prevention.py tests/test_litellm/llms/openai/test_openai_image_edit_transformation.py -q
  • uv run --no-sync pytest tests/test_litellm/proxy/image_endpoints/test_azure_routes.py -q
  • uv run --no-sync ruff format --check litellm/llms/custom_httpx/http_handler.py tests/test_litellm/llms/custom_httpx/test_credential_leak_prevention.py tests/test_litellm/llms/openai/test_openai_image_edit_transformation.py tests/test_litellm/proxy/image_endpoints/test_azure_routes.py
  • uv run --no-sync ruff check litellm/llms/custom_httpx/http_handler.py tests/test_litellm/llms/custom_httpx/test_credential_leak_prevention.py tests/test_litellm/llms/openai/test_openai_image_edit_transformation.py tests/test_litellm/proxy/image_endpoints/test_azure_routes.py
  • git diff --check origin/litellm_internal_staging...HEAD

@greptileai

@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 31 untouched benchmarks


Comparing samrusani:fix/issue-26552-image-edit-mask-stream (9d3475d) with litellm_internal_staging (c28cbb8)1

Open in CodSpeed

Footnotes

  1. No successful run was found on litellm_internal_staging (e24a914) during the generation of this report, so c28cbb8 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@samrusani samrusani changed the title fix: handle image edit mask multipart errors without RequestNotRead fix(httpx): sanitize unread multipart headers in masked errors Jul 28, 2026

Copy link
Copy Markdown
Author

Updated the title and description to reflect the current incremental scope. #26718 already fixed the original RequestNotRead crash; this PR is now explicitly a follow-up that removes stale multipart entity headers when an unread request body is replaced with empty content, with focused image-edit proxy and mask regression coverage. No code changed as part of this reframe.

Copy link
Copy Markdown
Author

@greptileai please review the current head under the reframed follow-up scope described above.

@samrusani
samrusani force-pushed the fix/issue-26552-image-edit-mask-stream branch from 5b9281c to 828538b Compare August 4, 2026 22:28
@samrusani

Copy link
Copy Markdown
Author

@greptileai

@samrusani
samrusani force-pushed the fix/issue-26552-image-edit-mask-stream branch from 828538b to 3902a43 Compare August 7, 2026 11:47

Copy link
Copy Markdown
Author

@greptileai

Copy link
Copy Markdown
Author

CI note: the current assert-ci-coverage failure is inherited from staging commit e1717c5e9c, which added tests/test_litellm/proxy/credential_endpoints/test_endpoints.py without assigning it to a CI job. This PR does not modify that test or CI configuration; the same base-wide check fails on both freshly rebased PRs.

@samrusani
samrusani force-pushed the fix/issue-26552-image-edit-mask-stream branch from 3902a43 to f0b2b47 Compare August 7, 2026 16:08
@samrusani

samrusani commented Aug 7, 2026

Copy link
Copy Markdown
Author

Rebased onto the current litellm_internal_staging head and resolved the remaining type-discipline CI findings in _build_masked_request. The unread multipart path now returns immediately after stripping entity headers, while the readable-content path remains unchanged.

Validation:

  • 44 focused HTTPX, OpenAI image-edit, and proxy image-route tests passed
  • Ruff and format checks passed
  • type-discipline, strict-rule, and basedpyright delta gates passed
  • diff check passed
  • GitHub lint and code-quality jobs passed

The remaining immediate failures are inherited from the current staging base: tests/test_litellm/proxy/credential_endpoints/test_endpoints.py has no assigned CI job, and uv.lock pins GitPython 3.1.57 while OSV reports 3.1.58 as fixed. This PR changes neither surface.

@greptileai please review the updated diff.

@samrusani

Copy link
Copy Markdown
Author

Maintainer rerun requested for the failed proxy-infra job: GitHub rejected the contributor-side rerun with Must have admin rights to Repository.

The shard passed 6,300 tests and failed only TestPriceDataReloadIntegration::test_distributed_reload_check_function, where the model-cost entry contained the current normalized fields instead of the test one-field dictionary. This PR does not touch model pricing or reload behavior, and the isolated failing test passes locally:

1 passed in 2.54s

Could a maintainer please rerun the failed job? No code change is indicated from this failure.

@samrusani
samrusani force-pushed the fix/issue-26552-image-edit-mask-stream branch from f0b2b47 to 9d3475d Compare August 8, 2026 14:26
@samrusani

Copy link
Copy Markdown
Author

Rebased onto current litellm_internal_staging at e24a9146e3. The rebase completed cleanly, and git range-diff confirms both patch commits are unchanged.

Validation:

  • 44 focused HTTPX/image-edit tests passed
  • the unrelated pricing reload control passed separately in a clean process
  • changed-file Ruff and format checks passed
  • type-discipline, strict-rule, and basedpyright delta gates passed
  • diff check passed

@greptileai please review the current head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants