return 415 for undecodable multipart field charset in post - #13462
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13462 +/- ##
==========================================
+ Coverage 98.42% 99.02% +0.60%
==========================================
Files 133 135 +2
Lines 49747 50518 +771
Branches 2628 2652 +24
==========================================
+ Hits 48961 50025 +1064
+ Misses 662 370 -292
+ 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 not alter performance
Comparing Footnotes
|
Signed-off-by: Javid Khan <dxbjavid@gmail.com>
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
Confidence Score: 5/5The PR appears safe to merge, with focused error translation and coverage for both affected decoding failures. The guarded decode operation now maps unsupported codec names and invalid encoded bytes to the same HTTP 415 behavior already used by sibling request-decoding paths, and no blocking or non-blocking defect remains. Reviews (1): Last reviewed commit: "Apply batched suggestions from code revi..." | Re-trigger Greptile |
Backport to 3.15: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 6b97ebd on top of patchback/backports/3.15/6b97ebd612e24507324e37d8bab6f8b91ba30abd/pr-13462 Backporting merged PR #13462 into master
🤖 @patchback |
Backport to 3.14: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 6b97ebd on top of patchback/backports/3.14/6b97ebd612e24507324e37d8bab6f8b91ba30abd/pr-13462 Backporting merged PR #13462 into master
🤖 @patchback |
|
Please create backports with the above instructions. |
What do these changes do?
BaseRequest.post()decodes a plain multipart/form-data text field withvalue.decode(charset), wherecharsetcomes from that part's ownContent-Typeheader and is fully attacker controlled. An unknown codec name (charset=not-a-real-codec) raisesLookupErrorand undecodable bytes raiseUnicodeDecodeError, and neither is caught, so a malformed part turns into an uncaught 500. The sibling urlencoded branch a few lines below, andBaseRequest.text(), already wrap the same decode inexcept (LookupError, UnicodeDecodeError)and raiseHTTPUnsupportedMediaType. This brings the multipart branch in line with those two, so a bad part charset now yields a 415 like the other body-parsing paths rather than a server error.Are there changes in behavior for the user?
A multipart field whose charset is unknown or whose bytes do not decode now surfaces as
HTTPUnsupportedMediaType(415) instead of an unhandled exception. Valid fields are unaffected.Is it a substantial burden for the maintainers to support this?
No. It is a two-line guard that mirrors existing code in the same function.
Related issue number
N/A
Checklist
CONTRIBUTORS.txtCHANGES/folder