Opened 12 years ago

Closed 12 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: dev
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 Simon Charette 12 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 by Simon Charette, 12 years ago

Keywords: static serve added
Owner: changed from nobody to Simon Charette
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.

by Simon Charette, 12 years ago

Attachment: 0001-18675.patch added

comment:2 by Simon Charette, 12 years ago

Has patch: set

All tests pass on Python 2.7.3rc1 sqlite3

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

comment:3 by Preston Holmes, 12 years ago

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 by Simon Charette, 12 years ago

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

In b3ee80a0cf0e60876f03b797d2bdc69505dbdfcb:

Fixed parse_http_date docstring and moved related tests

Refs #18675.

comment:6 by Claude Paroz <claude@…>, 12 years ago

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