Opened 10 years ago
Closed 10 years ago
#23887 closed Bug (fixed)
Invalid multipart boundary causes internal server error
Reported by: | Tim Graham | Owned by: | Claude Paroz |
---|---|---|---|
Component: | HTTP handling | Version: | 1.7 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Reported by Antti Häyrynen and verified by me.
In http/request.py _load_post_and_files
when calling self.parse_file_upload
failures raise MultiPartParserError
, which causes http request handler to throw 500 in case multipart has something wrong (invalid boundary and apparently negative content length). I think bad request would be better response to this.
Attached is a sample HTTP request, which has a bell in mime boundary that makes django return 500.
To reproduce (you must have the admin installed so /admin/login/ doesn't 404):
$ nc 127.0.0.1 8000 < beep-500.httpreq
Attachments (1)
Change History (7)
by , 10 years ago
Attachment: | beep-500.httpreq added |
---|
comment:1 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:2 by , 10 years ago
Has patch: | set |
---|
comment:3 by , 10 years ago
Did you consider having MultiPartParserError
inherit SuspiciousOperation
?
That would result in a 400 (if memory serves).
comment:4 by , 10 years ago
Right, it would be simpler/cleaner.
However, I wonder if this would be semantically correct. Can we really consider MultiPartParserError
as a SuspiciousOperation
? Note that SuspiciousOperation
exceptions are logged to the security logger.
comment:5 by , 10 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Right, it looks like it isn't the best choice in that case. I just wanted to make sure it had been considered :-)
I left a small suggestion on the PR, otherwise this is good to go.
comment:6 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
https://github.com/django/django/pull/3599