Sort RFC 2231 filename continuations numerically - #13500
Conversation
content_disposition_filename() assembled filename*N continuation sections with a lexicographic sort, so filename*10 sorted before filename*2 and any filename split into 10 or more sections was silently truncated after the first two. RFC 2231 Section 3 requires numeric ordering of continuation sections. Sort the sections by their integer index and decode each section according to its own extended (*N*, percent-encoded) or quoted (*N, literal) marker per RFC 2231 Section 4.1, using the charset declared by the initial extended section. This also stops apostrophes inside quoted sections from being misread as an RFC 5987 charset prefix, which previously raised ValueError or dropped parts of the name.
psf-chronographer
Bot
added
the
bot:chronographer:provided
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13500 +/- ##
=======================================
Coverage 99.00% 99.00%
=======================================
Files 132 132
Lines 49635 49701 +66
Branches 2575 2583 +8
=======================================
+ Hits 49141 49207 +66
Misses 370 370
Partials 124 124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (4): Last reviewed commit: "Cover a multibyte character split across..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Fixed :func:`~aiohttp.content_disposition_filename` truncating filenames split | ||
| into 10 or more RFC 2231 continuation sections by sorting the sections | ||
| numerically instead of lexicographically, and decoding each section according | ||
| to its own extended/quoted marker -- by :user:`2sumtech`. |
There was a problem hiding this comment.
This adds only the issue-numbered fragment, while the repository convention requires a CHANGES/13500.bugfix.rst symlink to 13499.bugfix.rst when the issue and PR numbers differ. Without it, the change does not follow the project's changelog bookkeeping convention.
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!
Merging this PR will not alter performance
Comparing 
Footnotes
|
Percent-decoding each extended section on its own dropped the whole filename when a multibyte character straddled a section boundary. Join the octets of adjacent extended sections before decoding, and reference the changelog fragment by PR number via a symlink. The changelog fragment no longer uses a :func: role, because content_disposition_filename is not in the API docs and the dangling reference failed the docs build.
Adds tests for octets that stay undecodable once the sections are joined, an unknown charset, and a non-numeric section index, so the new branches are exercised.
A three-octet character can land in three different extended sections; exercise that the whole run is still joined before decoding.
1 participant
What do these changes do?
Fix
content_disposition_filename()assembling RFC 2231 continuation sections in lexicographic order:filename*10sorted beforefilename*2, so filenames split into 10+ sections were silently truncated after two sections. Sections are now sorted by their integer index (RFC 2231 Section 3), and each section is decoded according to its own extended (*N*, percent-encoded) or quoted (*N, literal) marker (RFC 2231 Section 4.1), using the charset declared by the initial extended section. This also stops apostrophes inside quoted sections from being misread as an RFC 5987charset'language'prefix, which previously raised an uncaughtValueError(one apostrophe) or dropped most of the filename (two).Fixes #13499
Are there changes in behavior for the user?
Only for malformed-or-mishandled inputs: 10+-section names are now complete; apostrophes in quoted sections are literal; extended sections are always percent-decoded even when the initial section carries no
charset''prefix (previously decoding was keyed on an apostrophe appearing anywhere in the joined value). Missing-section, leading-zero, and starts-at-1 handling is unchanged and covered by the existing greenbytes-derived tests.Testing
Five new tests in
tests/test_multipart_helpers.py::TestContentDispositionFilename(11-section reassembly plain and with charset, quoted/extended mix, apostrophes, single-section guard); all fail (or crash with theValueError) on master and pass with the fix. Fulltest_multipart_helpers.py+test_multipart.py: 253 passed. flake8/mypy clean on touched files; CHANGES fragment13499.bugfix.rstincluded.Disclosure
Drafted with Claude Code (Fable 5); opened as draft pending human review per AGENTS.md.