ancruna / mongoose

Automatically exported from code.google.com/p/mongoose
MIT License
0 stars 0 forks source link

Broken 304 from patch for Issue 269 #270

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

1. Create XX.html
2. Load in browswer
3. Change XX.html
4. Reload in browser, changes does not appear (if timezone is -GMT)

What is the expected output? What do you see instead?

Change should appear, instead you get 304, not modified

What version of the product are you using? On what operating system?

Latest clone from repo,  on linux

Please provide any additional information below.

It seems in the fix to Issue 269 (Revision: 983c674b7cfc ) where the HTTP 
modified headers were changed to GMT to follow RFC 2616 did not go far enough.  
In is_not_modified() the a comparison is made of the HTTP header time (in GMT) 
with the file modify time, which is localtime.  Depending on your timezone, 
this makes it look like your file has not changed, when it actually has. 

My simple fix (for Unix) was to change mg_stat() to make sure to store the file 
modification time in GMT.

-    stp->mtime = st.st_mtime;
+    stp->mtime = mktime(gmtime(&st.st_mtime));

I'm not sure of the proper windows fix, since the gmtime there is currently a 
stub function.

Original issue reported on code.google.com by glac...@tethermedia.com on 4 Aug 2011 at 10:03

GoogleCodeExporter commented 9 years ago
My simple fix above is also not complete.  It seems in handle_file_request() 
the previous patch passes stp->mtime to gmtime() to fill in the Last-Modified 
header.  Since my other line now stores stp->mtime in GMT, that logic no longer 
works, this strftime() should be changed back to local time.

From:
  (void) strftime(lm, sizeof(lm), fmt, gmtime(&stp->mtime));

Now back to:
  (void) strftime(lm, sizeof(lm), fmt, localtime(&stp->mtime));

Obviously, one could also change the logic in is_not_modified() as well to 
assume the stp->mtime is local and the modified-since date string is GMT, 
normalize, then do the math.

Original comment by glac...@tethermedia.com on 4 Aug 2011 at 10:30

GoogleCodeExporter commented 9 years ago
That fix was handling just the headers that were sent to the client.
I don't think it can affect in any way is_not_modified() function.
is_not_modified() takes the modification time returned by stat(), and compares 
it with the parsed If-Modified-Since header, done by parse_date_string().
I suspect, that parse_date_string() has a bug, around daylight saving time. I 
don't have cycles right now  to verify that. If you can do that (just check 
that the parsed date is off by ~ 1 hour), that would be appreciated.

Original comment by valenok on 4 Aug 2011 at 10:47

GoogleCodeExporter commented 9 years ago
parse_date_string() appears to be ok:

 tmp = parse_date_string(ims);
 printf("ims: %s\n",ims);
 printf("parse_date_string time: %s\n",ctime(&tmp));

 ims: Thu, 04 Aug 2011 23:14:23 EDT
 parse_date_string time: Thu Aug  4 23:14:23 2011

Original comment by glac...@tethermedia.com on 5 Aug 2011 at 12:43

GoogleCodeExporter commented 9 years ago
For windows, the fix is to change parse_date_string() to assume the time passed 
is in UTC so it parses correctly and returns a time_t in UTC. Second, 
handle_file_request() should not use "%Z" in the format string for the dates 
since it should be only using UTC. This works because mg_stat(), on Windows at 
least, always gets file info in UTC. If we assume UTC everywhere then this 
problem seems to be resolved.

in handle_file_request()
  const char *fmt = "%a, %d %b %Y %H:%M:%S %Z", *msg = "OK", *hdr;
becomes
  const char *fmt = "%a, %d %b %Y %H:%M:%S GMT", *msg = "OK", *hdr;

in parse_date_string
  // Set Daylight Saving Time field
  current_time = time(NULL);
  tmp = localtime(¤t_time);
  tm.tm_isdst = tmp->tm_isdst;

  return mktime(&tm);
becomes
  // Set Daylight Saving Time field
  // ignored this as we assume UTC time
  //current_time = time(NULL);
  //tmp = localtime(¤t_time);
  //tm.tm_isdst = tmp->tm_isdst;

return _mkgmtime(&tm);

It sounds like mg_stat() in unix may return file mod times in localtime. If 
that is the case then mg_stat() should also be changed to convert the times to 
UTC for unix builds only.

Original comment by jcmay...@gmail.com on 16 Aug 2011 at 8:34

GoogleCodeExporter commented 9 years ago
Submitted 
http://code.google.com/p/mongoose/source/detail?r=1e84fd62ee06c74dd5af10981780d3
a4dc78508d
This fixed parse_date_string() to treat time in UTC.

Original comment by valenok on 26 Aug 2011 at 8:09