Re: [PATCH] adaptation_meta option

On 10/28/2011 08:16 PM, Alex Rousskov wrote:
> On 10/28/2011 11:02 AM, Tsantilas Christos wrote:
>> On 10/28/2011 02:57 AM, Amos Jeffries wrote:
>>> On 28/10/11 00:38, Tsantilas Christos wrote:
>>>> On 10/22/2011 04:03 AM, Amos Jeffries wrote:
>>>>> On 22/10/11 03:03, Tsantilas Christos wrote:
>>>>>> This option allows Squid administrator to add custom ICAP request
>>>>>> headers or eCAP options to Squid ICAP requests or eCAP transactions.
>>>>>> Use it to pass custom authentication tokens and other
>>>>>> transaction-state
>>>>>> related meta information to an ICAP/eCAP service.
>>>>>>
>>>>>> The addition of a meta header is ACL-driven:
>>>>>> adaptation_meta name value [!]aclname ...
>>>>>>
>>>>>> Processing for a given header name stops after the first ACL list
>>>>>> match.
>>>>>> Thus, it is impossible to add two headers with the same name. If no
>>>>>> ACL
>>>>>> lists match for a given header name, no such header is added. For
>>>>>> example:
>>>>>>
>>>>>> # do not debug transactions except for those that need debugging
>>>>>> adaptation_meta X-Debug 1 needs_debugging
>>>>>>
>>>>>> # log all transactions except for those that must remain secret
>>>>>> adaptation_meta X-Log 1 !keep_secret
>>>>>>
>>>>>> # mark transactions from users in the "G 1" group
>>>>>> adaptation_meta X-Authenticated-Groups "G 1" authed_as_G1
>>>>>>
>>>>>> The "value" parameter may be a regular squid.conf token or a "double
>>>>>> quoted string". Within the quoted string, use backslash (\) to escape
>>>>>> any character, which is currently only useful for escaping backslashes
>>>>>> and double quotes. For example,
>>>>>> "this string has one backslash (\\) and two \"quotes\""
>>>>>>
>>>>>>
>>>>>> This is a Measurement Factory project
>>>>>
>>>>
>>>> I am sending a new patch here, which is also updated to apply to the
>>>> latest trunk.
>>>>
>>>>>
>>>>> Cool. I like.
>>>>>
>>>>> I just have one doubt on the design end of things. Can we limit it to
>>>>> not overwriting or adding standard headers? rather than just warning.
>>>>> Adding custom headers and ones needed for future protocol extensions is
>>>>> all fine and well. But wrongly adding central protocol headers midway
>>>>> down a chain is an attractive trap newbie admin are always asking about
>>>>> how to do, even if though breaks the final result.
>>>>
>>>> Do we have any decision here? Should we deny overwriting standard
>>>> headers?
>>>>
>>>
>>> +1. This can go in if you want to.
>>>
>>> I am still of the opinion that we should not allow that until someone
>>> comes up with a proper need case. But not strongly enough to veto the
>>> change. I'm okay with the critical-level warnings.
>>>
>>> An alternative is to followup with another patch adding an
>>> --enable-icap-violations option to configure and wrapping some warn vs
>>> reject code in #if ICAP_VIOLATIONS.
>>
>> This is a new version which add the "--enable-icap-violations" option.
>>
>> I think it is OK. If we do not like we can convert it to a
>> "--disable-icap-violations" option :-)
>
>
> Hi Christos,
>
> Sorry, I really do not like another VIOLATIONS macro because while
> it is simple now it is likely to bloat with more code in the future.
> Moreover, as we discussed, not all user-configurable headers with
> standard names are necessarily ICAP violations.
>
> I would very much rather just leave the fatalf() code and remove the
> macro. This will match Amos preferences. While it may not match yours
> and mine, I think we do not have strong ones, and I really hate seeing
> another VIOLATIONS macro added.
>
> If you agree, please commit with the fatalf() code and without
> USE_ICAP_VIOLATIONS.

OK, I will remove the macro and I will commit with the "fatal()"

>
>
> Thank you,
>
> Alex.
>
Received on Fri Oct 28 2011 - 17:34:04 MDT