Repository navigation
Conversation
`BodyPartReader.read_chunk` picked its read strategy by testing `self._length` for truthiness, which routed a part with `Content-Length: 0` through `_read_chunk_from_stream` instead of `_read_chunk_from_length`. The stream path then asserted that the requested chunk size is at least `_boundary_len`, and any caller asking for a small chunk hit `AssertionError` -- even though `Content-Length: 0` already says there is nothing to read. A `Content-Length: 0` body part is legal outside `multipart/form-data` (used by `multipart/mixed`, `multipart/related`, and similar), where the empty-part special case that nulls out `_length` does not apply. Every other use of `self._length` in `multipart.py` already checks `is not None`; `read_chunk` is the odd one out. Switching the test to `is not None` keeps the empty length on the length path, which returns `b""` immediately and marks the part at EOF -- matching the behaviour of any other zero-length Content-Length caller.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The fix and regression test address the reported zero-length multipart failure.
Review effort: Lite
Findings: None
What changed in this PR
Fixes BodyPartReader.read_chunk for multipart parts with Content-Length: 0, preventing an assertion failure and correctly marking the part at EOF.
Changes:
- Uses an explicit
Nonecheck for content length. - Adds regression coverage for empty output and EOF state.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13795 +/- ##
=======================================
Coverage 99.04% 99.04%
=======================================
Files 135 135
Lines 51282 51290 +8
Branches 2691 2691
=======================================
+ Hits 50793 50801 +8
Misses 368 368
Partials 121 121
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
|
| self._b64_carry = b"" | ||
| want = max(want, self._boundary_len) | ||
| if self._length: | ||
| if self._length is not None: |
There was a problem hiding this comment.
Add Required Changelog Fragment
This multipart parsing bug fix has no release-note fragment or required PR-numbered symlink. The repository directive requires both entries for this user-visible fix, so this repository requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Merging this PR will not alter performance
Comparing Footnotes
|

Fixes #13758.
BodyPartReader.read_chunkroutes between the length-based and stream-based readers withif self._length. A part withContent-Length: 0therefore fell through to_read_chunk_from_stream, whose first action is:so any caller asking for a small chunk hit
AssertionErroreven though the part explicitly declares zero content.Content-Length: 0is legal outsidemultipart/form-data(e.g.multipart/mixed,multipart/related), where the form-data empty-part shortcut that nulls out_lengthdoes not apply.The other six uses of
self._lengthinmultipart.pyalready checkis not None; this was the odd one out. Replacing the truthiness test withis not Nonekeeps the zero-length case on the length path, which immediately returnsb""and marks the part at EOF.Regression test added in
TestPartReader.test_read_chunk_with_content_length_zero, asserting both the returned empty bytes and the EOF transition.Verified locally against aiohttp 3.13.3 by patching
multipart.pyand running a standalone reproducer (now returnsb''instead of raising). CI on master will run the fulltest_multipart.pysuite.AI disclosure
Prepared with AI assistance (kilo assistant). Fix, regression test, and PR description developed and verified locally before submission.