Re: [Squid Web Proxy Wiki] Update of "Features/AddonHelpers" by ChristosTsantilas

On 09/21/2012 03:22 AM, Amos Jeffries wrote:
> On 21/09/2012 6:13 a.m., Alex Rousskov wrote:
>> On 09/20/2012 04:54 AM, Tsantilas Christos wrote:
>>> On 09/20/2012 09:40 AM, Amos Jeffries wrote:
>>>> On 20/09/2012 9:40 a.m., wiki_at_wiki.squid-cache.org wrote:
>>>>
>>>>> + === SSL server certificate validator ===
>>>>> +
>>>>> + ## start sslcrtvd protocol
>>>>> + This interface is similar to the SSL certificate generation
>>>>> interface.
>>>>> +
>>>>> + Input ''line'' received from Squid:
>>>>> + {{{
>>>>> + request size [body]
>>>>> + }}}
>>>>> +
>>>> Please make sure this interface is supporting concurrency. The helper
>>>> core routines do.
>>> The helperSubmit method used to submit request so looking in the code
>>> looks that both SSL certificate generation and validation helper
>>> interfaces should support concurrency.
>>> But I did not check it. Todo...
>> Amos, please correct me if I am wrong, but I think we need to add some
>> kind of request ID field to the validation request and validation
>> response messages for future helpers to be able to support concurrent
>> requests. If the existing trunk code does that automagically, great. If
>> not, let's add a new field and make sure that the sample helper returns
>> it in the response (so that Squid can match requests with responses).
>>
>> The sample helper does not have to support concurrent requests, of
>> course.
>
> The helper.* code already supports concurrency channels with an optional
> ID field, as documented in all the other helper protocols. This is
> controlled directly by the HelperChildConfig object field values from
> squid.conf. It's only a matter of whether you use HelperChildConfig to
> parse the config settings, and documenting the "[channel-ID] " field in
> the wiki same as all the other helpers.
>
> The calling code can, if it wishes, make multiple calls to
> helperSubmit() with different values. You can utilize a request-ID
> inside the callback data params if you wish to collate requests and
> responses. The Auth::UserRequest and child objects in authentication are
> a somewhat abused form of this.

Thank you for explaining this. We will add channel-ID key:value pair to
the helper message syntax on the wiki. It is a petty the existing
general helper code does not handle collating. I wonder if we should
submit concurrency support as a separate patch, after the baseline code
is reviewed and polished? It would be easier to review that as a
separate feature IMO.

>>>> What about tag= to tag the transaction with a meta low/group tag?
>>>> What about passing an existing transaction tag to the helper?
>>>> What about log= ? (ie a loggable compaction of all those errors)
>>> At this time we do not need to tag or log transactions.
>> If other helpers support tags, let's add that support (in both
>> directions). Otherwise, this can be left as a future feature or even
>> ignored if a better feature is available (see below).
>
> So far it is just external ACL interface. I am hoping to make it
> generic, so if it fits and has a use there as well it would be good. You
> could leave it off and get it for free when this interface is converted
> along with the others to using HelperReply object responses instead of
> string responses.

OK, given the current status, I think we should not add tag support to
the certificate validation helper. We should discuss how to add meta
headers support instead (tag is just a primitive form of meta headers: a
single anonymous header field [value]).

Thank you,

Alex.

>> As for "log", a much better mechanism would be meta headers, IMO: They
>> support more than one named entry, they can be matched using ACLs, they
>> can be passed to adaptation services, and they can be logged, all using
>> existing code (patch posted to squid-dev earlier). I suggest that this
>> is revisited as a separate project where we can discuss the right helper
>> message syntax to send and receive meta headers.
>
> Ack.
>
> Amos
Received on Fri Sep 21 2012 - 15:00:26 MDT