Ticket #790 (defect)
Opened 3 months ago
Last modified 3 months ago
[PATCH] Request body of PUT with no Content-Type is parsed incorrectly
Status: closed (fixed)
| Reported by: | miles.chris@gmail.com | Assigned to: | fumanchu |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.1 |
| Component: | CherryPy code | Keywords: | |
| Cc: | miles.chris@gmail.com |
When CP3 receives a request (such as a PUT) containing an entity body, but without a Content-Type header, it still tries to parse the entity body to pull out params. This results in the controller method being called with a nonsensical set of keyword args.
CP2 didn't do this, because _cpwsgiserver.HTTPRequest.parse_request would always create a Content-Type header with an empty value if the client left that header out.
CP3 should behave the same as CP2, which is to not attempt to parse the request body unless the Content-Type is "application/x-www-form-urlencoded" or multipart.
The attached patch changes CP3 to behave this way (includes updated unit tests).
Also see the thread at http://groups.google.com/group/cherrypy-devel/browse_thread/thread/9db04c18725dcf3c
Attachments
Change History
02/20/08 02:37:24: Modified by miles.chris@gmail.com
- attachment cp3_no_content_type.patch added.
02/20/08 10:32:27: Modified by fumanchu
- milestone set to 3.1.
+1 on the ticket needing fixed, but -1 on the patch. No code should modify request.headers; it's important that applications have access to the actual header values sent (or not sent) in the HTTP request. Better to fake out just the body parser than the entire stack.
02/21/08 03:03:35: Modified by miles.chris@gmail.com
- attachment cp3_no_content_type2.patch added.
Ensure a Content-Type header is passed to FieldStorage? so request body is not parsed unnecessarily.
02/21/08 03:05:39: Modified by miles.chris@gmail.com
- cc set to miles.chris@gmail.com.
Fair call. Attached is a new patch (cp3_no_content_type2.patch) that passes a modified (if necessary) copy of the headers to FieldStorage, ensuring that a "Content-Type" header is included. This ensures that FieldStorage doesn't attempt to parse the request body if the client didn't supply a "Content-Type" header, but leaves the actual request header values untouched outside of this section.
No unit tests needed to be updated to support this patch.
02/21/08 12:44:12: Modified by fumanchu
- owner changed from rdelon to fumanchu.
- status changed from new to assigned.
Test and not-quite-fix in [1899]. I can't figure out why it doesn't pass. :/
02/21/08 22:14:59: Modified by miles.chris@gmail.com
It doesn't work because self.headers.copy() returns a dict rather than another HeaderMap object. That's why I had to make the copy using h = http.HeaderMap(self.headers.items()).
An alternative fix would be to overload the copy method of HeaderMap to return a copy of itself properly.
02/22/08 03:51:08: Modified by fumanchu
- status changed from assigned to closed.
- resolution set to fixed.
Bah. That doesn't make any sense. But it works, so applied in [1900].
02/22/08 04:05:50: Modified by fumanchu
Ah. Now I remember, cgi.Fieldstorage expects all lowercase header keys. HeaderMap? works around that by overriding __contains__ etc to title-case the search key.
Sheesh.
Patch to CP3 trunk to force creation of Content-Type header if not supplied by client