Opened 3 years ago

Closed 3 years ago

#18675 closed Bug (fixed)

If-modified-since for static resources not working.

Reported by: zimnyx Owned by: charettes
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 charettes 3 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 3 years ago by charettes

  • Keywords static serve added
  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to charettes
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.4 to master

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 3 years ago by charettes

comment:2 Changed 3 years ago by charettes

  • Has patch set

All tests pass on Python 2.7.3rc1 sqlite3

Last edited 3 years ago by charettes (previous) (diff)

comment:3 Changed 3 years ago by ptone

  • Easy pickings set
  • Triage Stage changed from Accepted to Ready 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 3 years ago by charettes

  • 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 3 years ago by Claude Paroz <claude@…>

In b3ee80a0cf0e60876f03b797d2bdc69505dbdfcb:

Fixed parse_http_date docstring and moved related tests

Refs #18675.

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

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

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