Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.02%. Comparing base (c207cad) to head (16ffafe).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##           master   #13509      +/-   ##
==========================================
+ Coverage   99.01%   99.02%   +0.01%     
==========================================
  Files         132      133       +1     
  Lines       49813    49924     +111     
  Branches     2587     2594       +7     
==========================================
+ Hits        49323    49439     +116     
+ Misses        368      366       -2     
+ Partials      122      119       -3     

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

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported boundary-prefix consumption path was removed because base64 alignment no longer reads supplemental bytes from the underlying stream.

_{Reviews (4): Last reviewed commit: "Fix" | Re-trigger Greptile}

Merging this PR will not alter performance

✅ 84 untouched benchmarks
🆕 2 new benchmarks
⏩ 83 skipped benchmarks¹

Performance Changes

_{Comparing fix-multipart-b64 (16ffafe) with master (2e64b0f)}

@aiolibsbot review

@Dreamsorcerer: @aiolibsbot review

Reviewed against PR head 8dc7772, measured in a worktree. Two blockers.

1. _prev_chunk grows unbounded on boundary-delimited parts. _read_chunk_from_stream reads flat size per call; _align_base64_chunk returns only cut and defers rest. Difference accumulates forever. Measured: 16 MiB part -> 3.36 MB buffer, single read_chunk(8192) returned 3.36 MB. Master stays at 8192 for same input. Benign CRLF base64 drifts too: 8276 -> 9590 bytes across 256 KiB -> 4 MiB payloads.

2. cut == 0 fallback regressed working sizes. Revert tests/test_multipart.py:428/:438 from 8 back to 6 — both fail with binascii.Error: Incorrect padding on b'VG\r\r\nl'. Master decodes sizes 5 through 15; this branch fails 5 and 6.

Credit where due: master raises ValueError for every non-multiple-of-4 size on length-delimited base64 parts. This branch fixes all sizes >= 4. Diagnosis correct, bound missing. Details inline.

Previous review — superseded by a newer review below.

@aiolibsbot review

Previous review — superseded by a newer review below.

@aiolibsbot review

PR Review — Fix multipart base64 edge cases

The unbounded-memory finding from last round is genuinely fixed and the underlying bug fix is substantial — one blocking item remains untouched: a confirmed read_chunk(5|6) regression that two pre-existing tests were edited to hide.

Verified on the current head 16ffafe4 in a clean worktree (aiohttp/multipart.py re-read in full, 141 passed / 2 skipped in tests/test_multipart.py, 288 passed across multipart + web-functional, black clean):

Fixed since last round. The one-line change _align_base64_chunk(chunk, size) → _align_base64_chunk(chunk, len(carry) + want) closes warning ＃1 completely. Capping against the inflated read rather than the requested size stops the call from reading more than it hands back. Measured at size == _boundary_len (the worst case I reported last time, 40-char boundary):

Flat, and max chunk size + 2. I then tried to break it on purpose — six whitespace-run shapes (AAAAA + 8000 spaces repeated, AAAAA + 8190 spaces, a 1 MiB single run) across sizes 42 / 43 / 8192 — and every carry stayed under size, every chunk under size + boundary_len.

The fix is worth more than the changelog says. A fuzz over 3 boundary lengths x 9 payload sizes x 4 wrapping shapes x 7 chunk sizes x both length/boundary paths: 141 failures on master, 0 on this branch. And on master a 1 MiB insignificant-byte run costs 11.3 s of CPU before raising, growing quadratically; here it is 0.01 s.

Pushing back on greptile's P1 (aiohttp/multipart.py:471, partial boundary prefix consumption): resolved by construction. want = max(want, self._boundary_len) means _read_chunk_from_stream can no longer be asked for fewer than _boundary_len bytes, so the one- or two-byte supplemental read that comment describes cannot occur.

Test quality is good. test_read_chunk_base64_bounded_by_requested_size and ..._length_delimited_carries_quartet both guard against going quiet (carried > len(chunks) // 2, carried > 10), and ..._padding_run_does_not_amplify states outright what it does not prove. That is the right answer to last round's objection.

• 🟡 multipart.py:413 — the cut == 0 fallback still returns an unaligned chunk. On b"VG\r\r\nltZSB0byBSZ\r\nWxheCE=", sizes 5 and 6 raise binascii.Error here and decode fine on master. tests/test_multipart.py:428/:438 were switched from size=6 to size=8; reverting that one number fails both on this branch. Reach is narrow (needs a very short boundary), but the contract narrowing is currently recorded nowhere — fix it, or restore the sizes in a test that asserts the new failure and name the minimum in the changelog.
• 🟢 THREAT_MODEL.md:668 — mitigation 4.9 still describes the deleted "extends each chunk" loop, and the CPU-amplification win above is not recorded anywhere.
• 🟢 docs/multipart_reference.rst:49 — size is now a cap that a chunk may exceed when a carry is pending; the reference and the changelog fragment both still describe the old contract.
• 🟢 tests/test_benchmarks_multipart.py:61 — same stale wording, and the body takes the cut == 0 return so the carry path the name promises is never measured. (Correcting myself: the walk-back loop is exercised, so this is not the no-op I called it last round.)

🟡 Important

        if not cut:
            # No whole quartet to hand back, and carrying the lot would make
            # no progress: the caller asked for this many bytes, and a part
            # that holds no quartet within them holds none to give.
            return chunk

🟢 Suggestions

   .. method:: read_chunk(size=chunk_size)
      :async:

      Reads body part content chunk of the specified size.

      :param int size: chunk size

Checklist

• Bounded memory on attacker-controlled input (was warning ＃1)
• No quadratic CPU on insignificant-byte runs
• No behavioural regression vs master — warning ＃1
• Pre-existing tests not weakened to fit the change — warning ＃1
• New behaviour covered by meaningful tests
• Length-delimited base64 works for all sizes >= boundary_len (broken on master)
• No over-read past Content-Length into following parts
• Full multipart + web-functional suites pass (288 passed, 14 skipped)
• Changelog fragment present and correctly attributed
• Docs updated for user-visible API change — suggestion ＃2
• THREAT_MODEL.md kept in sync per AGENTS.md
• Benchmarks follow repo conventions and measure what they name
• No hardcoded secrets or unsafe deserialization
• No resource leaks or infinite-loop risk (read_chunk never returns empty while not at_eof)
• black/isort/flake8 clean

To rebase and address feedback, mention me: @aiolibsbot rebase critical (fixes 🔴 only), @aiolibsbot rebase important (fixes 🔴 + 🟡), or @aiolibsbot rebase --fix for all. (A bare @aiolibsbot rebase only rebases onto the base branch.)

Automated review by Kōan (Claude) HEAD=16ffafe 10 min 51s

Backport to 3.15: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.15/2d9c5c1062eff0034fb3ab120897a6c1b3ff7041/pr-13509

Backported as #13512

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Backport to 3.14: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.14/2d9c5c1062eff0034fb3ab120897a6c1b3ff7041/pr-13509

Backported as #13513

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.