[squid-dev] [PATCH] Fix reopened bug 2833

[Alex Rousskov]

On 05/22/2017 10:26 AM, Alex Rousskov wrote:
> On 05/08/2017 08:03 AM, Eduard Bagdasaryan wrote:
>> This patch fixes [reopened] bug 2833.
>>
>> A security fix made in r14979 had a negative effect on collapsed
>> forwarding. All "private" entries were considered automatically
>> non-shareable among collapsed clients. However this is not true: there
>> are many situations when collapsed forwarding should work despite of
>> "private" entry status: 304/5xx responses are good examples of that.
>> This patch fixes that by means of a new StoreEntry::shareableWhenPrivate
>> flag.
>>
>> The suggested fix is not complete: to cover all possible situations we
>> need to decide whether StoreEntry::shareableWhenPrivate is true or not
>> for all contexts where StoreEntry::setPrivateKey() is used. This patch
>> fixes only few important cases inside http.cc, making CF (as well
>> collapsed revalidation) work for some [non-cacheable] response status
>> codes, including 3xx, 5xx and some others.
>>
>> Also: avoid sending 304 responses for non-conditional requests.
>> Before this change, the original 'non-conditional' HttpRequest was still
>> marked (and processed) as 'conditional' after revalidation completion.
>> That happened because 'Last-Modified' and 'ETag' values were not
>> saved/restored while performing internal revalidation request.

> I did not see any reviews for this patch. I plan to commit it if there
> are no last-minute objections.
 Committed to v5 (r15168 and 15169).

I extracted the "Also:..." part into a separate commit because:

* It looked buggy -- the absent lastmod value is -1, not 0 (fixed).
* More work is needed to finish this work IMO (added TODO to commit).
* Bug 4606 has a pending patch containing the same change.

Normally, I would not even commit this second part under these
conditions, but I was worried that if I do not commit it, then the
primary fix (v5 r15168) will suddenly expose the "sending HTTP/304
responses to unconditional requests" problem in a major way.

Alex.