Re: [MERGE] c++-refactor HttpHdrCc

 On Thu, 29 Sep 2011 17:59:13 +0200, Kinkie wrote:
> 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).

 B2 is not a valid option AFAIK. HTTP does not offer the option A2 as
 you are discussing right now.
 Since we know the header we must either treat it as an option known
 (A1) or something unknown (A3).

  * A3 means we can ignore it for operational purposes (like A2), the
 natural result being B3.

 A1 means we have to decide betrween B1 or B3. Treating it as a known
 directive which has been extended with some unknown extension value
 (B3). Or as definitely malformed and invalid (B1).

 Amos
Received on Sat Oct 01 2011 - 03:30:55 MDT