Skip to content

cap base64 body-part reads at the declared Content-Length - #13497

Closed
arshsmith1 wants to merge 2 commits into
aio-libs:masterfrom
arshsmith1:multipart-base64-content-length-overread
Closed

cap base64 body-part reads at the declared Content-Length#13497
arshsmith1 wants to merge 2 commits into
aio-libs:masterfrom
arshsmith1:multipart-base64-content-length-overread

Conversation

@arshsmith1

Copy link
Copy Markdown
Contributor

What do these changes do?

BodyPartReader.read_chunk pads base64 body-part data out to whole 4-byte groups by topping up from the stream. For a length-delimited part (one carrying Content-Length, e.g. inside multipart/mixed) that top-up read went straight to self._content.read() with no bound, so when a part's whitespace-stripped base64 length is not a multiple of 4 it reads past the declared length into the trailing CRLF, the boundary and the parts that follow. A part sent with Content-Length: 19 comes back as 75 bytes carrying a later part's data, and once _read_bytes overshoots _length a subsequent read can pass a negative size to StreamReader.read and drain the rest of the stream. The top-up read is now capped at the bytes still owed by the declared part length, so a length-delimited part stops at its boundary.

Are there changes in behavior for the user?

Only for the malformed case. A base64 part whose declared length is a proper multiple of 4 (including CRLF-wrapped bodies) reads and decodes exactly as before across chunk sizes; a part that previously over-read now returns just its declared bytes and reaches EOF at the boundary instead of pulling in the following parts.

Is it a substantial burden for the maintainers to support this?

No. It bounds one existing read by the part length the reader already tracks.

Related issue number

None.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes - N/A, no public API change
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt (already listed)
  • Add a new news fragment into the CHANGES/ folder

@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Aug 19, 2026
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.00%. Comparing base (8c8906a) to head (782a264).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
aiohttp/multipart.py 40.00% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #13497   +/-   ##
=======================================
  Coverage   99.00%   99.00%           
=======================================
  Files         132      132           
  Lines       49635    49651   +16     
  Branches     2575     2577    +2     
=======================================
+ Hits        49141    49158   +17     
  Misses        370      370           
+ Partials      124      123    -1     
Flag Coverage Δ
Autobahn 22.03% <5.88%> (-0.01%) ⬇️
CI-GHA 98.91% <82.35%> (+<0.01%) ⬆️
OS-Linux 98.69% <82.35%> (-0.01%) ⬇️
OS-Windows 97.02% <82.35%> (+<0.01%) ⬆️
OS-macOS 97.94% <82.35%> (+<0.01%) ⬆️
Py-3.10 98.13% <82.35%> (+<0.01%) ⬆️
Py-3.11 98.37% <82.35%> (+<0.01%) ⬆️
Py-3.12 98.46% <82.35%> (+<0.01%) ⬆️
Py-3.13 98.45% <82.35%> (+<0.01%) ⬆️
Py-3.14 98.47% <82.35%> (+<0.01%) ⬆️
Py-3.14t 97.56% <82.35%> (-0.01%) ⬇️
Py-pypy-3.11 97.41% <82.35%> (+0.01%) ⬆️
VM-macos 97.94% <82.35%> (+<0.01%) ⬆️
VM-ubuntu 98.69% <82.35%> (-0.01%) ⬇️
VM-windows 97.02% <82.35%> (+<0.01%) ⬆️
cython-coverage 82.24% <0.00%> (-0.02%) ⬇️

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.

@codspeed-hq

codspeed-hq Bot commented Aug 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 84 untouched benchmarks
⏩ 83 skipped benchmarks1


Comparing arshsmith1:multipart-base64-content-length-overread (782a264) with master (8c8906a)

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.

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.

2 participants