iron / staticfile

Static file-serving middleware for the Iron web framework.
MIT License
63 stars 56 forks source link

Caching will rarely work on systems that have nanosecond-precision filetimes #75

Closed shepmaster closed 8 years ago

shepmaster commented 8 years ago

Debugging why my caching wasn't caching, I printed out these values:

Timespec { sec: 1467045092, nsec: 368456132 } // last_modified_time
Timespec { sec: 1467045092, nsec: 0 }         // if_modified_since

Looking at the code:

let time = FileTime::from_last_modification_time(&metadata);
Timespec::new(time.seconds() as i64, time.nanoseconds() as i32)

But the HTTP header will never have nanosecond precision (If-Modified-Since:Mon, 27 Jun 2016 16:31:32 GMT), so the comparison will fail unless the nanosecond field happens to be zero.

My local fix is to change the code to

Timespec::new(time.seconds() as i64, 0)

Does that feel like the right solution? I'd be happy to submit a PR to that end.

TheNeikos commented 8 years ago

That seems correct by the spec: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.25

shepmaster commented 8 years ago

That seems correct by the spec

@TheNeikos could you clarify what "that" is in your sentence? You could mean "caching should rarely work" is correct, or "ignoring nanoseconds" is correct.

untitaker commented 8 years ago

We could switch to ETags to preserve nanosecond precision. However, your fix is correct (I assume there is no way to represent nanoseconds in HTTP dates).

TheNeikos commented 8 years ago

@TheNeikos could you clarify what "that" is in your sentence? You could mean "caching should rarely work" is correct, or "ignoring nanoseconds" is correct.

The latter of course

Hoverbear commented 8 years ago

@shepmaster That change LGTM please feel free to make a PR. I'll merge it when the tests pass. :)

shepmaster commented 8 years ago

@Hoverbear uhh, didn't you already merge it? My bad for not linking to this issue though! 😇

Hoverbear commented 8 years ago

LOL. You're right!