Code

#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

Description

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()
  • HTTP_IF_MODIFIED_SINCE header.

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 21 months ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 21 months 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 21 months ago by charettes

comment:2 Changed 21 months ago by charettes

  • Has patch set

All tests pass on Python 2.7.3rc1 sqlite3

Last edited 21 months ago by charettes (previous) (diff)

comment:3 Changed 21 months 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 21 months 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 19 months ago by Claude Paroz <claude@…>

In b3ee80a0cf0e60876f03b797d2bdc69505dbdfcb:

Fixed parse_http_date docstring and moved related tests

Refs #18675.

comment:6 Changed 19 months 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.