#17187 closed Bug (needsinfo)
Incompatibility with Cherokee webserver's FastCGI handling
Reported by: | Samuel Cormier-Iijima | Owned by: | |
---|---|---|---|
Component: | HTTP handling | Version: | 1.3 |
Severity: | Normal | Keywords: | cherokee flup fastcgi fcgi content-length uwsgi |
Cc: | sam@…, leos@… | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This issue is causing any POST request with zero content-length to throw an error (in flup of all places) when set up as a FastCGI backend to Cherokee.
Traceback: File "/home/sciyoshi/django/django/core/handlers/wsgi.py" in _get_request 196. self._request = datastructures.MergeDict(self.POST, self.GET) File "/home/sciyoshi/django/django/core/handlers/wsgi.py" in _get_post 210. self._load_post_and_files() File "/home/sciyoshi/django/django/http/__init__.py" in _load_post_and_files 289. self._post, self._files = QueryDict(self.raw_post_data, encoding=self._encoding), MultiValueDict() File "/home/sciyoshi/django/django/http/__init__.py" in _get_raw_post_data 251. self._raw_post_data = self.read() File "/home/sciyoshi/django/django/http/__init__.py" in read 301. return self._stream.read(*args, **kwargs) File "/home/sciyoshi/env2/local/lib/python2.7/site-packages/flup/server/fcgi_base.py" in read 161. self._waitForData() File "/home/sciyoshi/env2/local/lib/python2.7/site-packages/flup/server/fcgi_base.py" in _waitForData 147. self._conn.process_input() File "/home/sciyoshi/env2/local/lib/python2.7/site-packages/flup/server/fcgi_base.py" in process_input 711. self._do_unknown_type(rec) File "/home/sciyoshi/env2/local/lib/python2.7/site-packages/flup/server/fcgi_base.py" in _do_unknown_type 824. self.writeRecord(rec) Exception Type: NameError at /account/ Exception Value: global name 'rec' is not defined
This issue happens with the combination of flup==1.0.2, cherokee==1.2.101, and django==1.3. I've already submitted a patch to Cherokee which fixes this on their end, but the edge case may still happen with other servers (an extra FCGI record being sent after the PARAMs). There is a simple fix for this in Django as well, so I'm submitting it here for consideration.
Attachments (1)
Change History (11)
by , 13 years ago
Attachment: | 17187.patch added |
---|
comment:1 by , 13 years ago
comment:3 by , 13 years ago
That's true, but at that point in the code content_length will always be zero, so there is no need to continue trying to read from the underlying stream.
comment:4 by , 13 years ago
Also, I would consider it a regression, since this has been broken since r14394 with no change to either flup or cherokee.
comment:5 by , 13 years ago
Easy pickings: | set |
---|
I second this.
The bug in flup clode was fixed in June, although it's not available as a "release" yet.
See his commit here
I pulled the fix for flup, and what happened as a result is that the request would process in django, but I would get a Cherokee 500 error (ot comming from the django app).
Also this issue doesn't seem to affect only fcgi, and not scgi.
While the bug may stem from an issue with cherokee, it works fine in django 1.2, and broke in the commit listed above.
comment:6 by , 13 years ago
Cc: | added |
---|
A similar issue happens when running against uWSGI. Django attempts to do a read() even if the content-length is explicitly set to zero. The result is that the read() call blocks. The repro in my case is really simple. Get the latest uWSGI, spin up simple django app, send a POST with nothing in it.
There's some good explanation of this issue in Graham Dumpleton's blog: http://blog.dscpl.com.au/2009/10/details-on-wsgi-10-amendmentsclarificat.html
Django should properly distinguish between unknown content-length (when trying to read might make sense) and one explicitly set to zero, when trying to read is a mistake. As per the WSGI spec, it would be wrong for uWSGI to return anything in response to a read() call, even an EOF when it set the content-length to 0.
comment:7 by , 13 years ago
Keywords: | uwsgi added |
---|
comment:8 by , 13 years ago
Easy pickings: | unset |
---|
comment:9 by , 13 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
It looks like this part of the code changed heavily since Django 1.3, the patch doesn't apply.
Based on code inspection, I'd say this problem can't happen anymore -- from WSGIRequest.__init__
:
try: content_length = int(self.environ.get('CONTENT_LENGTH')) except (ValueError, TypeError): content_length = 0 self._stream = LimitedStream(self.environ['wsgi.input'], content_length)
If it still occurs on trunk, could you reopen the ticket and provide a test case? (or at least simple steps to reproduce the problem, involving as few third-party tools as possible).
comment:10 by , 13 years ago
Thanks for looking at this. It looks like LimitedStream
should prevent this from happening again.
The test case could easily be done with a mock, basically if the CONTENT_LENGTH
is explicitly set to 0 in the request, Django should not call read()
on self.environ['wsgi.input']
. If however, the CONTENT_LENGTH
is not properly specified or an invalid integer, Django may still call read()
as per the wsgi discussion I linked to before.
It looks like the code changes have made it so that Django will not call read()
if a CONTENT_LENGTH
is not provided (or is invalid). That's probably the right action to take but should get noted in the release notes since it Django now behaves differently.
or see the commit on github: https://github.com/chideit/django/commit/b29cb53646fafe2e5cf7a258bd80779c9656532a