----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22890/#review46449 -----------------------------------------------------------
This mostly looks fine - just a few comments below: Unless you are planning to do this work for windows too you should raise a JIRA for this work on that platform. Is there a JIRA that this relates to? /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp <https://reviews.apache.org/r/22890/#comment81844> [Personal opinion] I think that it would be easier to remember the option if it began with "ssl-" like all the other ssl related options. I'd suggest "ssl-ignore-hostname-verification-failure". /trunk/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp <https://reviews.apache.org/r/22890/#comment81843> I'm not sure this should be at warning level. Since the user had to specifically request this behaviour it is expected, so warning might be too noisy - perhaps info? BTW In the failure case do we log enough information to really figure out what is wrong? If not then maybe we should always go through this callback with the verification flag passed in here somehow. This way we can log a better error message. - Andrew Stitcher On June 23, 2014, 9:03 p.m., Gordon Sim wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22890/ > ----------------------------------------------------------- > > (Updated June 23, 2014, 9:03 p.m.) > > > Review request for qpid, Andrew Stitcher and Pavel Moravec. > > > Repository: qpid > > > Description > ------- > > Adds a connection option ignore_ssl_hostname_verification_failure, which if > set to true will cause a connect attempt to proceed even if the hostname > connecting to does not match the peers certificate. > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/client/ConnectionSettings.h 1604917 > /trunk/qpid/cpp/src/qpid/client/ConnectionSettings.cpp 1604917 > /trunk/qpid/cpp/src/qpid/client/SslConnector.cpp 1604917 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/ConnectionImpl.cpp 1604917 > /trunk/qpid/cpp/src/qpid/messaging/ConnectionOptions.cpp 1604917 > /trunk/qpid/cpp/src/qpid/messaging/amqp/SslTransport.cpp 1604917 > /trunk/qpid/cpp/src/qpid/sys/ssl/SslSocket.h 1604917 > /trunk/qpid/cpp/src/qpid/sys/ssl/SslSocket.cpp 1604917 > > Diff: https://reviews.apache.org/r/22890/diff/ > > > Testing > ------- > > > Thanks, > > Gordon Sim > >
