Conversation
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13688 +/- ##
==========================================
- Coverage 99.03% 99.02% -0.01%
==========================================
Files 135 135
Lines 50940 50969 +29
Branches 2677 2680 +3
==========================================
+ Hits 50446 50472 +26
- Misses 370 371 +1
- Partials 124 126 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Confidence Score: 3/5Not safe to merge: buffered multipart serialization can differ from streamed output, and the change adds unnecessary peak memory use for large encoded parts. The repository-required docstring cleanup and changelog fragment are also required before merging. Two independently reproduced functional problems affect multipart serialization behavior and memory consumption. Files Needing Attention: aiohttp/multipart.py, tests/test_multipart.py, and CHANGES/13688.bugfix.rst
 T-Rex LogsWhat T-Rex did
Comments Outside Diff (1)
Reviews (1): Last reviewed commit: "[pre-commit.ci] auto fixes from pre-comm..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| w.enable_compression(_e) | ||
| if _te: | ||
| w.enable_encoding(_te) | ||
| await w.write(part_bytes) |
There was a problem hiding this comment.
For a payload that writes several chunks, write() applies quoted-printable encoding per chunk, while as_bytes() first combines those chunks and encodes them once. A payload writing b"hello ", b"world", and b"!" emits hello=20world! through write() but hello world! through as_bytes(). Callers using buffered serialization can therefore send multipart bytes that differ from the streamed representation, including different message signatures or checksums.
Knowledge Base Used: Payloads, forms, and multipart bodies
Artifacts
- The authored executable script creates a multi-chunk payload and compares the multipart `write()` and `as_bytes()` paths, demonstrating the chunk-boundary discrepancy.
- The command-captured source shows the complete standalone test that was executed against the local pure-Python aiohttp checkout.
- Executing the streaming path produced `hello=20world!` from three source chunks, showing chunk-boundary-sensitive encoding.
- Executing the as-bytes path produced `hello world!` from the same logical payload, confirming it differs from streaming output.
| w.enable_encoding(_te) | ||
| await w.write(part_bytes) | ||
| await w.write_eof() | ||
| parts.append(bytes(writer.buffer)) |
There was a problem hiding this comment.
bytes(writer.buffer) duplicates the entire transformed part while the original bytearray remains live, before b"".join(parts) allocates the final multipart body. In a reproduced 12 MiB base64 case, this retained an additional 16 MiB encoded-body allocation. Large encoded multipart payloads therefore consume substantially more peak memory and may fail under memory limits.
Knowledge Base Used: Payloads, forms, and multipart bodies
Artifacts
- The authored probe compares retaining the transformed bytearray with copying it to bytes before final multipart assembly.
- The captured source records the complete instrumented comparison used for the allocation measurement.
- The retained-bytearray run records memory immediately before and after the final join.
- The bytes-copy run records the additional full transformed-body allocation before the final join.
|
|
||
|
|
||
| async def test_multipart_writer_as_bytes_with_encoding() -> None: | ||
| """Test that MultipartWriter.as_bytes() applies content encoding.""" |
There was a problem hiding this comment.
The new docstring repeats the test name and assertion instead of documenting behavior that is not apparent from the code. This violates the repository directive against comments and docstrings that merely restate code, and the 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!
| @@ -1174,9 +1174,29 @@ async def as_bytes(self, encoding: str = "utf-8", errors: str = "strict") -> byt | |||
| # Add headers | |||
There was a problem hiding this comment.
This changes user-visible serialization of encoded multipart payloads, but no CHANGES/13688.bugfix.rst fragment is included. This violates the repository directive requiring bug-fix changelog fragments, and the 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!
1 participant
Fixes #13495
When a part appended to a non-form-data
MultipartWritercarriesContent-EncodingorContent-Transfer-Encoding,write()compresses/encodes the part on the wire viaMultipartPayloadWriter, butas_bytes()previously returned the raw, untransformed part content.This fix updates
as_bytes()to properly apply encoding viaMultipartPayloadWriterto match the wire protocol, adding consistent behavior.