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., [email protected] 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
