Fix BodyPartReader.read_chunk crash on Content-Length: 0 by qianbkk · Pull Request #13795 · aio-libs/aiohttp · GitHub
Skip to content

Fix BodyPartReader.read_chunk crash on Content-Length: 0 - #13795

Closed
qianbkk wants to merge 2 commits into
aio-libs:masterfrom
qianbkk:fix-issue-13758-content-length-zero
Closed

qianbkk wants to merge 2 commits into
aio-libs:masterfrom
qianbkk:fix-issue-13758-content-length-zero

Conversation

@qianbkk

@qianbkk qianbkk commented Sep 23, 2026

Copy link
Copy Markdown

Fixes #13758.

BodyPartReader.read_chunk routes between the length-based and stream-based readers with if self._length. A part with Content-Length: 0 therefore fell through to _read_chunk_from_stream, whose first action is:

assert size >= self._boundary_len, "Chunk size must be greater or equal than boundary length + 2"

so any caller asking for a small chunk hit AssertionError even though the part explicitly declares zero content. Content-Length: 0 is legal outside multipart/form-data (e.g. multipart/mixed, multipart/related), where the form-data empty-part shortcut that nulls out _length does not apply.

The other six uses of self._length in multipart.py already check is not None; this was the odd one out. Replacing the truthiness test with is not None keeps the zero-length case on the length path, which immediately returns b"" and marks the part at EOF.

-        if self._length:
+        if self._length is not None:

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.py and running a standalone reproducer (now returns b'' instead of raising). CI on master will run the full test_multipart.py suite.

AI disclosure

Prepared with AI assistance (kilo assistant). Fix, regression test, and PR description developed and verified locally before submission.

`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.
@qianbkk
qianbkk requested a review from asvetlov as a code owner September 23, 2026 21:15
Copilot AI lite review requested due to automatic review settings September 23, 2026 21:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 None check for content length.
  • Adds regression coverage for empty output and EOF state.
File Description
tests/​test_multipart.py Adds zero-length part regression coverage.
aiohttp/​multipart.py Corrects reader selection for zero-length parts.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.04%. Comparing base (7e67714) to head (3213579).
✅ All tests successful. No failed tests found.

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           
Flag Coverage Δ
Autobahn 21.92% <11.11%> (-0.01%) ⬇️
CI-GHA 98.87% <100.00%> (+<0.01%) ⬆️
OS-Linux 98.65% <100.00%> (+<0.01%) ⬆️
OS-Windows 97.26% <100.00%> (+<0.01%) ⬆️
OS-macOS 98.13% <100.00%> (-0.01%) ⬇️
Py-3.10 98.07% <100.00%> (+<0.01%) ⬆️
Py-3.11 98.30% <100.00%> (+<0.01%) ⬆️
Py-3.12 98.39% <100.00%> (+<0.01%) ⬆️
Py-3.13 98.37% <100.00%> (+<0.01%) ⬆️
Py-3.14 98.41% <100.00%> (-0.01%) ⬇️
Py-3.14t 97.78% <100.00%> (-0.01%) ⬇️
Py-pypy-3.11 97.35% <100.00%> (-0.02%) ⬇️
VM-macos 98.13% <100.00%> (-0.01%) ⬇️
VM-ubuntu 98.65% <100.00%> (+<0.01%) ⬆️
VM-windows 97.26% <100.00%> (+<0.01%) ⬆️
cython-coverage 83.16% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@greptile-apps

greptile-apps Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

Not merge-safe until the outstanding repository changelog requirement is fully satisfied.

Reviews (2) · Last reviewed commit: "Add CHANGES fragment for #13758"

Comment thread aiohttp/multipart.py
self._b64_carry = b""
want = max(want, self._boundary_len)
if self._length:
if self._length is not None:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 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!

@codspeed

codspeed Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 97 untouched benchmarks
⏩ 83 skipped benchmarks1


Comparing qianbkk:fix-issue-13758-content-length-zero (3213579) with master (7e67714)

Open in CodSpeed

Footnotes

  1. 83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@qianbkk
qianbkk requested a review from webknjaz as a code owner September 24, 2026 03:19
@pyba-chronographer pyba-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Sep 24, 2026
@qianbkk

qianbkk commented Sep 24, 2026

Copy link
Copy Markdown
Author

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

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BodyPartReader.read_chunk() crashes with AssertionError on a body part with Content-Length: 0

3 participants