Re: [PATCH] adaptation_meta option

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 :-)

>
> Amos