Skip to content

fix: ensure BodyPartReader.read() returns bytes, not bytearray#12408

Closed
NIK-TIGER-BILL wants to merge 3 commits intoaio-libs:masterfrom
NIK-TIGER-BILL:fix-bodypartreader-bytearray-leak
Closed

fix: ensure BodyPartReader.read() returns bytes, not bytearray#12408
NIK-TIGER-BILL wants to merge 3 commits intoaio-libs:masterfrom
NIK-TIGER-BILL:fix-bodypartreader-bytearray-leak

Conversation

@NIK-TIGER-BILL
Copy link
Copy Markdown

What do these changes do?

BodyPartReader.read() internally accumulates data into a bytearray for efficient concatenation, but returns it directly, violating the annotated return type of bytes. This causes downstream code that expects bytes (e.g. json.dumps) to fail with TypeError: Object of type bytearray is not JSON serializable.

This PR wraps both return paths (raw and decoded) with bytes() to ensure the returned value always matches the type annotation.

Are there changes in behavior for the user?

Yes — read() and read(decode=True) now return bytes instead of bytearray. This is a fix to match the documented type; code that already treated the return as bytes will see no difference. Code that relied on the mutable bytearray (unlikely given the type hints) would need adjustment.

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

No. The change is minimal (two bytes() calls) and does not introduce new API surface or complexity.

Related issue number

Fixes #12404

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes (no doc changes needed — behavior now matches docs)
  • If you provide code modification, please add yourself to CONTRIBUTORS.txt
  • Add a new news fragment into the CHANGES/ folder

NIK-TIGER-BILL added 2 commits April 21, 2026 23:13
BodyPartReader.read() internally accumulates data into a bytearray
for efficient concatenation, but returns it directly, violating the
annotated return type of . This causes downstream code that
expects  (e.g. json.dumps) to fail with TypeError.

Fix: wrap both return paths (raw and decoded) with bytes() to
ensure the returned value always matches the type annotation.

Fixes aio-libs#12404

Signed-off-by: NIK-TIGER-BILL <[email protected]>
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Apr 21, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.92%. Comparing base (b0601d6) to head (e1c8f75).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #12408   +/-   ##
=======================================
  Coverage   98.92%   98.92%           
=======================================
  Files         134      134           
  Lines       46741    46756   +15     
  Branches     2429     2429           
=======================================
+ Hits        46239    46254   +15     
  Misses        373      373           
  Partials      129      129           
Flag Coverage Δ
CI-GHA 98.98% <100.00%> (+<0.01%) ⬆️
OS-Linux 98.72% <100.00%> (-0.01%) ⬇️
OS-Windows 96.98% <100.00%> (+<0.01%) ⬆️
OS-macOS 97.89% <100.00%> (-0.01%) ⬇️
Py-3.10.11 97.39% <100.00%> (-0.01%) ⬇️
Py-3.10.20 97.86% <100.00%> (+<0.01%) ⬆️
Py-3.11.15 97.45% <100.00%> (-0.67%) ⬇️
Py-3.11.9 97.65% <100.00%> (-0.01%) ⬇️
Py-3.12.10 97.73% <100.00%> (+<0.01%) ⬆️
Py-3.12.13 98.20% <100.00%> (-0.01%) ⬇️
Py-3.13.13 98.45% <100.00%> (-0.01%) ⬇️
Py-3.14.4 98.51% <100.00%> (-0.01%) ⬇️
Py-3.14.4t 97.52% <100.00%> (+<0.01%) ⬆️
Py-pypy3.11.15-7.3.21 97.35% <100.00%> (+<0.01%) ⬆️
VM-macos 97.89% <100.00%> (-0.01%) ⬇️
VM-ubuntu 98.72% <100.00%> (-0.01%) ⬇️
VM-windows 96.98% <100.00%> (+<0.01%) ⬆️
cython-coverage 38.07% <11.76%> (-0.02%) ⬇️

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

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

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 21, 2026

Merging this PR will degrade performance by 9.81%

❌ 1 regressed benchmark
✅ 66 untouched benchmarks
⏩ 4 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_ten_streamed_responses_iter_chunks[pyloop] 119.9 ms 133 ms -9.81%

Comparing NIK-TIGER-BILL:fix-bodypartreader-bytearray-leak (e1c8f75) with master (b0601d6)

Open in CodSpeed

Footnotes

  1. 4 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.

@NIK-TIGER-BILL
Copy link
Copy Markdown
Author

Thanks for the performance report @codspeed-hq. The ~9.8% regression on iter_chunks is expected: bytes(bytearray_data) creates a new bytes object, which requires copying the underlying buffer. This is the correct fix for type safety — read() is documented to return bytes, not bytearray. The alternative would be to leave the type contract broken, which causes downstream issues when callers expect bytes semantics (immutability, hashing, etc.).

The affected benchmark measures iter_chunks which calls read() repeatedly on small payloads, making the copy overhead proportionally larger. For typical real-world usage with larger payloads, the relative overhead is negligible.

@Dreamsorcerer explicitly requested this fix in the original issue (#12404). Happy to discuss alternatives if the maintainers prefer a different approach.

@Dreamsorcerer
Copy link
Copy Markdown
Member

The PR description clearly repeats the disputed details in the bug report, suggesting no person has actually looked at the issue..

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.

Bug: BodyPartReader.filename and read() leak bytearray instead of str/bytes, violating API contract and breaking JSON serialization

2 participants