Opened 16 years ago

Closed 14 years ago

#10627 closed (duplicate)

Middleware using len() on response.content is gobbling generators

Reported by: FunkyBob Owned by: nobody
Component: Core (Other) Version: dev
Severity: Keywords: middleware
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We recently found that a slow view (4 to 5 minutes) was made even worse because we couldn't give a generator to HttpResponse and have it still work.

This turned out to be because two of the built-in middlewares ( gzip and conditional-get ) assume the response.content is a string, and call len(response.content) -- which consumes the generator.

Once we bypassed these, the time to first response was seconds, and the entire process ran smoothly.

So, attached are two patches to work around this issue:

  1. In the case of conditional-get, it skips setting Content-Length header if not response._is_string.
  2. In the case of gzip, simply move the Content-Encoding test to the front, so the documentation saying gzip will be skipped on anything with a Content-Encoding header works as expected.

Attachments (2)

gzip.py.diff (1.2 KB ) - added by FunkyBob 16 years ago.
Fix for django/middleware/gzip.py
http.py.diff (504 bytes ) - added by FunkyBob 16 years ago.
Fix for django/middleware/http.py

Download all attachments as: .zip

Change History (7)

by FunkyBob, 16 years ago

Attachment: gzip.py.diff added

Fix for django/middleware/gzip.py

by FunkyBob, 16 years ago

Attachment: http.py.diff added

Fix for django/middleware/http.py

comment:1 by FunkyBob, 16 years ago

I realise there are other approaches I could have taken with gzip, but I took the "simple and safe" path.

I expect more discussion would be wanted on how to approach the problem ... such as:

  1. How do you determine if the content from a generator will be worth compressing, or do you just always try/never try?
  2. Do we want the gzip layer consuming generators and compressing, or leave them alone?

In our case, we leave it up to Apache to gzip any content we ask of it, so it's a non-issue.

comment:2 by Jacob, 16 years ago

Resolution: duplicate
Status: newclosed

Duplicate of # 6527

comment:3 by Jacob, 16 years ago

This is part of a general set of "HttpResponse doesn't play well with iterators" bugs (#6027, #6527, ...) that we're aware of. It's not really worth having Yet Another Bug in this series, so I've marked duplicate even those bugs don't directly address this *specific* problem. We'll fix the overall issue, though, and all these will go away.

comment:4 by FunkyBob, 14 years ago

Resolution: duplicate
Status: closedreopened

I just wanted to ping this ticket, since the patches work, pass tests on r15702, and work around the specific issues.

Importantly, the gzip patch makes the gzip middleware more closely match the documentation -- specifically, that the Response will not be processed by gzip middleware if it has Content-Encoding set.

comment:5 by Russell Keith-Magee, 14 years ago

Resolution: duplicate
Status: reopenedclosed

@funkybob - none of what you say changes the fact that this ticket is a duplicate of other tickets. If you want to address this issue, please take the discussion to the original tickets.

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