Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#7535 closed (fixed)

django.views.static.serve does not properly set Content-Encoding header

Reported by: Kevin Hunter <hunteke@…> Owned by: nobody
Component: Generic views Version: master
Severity: Keywords: content-encoding static view development manage.py
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

Amidst trying to gzip a javascript file, I noted that the interaction between my browser (FF3/Linux) and the manage.py server only showed me the compressed version of the requested file. I believe it's due to not properly setting the Content-Encoding header.

This patch adds the Content-Encoding header to django.views.static.serve as returned by mimetypes.guess_type .

Attachments (3)

static.py.patch (905 bytes) - added by Kevin Hunter <hunteke@… 7 years ago.
Content-Encoding patch for django.views.static.serve
static.py.2.patch (936 bytes) - added by Kevin Hunter <hunteke@…> 7 years ago.
Content-Encoding patch for django.views.static.serve, revision 2
static.py.3.patch (907 bytes) - added by Kevin Hunter <hunteke@…> 7 years ago.
Content-Encoding patch for django.views.static.serve, revision 3, hopefully final

Download all attachments as: .zip

Change History (15)

Changed 7 years ago by Kevin Hunter <hunteke@…

Content-Encoding patch for django.views.static.serve

comment:1 Changed 7 years ago by edgarsj

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 7 years ago by Bastian Kleineidam <calvin@…>

  • Patch needs improvement set

The patch will break if mimetypes cannot guess the content type or the encoding. Specifically this expression:

mimetypes.guess_type(fullpath) or mimetype

will never use the second mimetype variable since .guess_type() returns a 2-tuple which has a boolean value of True.

It is easier (albeit a little longer) to check both content-type and -encoding separately for None and use appropriate fallbacks.

Changed 7 years ago by Kevin Hunter <hunteke@…>

Content-Encoding patch for django.views.static.serve, revision 2

comment:3 Changed 7 years ago by Kevin Hunter <hunteke@…>

Heh, length I am not worried about; I'm just learning. For whatever reason, mimetypes and encodings confuse me. In any event, I was attempting to follow the "compressed" look of the code.

Does this second revision of the patch work then?

comment:4 follow-up: Changed 7 years ago by Bastian Kleineidam <calvin@…>

The patch looks ok. You can shorten the code a bit by assigning a tuple to multiple variables:

mimetype, encoding = mimetypes.guess_type(fullpath) 
if mimetype is None:
    mimetype = 'application/octet-stream'

comment:5 in reply to: ↑ 4 Changed 7 years ago by Kevin Hunter <hunteke@…>

Ah cool, and even one more line?
mimetype = mimetype or 'application/octet-stream'

comment:6 Changed 7 years ago by Bastian Kleineidam <calvin@…>

Yes, the "or" expression would save another line :-)

comment:7 Changed 7 years ago by Kevin Hunter <hunteke@…>

Is this patch good then? I note the "Patch needs improvement" bit is still set so is there anything else I need to do while this is still fresh in my mind? Should I submit .3 of the patch with minification discussed?

comment:8 follow-up: Changed 7 years ago by Bastian Kleineidam <calvin@…>

Yes, add a .3 patch (or replace the .2 one) and delete the improvement checkbox. Thanks for the work.

Changed 7 years ago by Kevin Hunter <hunteke@…>

Content-Encoding patch for django.views.static.serve, revision 3, hopefully final

comment:9 in reply to: ↑ 8 Changed 7 years ago by Kevin Hunter <hunteke@…>

  • Patch needs improvement unset

Alright, thar she blows. v3, hopefully final patch, added. Pleasure doin' business with ya.

comment:10 Changed 5 years ago by adamnelson

  • Patch needs improvement set

Patch needs to be ported to current version of source:/django/trunk/django/views/static.py.

comment:11 Changed 5 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

(In [13868]) Fixed #7535 -- Correctly set Content-Encoding header in static files serving view. Thanks for the report and patch, Kevin Hunter.

comment:12 Changed 5 years ago by jezdez

(In [13869]) [1.2.X] Fixed #7535 -- Correctly set Content-Encoding header in static files serving view. Thanks for the report and patch, Kevin Hunter.

Backport from trunk (r13868).

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