Re: [MERGE] c++-refactor HttpHdrCc

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.

Thank you,

Alex.
Received on Thu Sep 29 2011 - 15:56:44 MDT