Re: [MERGE] c++-refactor HttpHdrCc

On Thu, Sep 29, 2011 at 5:55 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> On 09/29/2011 09:10 AM, Kinkie wrote:
>
>> How about proceeding like this?
>> This has already grown way past a "playground" or "refactoring" job
>> (it changes some behaviour).
>> We merge as-is, and then change the behaviour in a follow-up commit to trunk.
>
> Sounds good to me.
>
>
>>>> Also, wouldn't that make the parser unnecessarily strict?
>>>
>>> Not sure what you mean. We got a malformed max-stale value. We have only
>>> two options when it comes to that value interpretation, I think:
>>>
>>>    A1. Treat it as valueless max-stale.
>>>    A2. Ignore it completely.
>>>
>>> Since valueless max-stale results in possibly very stale content being
>>> served to an unsuspecting client, I think #2 is much safer (and
>>> compliant with HTTP).
>>
>> You know way more than I do about this, so ok.
>>
>>> BTW, we have three options when it comes to forwarding the malformed
>>> directive:
>>>
>>>    B1. Forward our own valid directive.
>>>    B2. Forward nothing.
>>>    B3. Forward the malformed directive.
>>>
>>> While option B3 could be the best, it requires more parsing changes.
>>> Option B2 is much better than B1, IMO, and requires minimal changes.
>>>
>>> We currently implement A1 and B1. It is OK to treat this problem as
>>> outside your patch scope. I just hope the above summary will help
>>> somebody fix this later.
>>
>> Implementing B3 would be trivial, if we are allowed to just forward Cc
>> directives as we received them (I didn't dare do it due to my lack of
>> in-depth HTTP compliance knowledge).
>>
>> I will gladly do it as a followup patch as the other fix above.
>> Do you agree?
>
> Sure, but I doubt correct B3 implementation would be trivial.
>
> It would be indeed relatively easy to forward a malformed max-stale
> directive "as is" using the "other" member, but if Squid then sets its
> own max-stale, then we would be forwarding two max-stales, which is not
> kosher. Similarly, if we gain some code to filter directives (e.g.,
> using squid.conf ACLs), this simple B3 implementation would not filter
> malformed ones.
>
> It would be OK to ignore all these problems if we could somehow prevent
> new/future Squid code from bumping into them accidentally, but I cannot
> think of a neat way to do that.
>
> IMO, we should not attempt B3 and stick with B2 for now. B2 is trivial.

Ok.
If that's fine by you then I'll push ahead the merge of the
currently-implemented feature set to this evening, and start working
on A2 (B2 is a natural consequence of it).

-- 
    /kinkie