Opened 6 years ago

Closed 6 years ago

#24158 closed Bug (fixed)

Static serve failure since 1.4.18

Reported by: TheRealEckat Owned by: Tim Graham
Component: HTTP handling Version: 1.4
Severity: Normal Keywords: development
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

With version 1.4.18 a change modified django.views.static.serve at line 65 for reading the file.
HttpResponse is called with the iter object, but this can't do anything with that.
From Django 1.5 the CompatibleStreamingHttpResponse is used at this point.

Reproducible by adding

if settings.DEBUG:
    urlpatterns += patterns('',
        url(r'^media/(?P<path>.*)$', 'django.views.static.serve', {
            'document_root': settings.MEDIA_ROOT,
        }),
   )

to the urls.py an call a file in that path.

The Response status is 200 and the content lenght is transmitted correctly.
In fact, only 0 bytes transferred.

A possible fix is, to change the line to

    response = HttpResponse(''.join(iter(lambda: f.read(STREAM_CHUNK_SIZE), '')), mimetype=mimetype)

Change History (9)

comment:1 Changed 6 years ago by TheRealEckat

Type: UncategorizedBug

comment:2 Changed 6 years ago by Tim Graham

Component: Generic viewsCore (Other)

I can't reproduce this problem on Python 2.7; serving files seems to work just fine. What version of Python are you using? Are you using a browser to request the files or something else?

comment:3 Changed 6 years ago by Tim Graham

Component: Core (Other)Documentation
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Actually, it seems likely that this is caused by one of your middlewares consuming the iterator prematurely, see the 1.5 release notes. We could document this caveat in the 1.4.18 release notes if it seems appropriate. Your proposed change would negate the benefit of the security fix.

comment:4 in reply to:  2 Changed 6 years ago by TheRealEckat

Replying to timgraham:

I can't reproduce this problem on Python 2.7; serving files seems to work just fine. What version of Python are you using? Are you using a browser to request the files or something else?

I use python 2.7.
A colleague can reproduce this problem.

I located a potential problem with the middlewares, but I commented them one by one out.
After first middleware, the length od the response is always zero.
The first two one in my settings are custom middlewares, but the third is the django CsrfViewMiddleware. This middleware does the same, the response length is then zero.

Thanks for the hint with the documentation of django 1.5, I'll check it on monday.

comment:5 Changed 6 years ago by TheRealEckat

UPDATE:

The problem is located in django.middleware.gzip.GZipMiddleware.

If I comment out the gzip middleware, the output is looking good.

comment:6 Changed 6 years ago by Benjamin Richter

I can reproduce this.

Using the GZipMiddleware on 1.4.18 and the static.serve view results in all served files having empty content.

Curiously it works for serving static files via static files contrib app which uses said view as well.

In 1.5 GZipMiddleware was changed to check for Streaming response, same could be backported to 1.4 checking response content for GeneratorType.

comment:7 Changed 6 years ago by Benjamin Richter

Component: DocumentationHTTP handling
Type: Cleanup/optimizationBug

I opened Pull request #3987 with a fix for GZipMiddleware: https://github.com/django/django/pull/3987

I backported django.utils.text.compress_sequence and some of the newer GZipMiddleware code to 1.4 to check for response._base_content_is_iter before consuming response.content and added handling of iterables in responses.

This bug also happens when returning HTTPResponse(iter(...)) in any view.

Is this a feasible solution? Was it okay to recategorize this ticket?
This is my first django ticket I'm working on, so any feedback is very welcome.

Version 0, edited 6 years ago by Benjamin Richter (next)

comment:8 Changed 6 years ago by Tim Graham

Has patch: set
Owner: changed from nobody to Tim Graham
Status: newassigned
Triage Stage: AcceptedReady for checkin

comment:9 Changed 6 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 1e39d0f6280abf34c7719db5e7ed1c333f5e5919:

[1.4.x] Fixed #24158 -- Allowed GZipMiddleware to work with streaming responses

Backport of django.utils.text.compress_sequence and fix for
django.middleware.gzip.GZipMiddleware when using iterators as
response.content.

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