On 11/11/2012 07:39 PM, Amos Jeffries wrote: > Are there any objections to committing this patch? > > http://bugs.squid-cache.org/attachment.cgi?id=2570&action=diff
Yes, there are. AFAICT, the patch does two things: 1) Patched code assumes that a _valid_ Date header is always present when HttpReply::hdrCacheInit() is called (but does not verify that assumption with an assert or some such). 2) Patched code adds a Date header to _some_ HTTP responses if a Date header (valid or _not_) is not already present. AFAICT, this is done after HttpReply::hdrCacheInit() has been called. Patched code does not add Date to all HTTP responses, and when it adds it, it does so too late. The changes result, among other things, in HttpReply::hdrExpirationTime() setting incorrect cached expires time in some cases (using negative date value in calculations). > Inserting Date: header on replies where it is missing. Using Age and > Last-Modified to infer dates older than current timestamp for > conservative expiry behaviour. Doing so before caching (AFAICT) to > improve HIT ratio. Perhaps I do not understand the true intention of this patch, but if the goal was to set HttpReply::expires value (calculated by hdrExpirationTime), then HttpReply::hdrExpirationTime() should have been modified accordingly. I have not reviewed the inference logic itself. HTH, Alex.
