Review Board 1.7.22


Update isStale method in HttpResponse to return true when max-age equals 0

Review Request #4877 - Created April 25, 2012 and submitted

Xiao Feng Yu
SHINDIG-1759
Reviewers
shindig
brianlil, ddumont, rbaxter, ssievers
shindig
Since max-age=0 means the response expires immediately when the response is generated, and there is no need to compute TTL with possibly incorrect server time.
We can update the isStale method in HttpResponse to return true if max-age equals 0 instead of relying on computed TTL.

 
Review request changed
Updated (April 26, 2012, 2:15 a.m.)
Ship it!
Posted (April 26, 2012, 1:03 p.m.)
LGTM
Posted (April 26, 2012, 2:56 p.m.)
As noted in the JIRA, the issue with the no-cache being returned on the first request, and the (incorrect) cached response on the second request occurs due to the modification of the cached response in the AbstractHttpCache.addResponse.   This code slams the request's refresh interval onto the cache-control headers, and puts that into the cache, while the original request with no-cache is returned to the user.   It seems like this fix would address only one specific issue, that of an incoming request with refresh interval = 0.   If the refresh interval specified  by the client is sufficiently smaller than the time skew between the two servers, wouldn't the same issue occur ?
  1. Hmmm...  
    
    In my experience a max-age on a cache response indicates the maximum period of time before you should toss out the whole cached object and ask again.  Not that you shouldn't ask until you've hit the max-age (which i thought what expires was for).
    
    So should we treat all responses as stale if they come back with max-age so that we can ask with a last-modified query?  I think this is what browsers do.
  2. My interpretation of the HTTP 1.1 spec
     - max-age - you are allowed to reuse without validating until the time expires
     - expires - supposed to be deprecated via 1.1 spec, with cache-control taking precedence .. though of course browsers will do what they will
     - no-cache - can't use the response for caching without re-validating with the server first
    
    Based on that, max-age shouldn't be treated as stale until the time expires
  3. It is possible if the max-age value is sufficiently smaller than the time skew which is maximum to be 3 minutes by default (otherwise Shindig will take the time on Shindig server). But in rare case it would happen in real world - when the server specified max-age, the common value would be in hours or 0.  
Ship it!
Posted (April 27, 2012, 6:02 p.m.)
LGTM
Ship it!
Posted (April 27, 2012, 6:08 p.m.)
Committed r1331528
  1. Please close this review.