Re: [PATCH] Support bump-ssl-server-first and mimic SSL server certificates

On 13/07/2012 5:30 a.m., Tsantilas Christos wrote:
> This is a new version of the patch.
> Not all of Amos comments handled please see my comments bellow.
>
>
> On 07/11/2012 03:38 PM, Amos Jeffries wrote:
>> src/cf.data.pre:
>> * Please use "IF " instead of "IFDEF " for the new inline #if macro. It
>> is confusing to tell whether any particular one should be IFDEF<space>
>> or IFDEF<colon>.
> I made this change but I have my doubts.
> The IFDEF is a reserved word which already used by cf_gen parser so it
> doesn't hurt to use it in a new form. The "if" is a valid statement of
> squid configuration file.

The other form though is a very different meaning - the entire directive
block of details is elided and replaced with instructions on what to
rebuild Squid with.
If this form were interchanged with the other form we would end up with
whole block sections of unrelated code missing. We *really* don't want
these two to get confused in the future misunderstanding.

>
>> src/client_side.h:
>> * setServerBump(Ssl::ServerBump *srvBump) has no else condition
>> reporting or handling when setting fails.
> It should not called if the bump mode already decided. Also we have
> other debug messages in other places to inform users for this
> bug/problem. To print the message in setServerBump we should convert
> this method to a non inline method. Does it worth?

Not worth it then. Can we assert(sslServerBump == srvBump) to catch
future abuse?

Ack, on the rest about pinning.

Amos
Received on Fri Jul 13 2012 - 01:13:49 MDT