----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7171/#review11696 -----------------------------------------------------------
/proton/trunk/proton-c/bindings/python/python.i <https://reviews.apache.org/r/7171/#comment25274> Ooo, I didn't know review-board highlited extraneous whitespace automatically. That's awesome. I hate extraneous whitespace. ;-) /proton/trunk/proton-c/include/proton/driver.h <https://reviews.apache.org/r/7171/#comment25276> I don't think we need this if we add public ssl constructors as mentioned below. I know I have something similar for sasl, but that was added just for backwards compatibility. I think we should keep the connector API narrow and just add an accessor for the transport and let people configure the transport however they want (e.g. sasl/ssl/whatever). /proton/trunk/proton-c/include/proton/ssl.h <https://reviews.apache.org/r/7171/#comment25275> I notice this file doesn't expose the ssl construtors directly. I think we actually should so that people can build their own drivers and easily support ssl. I also think it will be useful in testing since we can directly drive an ssl configured transport from the tests and create all sorts of different permutations of interaction deterministically rather than waiting for them to come up by chance via a real driver. /proton/trunk/proton-c/src/ssl/openssl.c <https://reviews.apache.org/r/7171/#comment25277> It might follow the naming conventions a little better if we had a single constructor named pn_ssl and passed in a flag or enum to determine server vs client. I'm not all that fussed on this point though. /proton/trunk/proton-c/src/ssl/openssl.c <https://reviews.apache.org/r/7171/#comment25279> For sasl I actually just return the existing one here. My thinking was that it would be convenient to be able to access the existing sasl layer given a transport, however I didn't want to have the transport's public API depend on sasl at all. However I can see how you might want the option to not automatically create the given security layer, so maybe the best of both worlds would be to have a separate accessor on the sasl side or a flag on the constructor that controls whether the layer is created automatically. Whatever we do we should probably be consistent between the two security layers (ssl & sasl). /proton/trunk/proton-c/src/ssl/openssl.c <https://reviews.apache.org/r/7171/#comment25280> General question, have you run proton-tests under valgrind? If not, probably a good idea just to check all this silly free stuff. - Rafael Schloming On Sept. 19, 2012, 4:11 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7171/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2012, 4:11 p.m.) > > > Review request for qpid and Rafael Schloming. > > > Description > ------- > > Patch to add SSL/TLS support to proton using OpenSSL. > > > This addresses bug PROTON-2. > https://issues.apache.org/jira/browse/PROTON-2 > > > Diffs > ----- > > /proton/trunk/examples/mailbox/README.txt 1387654 > /proton/trunk/examples/mailbox/fetch 1387654 > /proton/trunk/examples/mailbox/post 1387654 > /proton/trunk/examples/mailbox/server 1387654 > /proton/trunk/examples/mailbox/ssl-setup.sh PRE-CREATION > /proton/trunk/proton-c/CMakeLists.txt 1387654 > /proton/trunk/proton-c/bindings/php/php.i 1387654 > /proton/trunk/proton-c/bindings/python/python.i 1387654 > /proton/trunk/proton-c/bindings/ruby/ruby.i 1387654 > /proton/trunk/proton-c/include/proton/cproton.i 1387654 > /proton/trunk/proton-c/include/proton/driver.h 1387654 > /proton/trunk/proton-c/include/proton/ssl.h PRE-CREATION > /proton/trunk/proton-c/pn_config.h.in 1387654 > /proton/trunk/proton-c/src/driver.c 1387654 > /proton/trunk/proton-c/src/engine/engine-internal.h 1387654 > /proton/trunk/proton-c/src/engine/engine.c 1387654 > /proton/trunk/proton-c/src/ssl/openssl.c PRE-CREATION > /proton/trunk/proton-c/src/ssl/ssl-internal.h PRE-CREATION > /proton/trunk/proton-c/src/ssl/ssl_stub.c PRE-CREATION > > Diff: https://reviews.apache.org/r/7171/diff/ > > > Testing > ------- > > "proton-tests" still works :) > > Updated "mailbox" example to support SSL - see mailbox/README.txt for > instructions on how to set up and run with SSL. > > > Thanks, > > Kenneth Giusti > >
