Middleware accessing HttpResponse.content breaks streaming HttpResponse objects.
|Reported by:||Tai Lee||Owned by:||ccahoon|
|Severity:||Normal||Keywords:||stream HttpResponse Content-Length|
|Cc:||real.human@…, mmitar@…, mindsocket||Triage Stage:||Accepted|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
CsrfMiddleware all access response.content directly or indirectly. This prevents a streaming response from being initiated until any generator passed to the
HttpResponse as content is consumed, which can cause a timeout when trying to stream large dynamically generated content.
I've already put together a patch based on the assumption that
False indicates content has been passed in as a dynamic generator and thus we shouldn't delay streaming a response to the browser while consuming the entire generator.
The patch implements the following:
- Allow middleware to assign a new generator to
HttpResponse.contentwithout consuming it (e.g.
- Compress chunks of content yielded by
GZipMiddlewareto allow streaming GZipped content
- Only generate an ETag in
- Only check that the length of
HttpResponse.contentis less than 200 in
- Only set the Content-Length header in
CommonMiddleware enabled by default and breaking the streaming response functionality if ETags are enabled in
settings.py, I'd consider this a bug. It can be worked around by manually specifying a bogus ETag before returning the response, which doesn't seem ideal.
With this patch, users still have the option of consuming a generator before passing it to the
HttpResponse in order to enable ETag and Content-Length headers, and conditional GZipping when the content length is less than 200.
CsrfMiddleware, the generator is only consumed when the Content-Type header is
application/xhtml+xml, which may be an acceptable compromise - no streaming response for HTML content if you choose to use
This patch accesses
HttpResponse._container from external middleware classes. Perhaps these properties could be made public and/or renamed to be more logical in this context?
Change History (57)
comment:1 Changed 8 years ago by
|milestone:||1.0 → post-1.0|
|Patch needs improvement:||unset|
|Triage Stage:||Unreviewed → Design decision needed|