[squid-dev] [PATCH] Fetch missing certificates

[Christos Tsantilas]

On 07/11/2016 07:53 PM, Alex Rousskov wrote:
> On 07/11/2016 10:18 AM, Christos Tsantilas wrote:
>> This patch includes a Downloader class which implemented as independent
>> AsyncJob class (in the initial patch was a ConnStateData kid).
>>
>> Currently runs an other related discussion under the mail thread "Care
>> and feeding of ConnStateData", but I believe that this discussion does
>> not affect a decision about applying or not the Downloader class.
>
> I agree. The "care and feeding" thread uses the latest stand-alone
> Downloader design to illustrate class boundaries/relationships, and the
> latest Downloader uses some ideas from that thread as well, but the
> _disagreements_ on that thread do not affect the latest Downloader class
> design. This long-awaited "fetch missing certificates" project is not
> blocked on that thread.
>
>
>> +    // Do not check for redirect, tos,nfmark and sslBump
>> +    http->calloutContext->redirect_done = true;
>> +    http->calloutContext->tosToClientDone = true;
>> +    http->calloutContext->nfmarkToClientDone = true;
>> +    http->calloutContext->sslBumpCheckDone = true;
>
> Why do we not want to do each of those things? I suspect the answer will
> differ for each feature, so we probably should document those reasons
> separately. For example:
>
> // no support for URL rewriting (yet) because ...
> http->calloutContext->redirect_done = true;

Well redirector works well. Looks that there is not any reason to 
disable redirector.



>
> // no Squid-client markings because there is no client connection
> // TODO: Make doCallouts() smarter about client connection presence?
> http->calloutContext->tosToClientDone = true;
> http->calloutContext->nfmarkToClientDone = true;

Also these are not really needed, they just save 1-2 "if" checks.
The doCallouts will check if exists a ConnStateData object and if not 
exists any will not call the related code.

I will remove these lines, if no objection...

>
> // no need to bump because Squid is the true client here
> http->calloutContext->sslBumpCheckDone = true;

We need this line, alternatively I can fix it inside doCallouts method 
(which maybe is better)

>
>
> This feature is useful even without URL rewriting support so we do not
> have to support URL rewrites, especially if adding such is not trivial,
> but I think we need to disclose why we are disabling things.
>
> Alex.