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.
Received on Tue Nov 13 2012 - 00:14:09 MST
This archive was generated by hypermail 2.2.0 : Tue Nov 13 2012 - 12:00:06 MST