Opened 4 years ago

Closed 4 years ago

#18675 closed Bug (fixed)

If-modified-since for static resources not working.

Reported by: Piotr Czachur Owned by: Simon Charette
Component: HTTP handling Version: master
Severity: Normal Keywords: static serve
Cc: charette.s@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no


How to reproduce

  1. Enable serving static files.
  2. Request same file multiple times.
  3. You always get HTTP "200", but it should be "304" after first request.

What causes the problem

django.views.static.was_modified_since() compares modification time taken from two sources:

  • os.stat()

os.stat() returns timestamp as float (Linux 64bit), and header contains integer timestamp, which results in such comparsions for unmodified files:

if 1331113576.68 > 1331113576:
     // assuming file was modified

which have incorrect result.

was_modified_since() should refuse comparing values of different types, or cast both to integers.

Notice that values returned by os.stat() can be OS-dependent.

Attachments (1)

0001-18675.patch (2.1 KB) - added by Simon Charette 4 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by Simon Charette

Keywords: static serve added
Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Simon Charette
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted
Version: 1.4master

Running Linux 64 bit myself I can confirm os.stat returns a float. I'll write a patch with a failing testcase.

I guess no-one stumbled upon this issue since serve is only meant to be used in development.

Changed 4 years ago by Simon Charette

Attachment: 0001-18675.patch added

comment:2 Changed 4 years ago by Simon Charette

Has patch: set

All tests pass on Python 2.7.3rc1 sqlite3

Last edited 4 years ago by Simon Charette (previous) (diff)

comment:3 Changed 4 years ago by Preston Holmes

Easy pickings: set
Triage Stage: AcceptedReady for checkin

is header_mtime always generated in a way that it is an int - or should it be cast at comparison time as well?

given that small caveat - I'd say this is RFC, I verified tests, and really far more people are using static serve in production behind things like gunicorn on heroku now.

comment:4 Changed 4 years ago by Simon Charette

Cc: charette.s@… added

Looking at the code behind `djangp.utils.http.parse_http_date` it seems that header_mtime can only be an int since it returns the number of seconds since epoch from a datetime with no considerations towards units of magnitude lesser than seconds.

However the docstring clearly states: Returns an[sic] floating point number expressed in seconds since the epoch, in UTC

I think the parse_http_date's docstring should be changed and it's return result also cast to int?

comment:5 Changed 4 years ago by Claude Paroz <claude@…>

In b3ee80a0cf0e60876f03b797d2bdc69505dbdfcb:

Fixed parse_http_date docstring and moved related tests

Refs #18675.

comment:6 Changed 4 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: newclosed

In 3cbe686af6b6a3f23a607b2eb7497c55d88e8345:

Fixed #18675 -- Fixed was_modified_since with floating-point mtime

Thanks Simon Charette for the patch.

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