New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
shepmaster opened this Issue Jun 27, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@shepmaster
Copy link
Contributor

shepmaster commented Jun 27, 2016

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

This comment has been minimized.

Copy link

TheNeikos commented Aug 15, 2016

@shepmaster

This comment has been minimized.

Copy link
Contributor

shepmaster commented Aug 15, 2016

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

This comment has been minimized.

Copy link
Member

untitaker commented Aug 15, 2016

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

This comment has been minimized.

Copy link

TheNeikos commented Aug 15, 2016

@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

This comment has been minimized.

Copy link
Member

Hoverbear commented Aug 17, 2016

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

@shepmaster

This comment has been minimized.

Copy link
Contributor

shepmaster commented Aug 17, 2016

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

@Hoverbear

This comment has been minimized.

Copy link
Member

Hoverbear commented Aug 17, 2016

LOL. You're right!

@Hoverbear Hoverbear closed this Aug 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment