Nancy icon indicating copy to clipboard operation
Nancy copied to clipboard

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

Open PaulBol opened this issue 6 years ago • 5 comments

Prerequisites

  • [x] I have written a descriptive issue title
  • [x] I have verified that I am running the latest version of Nancy
  • [x] I have verified if the problem exist in both DEBUG and RELEASE mode
  • [x] I have searched open and closed issues to ensure it has not already been reported

Description

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

Steps to Reproduce

  • Return GenericFileResponse for file on disk
  • Replace file on disk with a modified date earlier than the current file
  • Return GenericFileResponse for file on disk - server responds with status code 304

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

  • Nancy version: 1.4.5
  • Nancy host
    • [ ] Nancy.Hosting.Aspnet
    • [ ] Nancy.Hosting.Self
    • [ ] Nancy.Owin
    • [ ] Other:
  • Other Nancy packages and versions:
  • Environment (Operating system, version and so on): Windows 10
  • .NET Framework version: 4.5.1
  • Additional information:

PaulBol avatar Oct 22 '18 11:10 PaulBol

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?

logiclrd avatar Nov 07 '18 16:11 logiclrd

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

  • Read timestamp x1 from file system
  • Send value rounded to full seconds to client as Last-Modified value
  • Receive If-Modified-Since from client
  • Upon next request again read timestamp x2 from file system
  • Determine difference between x1 and x2 rounding anything <1 seconds to 0 ((int)(lastModified.Value - requestDate.Value).TotalSeconds cuts off fractional part)

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

PaulBol avatar Dec 10 '18 13:12 PaulBol

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.

logiclrd avatar Dec 10 '18 16:12 logiclrd

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.

PaulBol avatar Dec 11 '18 08:12 PaulBol

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.

logiclrd avatar Dec 11 '18 08:12 logiclrd