Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#17187 closed Bug (needsinfo)

Incompatibility with Cherokee webserver's FastCGI handling

Reported by: Samuel Cormier-Iijima Owned by: sam@…
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)

17187.patch (763 bytes ) - added by Samuel Cormier-Iijima 13 years ago.

Download all attachments as: .zip

Change History (11)

by Samuel Cormier-Iijima, 13 years ago

Attachment: 17187.patch added

comment:2 by Alex Gaynor, 13 years ago

All I see is a NameError, which is clearly a bug in flup's code.

comment:3 by Samuel Cormier-Iijima, 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 Samuel Cormier-Iijima, 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 Amin Mirzaee, 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 Leo Shklovskii, 13 years ago

Cc: leos@… added

A similar issue happens when running against uWSGI. Django attempts to do a read() even if the content-length us explicitly set to zero. The result is that the read() call blocks.

It should properly distinguish between unknown content-length (when trying to read might make sense) and one explicitly set to zero.

The repro in my case is really simple. Get the latest uWSGI, spin up simple django app, send a post with nothing in it.

Version 0, edited 13 years ago by Leo Shklovskii (next)

comment:7 by Leo Shklovskii, 13 years ago

Keywords: uwsgi added

comment:8 by Leo Shklovskii, 13 years ago

Easy pickings: unset

comment:9 by Aymeric Augustin, 13 years ago

Resolution: needsinfo
Status: newclosed

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 Leo Shklovskii, 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.

Note: See TracTickets for help on using tickets.
Back to Top