Re: [PATCH] cert validation cache

On 12/11/2012 06:02 PM, Alex Rousskov wrote:
> On 12/11/2012 03:50 AM, Amos Jeffries wrote:
>> On 11/12/2012 9:19 p.m., Tsantilas Christos wrote:
>>> If there is not any objection I will apply the latest "cert validation
>>> cache" patch to trunk.
>
>> This patch is also threaded with "#if 1 // USE_SSL_CERT_VALIDATOR" just
>> like the other one and will need re-testing without it.
>
> I think we should either use proper USE_SSL_CERT_VALIDATOR conditional
> or make this code unconditional. Iff nobody has strong opinions about
> it, I suggest making this code unconditional (no #ifs).

This "#if" originally added to help us keep track of the code we added
for this feature. It was just a development step.
It is not really needed.

If we agree to remove these "#if" statements I will remove them with a
separate patch to trunk after commit this one.

>
> The certificate validator is not enabled by default and the extra code
> does not add a lot of performance overhead, does it?

Nop. Just an if(Ssl::TheConfig.ssl_crt_validator) before ssl initialised
and an other one if an SSL error found on certificate.

>
> The only reason I can see for making this code conditional is to be able
> to disable it if this patch breaks current built-in validator somehow
> (even when the optional validation helper is disabled). However, perhaps
> it is better to learn about such bugs sooner because they will bite us
> anyway.
>
> Any other reasons for disabling this code (and not just the feature)?
>
>
> Thank you,
>
> Alex.
>
>
Received on Tue Dec 11 2012 - 16:40:48 MST