strip leading separators in all content-disposition filename forms

[arshsmith1]

What do these changes do?

parse_content_disposition strips leading path separators when the filename arrives as a quoted-string, but the RFC 5987 filename* and the RFC 2231 filename*0 / filename*0* continuation forms skip that step, so the sender picks whether the normalisation runs at all: filename*=UTF-8''%2Fetc%2Fcron.d%2Fevil comes back as /etc/cron.d/evil where filename="/etc/cron.d/evil" comes back as etc/cron.d/evil. content_disposition_filename also prefers filename* over filename and reassembles continuation parts on its own, so the unnormalised spelling is the one BodyPartReader.filename and FileField.filename hand to applications. The same lstrip now runs in the continuation and extended-parameter branches and on the reassembled continuation value, so every spelling of the header lands the same way.

The divergence is already visible in the test file: test_attabspath / test_attabspathwin assert the strip on the quoted form, while test_attwithfn2231abspathdisguised asserted the raw \foo.html for the 2231 form of the same greenbytes case. That expectation is updated here, and the other three forms get coverage alongside it.

Are there changes in behavior for the user?

A leading / or \ is now removed from filename* and from filename*N continuation values, matching what a quoted filename has always done. Nothing else about the value changes.

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

No. It reuses the existing lstrip("\\/") at three more spots in the same two functions; no new helpers or API.

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 documented behaviour changes
• If you provide code modification, please add yourself to CONTRIBUTORS.txt — already listed
• Add a new news fragment into the CHANGES/ folder

Local run

$ PYTHONPATH=. AIOHTTP_NO_EXTENSIONS=1 pytest tests/ -q -k "not benchmark"
4390 passed, 92 skipped, 107 deselected, 14 xfailed in 260.01s

# the six new/updated assertions on the unpatched tree
FAILED tests/test_multipart_helpers.py::TestParseContentDisposition::test_attwithfn2231abspathdisguised
FAILED tests/test_multipart_helpers.py::TestParseContentDisposition::test_attwithfn2231abspath
FAILED tests/test_multipart_helpers.py::TestParseContentDisposition::test_attfncontabspath
FAILED tests/test_multipart_helpers.py::TestContentDispositionFilename::test_filename_ext_abspath
FAILED tests/test_multipart_helpers.py::TestContentDispositionFilename::test_attfncontencabspath[params0]
FAILED tests/test_multipart_helpers.py::TestContentDispositionFilename::test_attfncontencabspath[params1]

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.98%. Comparing base (12211f7) to head (d1848c4).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #13206   +/-   ##
=======================================
  Coverage   98.98%   98.98%           
=======================================
  Files         131      131           
  Lines       48955    48969   +14     
  Branches     2550     2550           
=======================================
+ Hits        48458    48472   +14     
  Misses        373      373           
  Partials      124      124           

Flag	Coverage Δ
Autobahn	22.11% <27.77%> (+<0.01%)	⬆️
CI-GHA	98.90% <100.00%> (+<0.01%)	⬆️
OS-Linux	98.67% <100.00%> (+<0.01%)	⬆️
OS-Windows	97.02% <100.00%> (+<0.01%)	⬆️
OS-macOS	97.93% <100.00%> (+<0.01%)	⬆️
Py-3.10	98.11% <100.00%> (+<0.01%)	⬆️
Py-3.11	98.37% <100.00%> (+<0.01%)	⬆️
Py-3.12	98.46% <100.00%> (+<0.01%)	⬆️
Py-3.13	98.44% <100.00%> (+<0.01%)	⬆️
Py-3.14	98.46% <100.00%> (+<0.01%)	⬆️
Py-3.14t	97.55% <100.00%> (+<0.01%)	⬆️
Py-pypy-3.11	97.43% <100.00%> (+<0.01%)	⬆️
VM-macos	97.93% <100.00%> (+<0.01%)	⬆️
VM-ubuntu	98.67% <100.00%> (+<0.01%)	⬆️
VM-windows	97.02% <100.00%> (+<0.01%)	⬆️
cython-coverage	37.88% <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]

Merging this PR will not alter performance

✅ 83 untouched benchmarks
⏩ 83 skipped benchmarks¹

---

_{Comparing arshsmith1:cd-filename-abspath-strip (c8de036) with master (8e814f5)²}

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. ↩

2. No successful run was found on master (62f0f90) during the generation of this report, so 8e814f5 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[Dreamsorcerer]

@aiolibsbot review

[aiolibsbot]

PR Review — strip leading separators in all content-disposition filename forms

Correct, well-tested consistency fix — merge-ready.

The divergence is real and the fix is minimal and right: quoted filename already stripped leading separators while filename*, filename*0 and filename*0* did not, letting the sender choose whether normalization ran. The three lstrip("\\/") additions land in exactly the branches that were skipping it, and I verified the coverage claim:

• Token-form continuation parts can't carry a separator (/ and \ are in helpers.SEPARATORS, excluded from TOKEN), so no gap there.
• Quoted continuation is stripped at parse (line 139); encoded continuation is stripped on reassembly (line 217); extended filename* is stripped at line 153.
• All internal callers feed parse-derived params, so every HTTP-reachable spelling now normalizes the same way.

Tests are thorough — updates the previously-divergent test_attwithfn2231abspathdisguised and adds the other three forms plus a parametrized encoded case. Changelog fragment and attribution (:user:arshsmith1, matching the PR author) are correct.

• Non-blocking suggestion: the plain (unencoded) reassembly path (line 224) and direct filename*/filename returns (194/196) in content_disposition_filename still skip the strip, but this is only reachable via direct API calls with hand-built params, not over HTTP.

---

🟢 Suggestions

1. Plain (unencoded) continuation reassembly still returns without lstrip
aiohttp/multipart.py:224

Nice, targeted fix — the three added lstrip("\\/") calls close the parse-time gaps, and I verified via helpers.SEPARATORS that / and \ are excluded from TOKEN, so a token-form filename*N part can never carry a leading separator. Combined with the quoted-part strip at line 139 and the encoded-reassembly strip at line 217, every HTTP-reachable filename spelling now normalizes identically. Confirmed all internal callers (BodyPartReader, FileField, client_reqrep.content_disposition) feed parse-derived params, so there is no reachable regression.

One small consistency gap remains, non-blocking: in content_disposition_filename, the encoded reassembly path (line 217) strips, but the plain reassembly path here (line 224) and the direct filename*/filename returns (lines 194, 196) do not. This is only reachable when a caller invokes the public content_disposition_filename with a hand-built params dict like {"filename*0": "/foo.", "filename*1": "html"} — through parse_content_disposition the first part is always pre-stripped, so it can't happen over HTTP.

Since the function is exported in __all__, you could add the strip at line 224 (return value.lstrip("\\/")) so the direct-call contract matches the parse-fed one. Entirely optional given the goal is HTTP-parsed normalization.

value = "".join(parts)
if "'" in value:
    ...
return value  # plain continuation reassembly is not lstripped

---

Checklist

• Path-separator normalization consistent across all HTTP-reachable filename forms
• New/updated tests cover the changed branches
• Changelog fragment present with correct type and attribution
• Public function contract fully consistent for direct callers — suggestion ＃1
• No error-handling regressions (lstrip inside existing try blocks)

---

Automated review by Kōan (Claude) HEAD=d1848c4 2 min 50s

[Sanjays2402]

lstrip runs on every continuation chunk here, so filename*0="dir"; filename*1="/file.txt" becomes dirfile.txt instead of dir/file.txt. Can this be deferred until after content_disposition_filename() joins the parts, as the encoded path does?

[arshsmith1]

Good catch, that was wrong. Stripping each quoted continuation part in isolation eats a separator that belongs in the middle of the joined value.

Moved it: the quoted branch in parse_content_disposition is back to what it was, and content_disposition_filename now strips the reassembled value on the plain path too, so it matches how the encoded path already worked. filename*0="dir"; filename*1="/file.txt" gives dir/file.txt again, and filename*0="/foo."; filename*1="html" still gives foo.html. Added a test for each.

That also covers the review bot's note about the plain reassembly path returning unstripped.