Apply part encodings in MultipartWriter.as_bytes() - #13496
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13496 +/- ##
========================================
Coverage 99.01% 99.01%
========================================
Files 135 135
Lines 50026 50162 +136
Branches 2611 2626 +15
========================================
+ Hits 49531 49668 +137
Misses 371 371
+ Partials 124 123 -1
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 degrade performance by 16.96%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | test_read_base64_part |
14.3 ms | 17.2 ms | -16.96% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing 2sumtech:fix/multipart-as-bytes-encodings (50228e3) with master (394959f)
Footnotes
-
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. ↩
PR Review — Apply part encodings in MultipartWriter.as_bytes()Correct, well-targeted fix — Specific things done well:
Key points:
🟢 Suggestions
1. quoted-printable still diverges from write() for multi-chunk payloads
|
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the earlier quoted-printable chunk-divergence and unbounded partial-line buffering issues are both addressed by the shared fixed-boundary encoder and streaming block flush. Reviews (3): Last reviewed commit: "Bound the quoted-printable partial-line ..." | Re-trigger Greptile |
MultipartPayloadWriter applied binascii.b2a_qp to each payload chunk independently, while as_bytes() encodes the complete body in one pass. b2a_qp derives the line-ending convention, soft line-break positions, and trailing-whitespace quoting from the buffer it is given, so a line ending, long line, or trailing whitespace split across chunk boundaries produced different wire bytes than as_bytes() (breaking the entity hash for digest auth with qop=auth-int), and a CRLF split across chunks was transmitted in a form that decodes to different body bytes. Encode quoted-printable data one \n-terminated line at a time in both paths, buffering the partial trailing line in MultipartPayloadWriter the same way the base64 path buffers unaligned groups, so the output is a pure function of the payload bytes regardless of chunking.
AGENTS.md asks for the issue-numbered fragment plus a symlink named after the PR, so towncrier credits both.
The per-line encoder held a partial line in memory until a newline arrived, so a newline-free payload segment accumulated in full and nothing reached the transport until EOF, defeating backpressure. Encode each line in fixed 4096-byte blocks joined by soft line breaks, with block boundaries at fixed offsets from the start of the line, and flush every complete block of the pending partial line as it arrives. Block-wise output differs from a single b2a_qp call (each call restarts its soft-break column counter), so as_bytes() uses the same block segmentation: the wire bytes stay a pure function of the payload bytes regardless of chunking and still decode to the original body, while the writer retains at most one block between write() calls.
126ba04 to
50228e3
Compare
|
Please follow the PR template: https://github.com/aio-libs/aiohttp/blob/master/.github/PULL_REQUEST_TEMPLATE.md |
|
done, restructured the description to the full template |
What do these changes do?
MultipartWriter.as_bytes()ignored the per-partContent-Encoding/Content-Transfer-Encodingthatwrite()applies viaMultipartPayloadWriter, so its result did not match the wire body. The output contradicted the parts' own headers, and digest authentication withqop=auth-intalways failed for such payloads because the middleware hashesas_bytes()while the wire carries the encoded body. This applies the same transformations (ZLibCompressorfor gzip/deflate, base64/quoted-printable for CTE) inas_bytes(), making the output byte-identical towrite().Are there changes in behavior for the user?
as_bytes()now returns compressed/encoded part bodies for non-form-data multiparts that declare those headers, matching what is actually sent on the wire.Is it a substantial burden for the maintainers to support this?
No. The change reuses the exact transformation code path
write()already exercises, so the two stay in sync by construction. No new public API, no new dependency.Related issue number
Fixes #13495
Checklist
CONTRIBUTORS.txtCHANGES/folder (13495.bugfix.rst)Test runs
New parametrized tests assert byte-equality between
as_bytes()and thewrite()wire output for gzip/deflate Content-Encoding and base64/quoted-printable CTE, plus content round-trips. 6 failed without the fix and 6 pass with it. Fulltest_multipart.py+test_formdata.py+test_client_middleware_digest_auth.py+test_payload.py: 448 passed. flake8 and mypy clean on touched files.Drafted with Claude Code (Fable 5); reviewed by @2sumtech.