Re: [PATCH] adaptation_meta option

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.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.16
   Beta testers wanted for 3.2.0.13