NancyFx / Nancy

Lightweight, low-ceremony, framework for building HTTP based services on .Net and Mono
http://nancyfx.org
MIT License
7.15k stars 1.47k forks source link

GenericFileResponse returns NotModified when file has been replaced with an older file #2938

Open PaulBol opened 5 years ago

PaulBol commented 5 years ago

Prerequisites

Description

GenericFileResponds returns NotModified when a previously returned file has been replaced with an older file.

Steps to Reproduce

Apparently GenericFileResponse is designed to behave like this in CacheHelpers.ReturnNotModified:

var requestDate = context.Request.Headers.IfModifiedSince;
if (requestDate.HasValue && lastModified.HasValue && ((int)(lastModified.Value - requestDate.Value).TotalSeconds) <= 0)

The difference will become negative when lastModified is before IfModifiedSince. From my perspective the check should be != 0.

System Configuration

logiclrd commented 5 years ago

It's not my code, but just to chime in, I think the "< 0" might be there to account for inaccuracies in some file systems where the time stamp might be off by a few seconds (or even less than a second) just because it lacks the resolution to store it accurately. The TotalSeconds property of TimeSpan is actually a double to allow fractional values. So, != 0 is almost certainly not the right test, but perhaps a test of "within 2 seconds either way" would be reasonable?

PaulBol commented 5 years ago

The check "within 2 seconds" should work as I see it. That would be Math.Abs((int)(lastModified.Value - requestDate.Value).TotalSeconds) <= 2

However, I tend to disagree that the != 0 test is wrong. The logic is

If we assume the file system reliably returns the same value for untouched files I don't see how the result could be != 0

logiclrd commented 5 years ago

Hmm, so thinking about this further, the following sequence comes to mind:

1) User is running an OS using a FAT-based filesystem. Server is using a much more reasonable file system. 2) User's client requests resource, server tells the client that the resource was last modified at 4:00:15 PM. 3) Client writes file to cache directory and updates the file's date/time to 4:00:15 PM. 4) Because the filesystem is FAT, it can't store 15 seconds, only 14 or 16. So, on the user's system, the file's date/time becomes 4:00:14 PM. 5) User repeats request some time later. File date/time is read from the cache directory. 6) Server is asked whether the resource has been modified since 4:00:14 PM. Actual resource date is 4:00:15 PM.

PaulBol commented 5 years ago

I don't think Nancy needs to worry about the client changing the time. See https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html:

Note: When handling an If-Modified-Since header field, some servers will use an exact date comparison function, rather than a less-than function, for deciding whether to send a 304 (Not Modified) response. To get best results when sending an If- Modified-Since header field for cache validation, clients are advised to use the exact date string received in a previous Last- Modified header field whenever possible.

Note: If a client uses an arbitrary date in the If-Modified-Since header instead of a date taken from the Last-Modified header for the same request, the client should be aware of the fact that this date is interpreted in the server's understanding of time. The client should consider unsynchronized clocks and rounding problems due to the different encodings of time between the client and server. This includes the possibility of race conditions if the document has changed between the time it was first requested and the If-Modified-Since date of a subsequent request, and the possibility of clock-skew-related problems if the If-Modified- Since date is derived from the client's clock without correction to the server's clock. Corrections for different time bases between client and server are at best approximate due to network latency.

I read this as: Clients, you better send the Last-Modified value as received. If you change it you're on your own - don't expect the server to make corrections for you if you fiddle around with the value.

logiclrd commented 5 years ago

That is very reasonable. :-) Given that, if the date round-trips with the exact string, then I concur that an exact date comparison is appropriate.