----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30753/#review72672 -----------------------------------------------------------
src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml <https://reviews.apache.org/r/30753/#comment118723> Netty usage is pluggable. In that case the SSL feature will be enabled when user user plugged-in zookeeper.serverCnxnFactory, zookeeper.clientCnxnSocket as Netty. isn't it? Better we could document this also. src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/30753/#comment118725> It should be enclosed with else condition, right? I mean if zookeeper.secureClientCnxn is not configured then set 'pipeline.addLast("handler", new ZKClientHandler());' src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118726> please include class level javadocs. Thanks! src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118731> please add javadoc src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118727> Should we suppress these exceptions? I think it good to notify the caller about the error condition by re-throwing the Exception. If requires you can create new Exception type also. src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118730> please add javadoc src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118729> Same as above. Its good to throw Exception back to the caller rather than suppressing it. src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118732> please add javadoc src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118728> As per the #createKeyManager method implementation it can return null. I suggest #createKeyManager can throw exception if not found any match instead of return null. whats your opinion? src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118733> Will the ssl work without trustmanagers, if not better throw exception to the caller rather than suppressing. src/java/main/org/apache/zookeeper/auth/X509Auth.java <https://reviews.apache.org/r/30753/#comment118735> As per the #createTrustManager method implementation it can return null. I suggest #createTrustManager can throw exception if not found any match instead of return null. whats your opinion? - Rakesh R On Feb. 7, 2015, 12:40 a.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30753/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2015, 12:40 a.m.) > > > Review request for zookeeper, fpj and Ian Dimayuga. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-2094 > SSL feature on Netty > > > Diffs > ----- > > build.xml e4c8e0374b25a5bcab7cfe77543378fdb8f98b06 > src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml > 914a24471b0a27f7cf650c3ed2eef1077178853f > src/docs/src/documentation/content/xdocs/zookeeperProgrammers.xml > 223cf8e5a856aefba6f5c106d3f4861d3de8f1e1 > src/docs/src/documentation/content/xdocs/zookeeperStarted.xml > b24b17ad5254321affcaef7b7ecb3a73fc266f84 > src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java > 87e7834bc91c52d4a2d100dbcc98d41a1b98b849 > src/java/main/org/apache/zookeeper/ZooKeeper.java > dd13cc9ba5096312b06999a03ae0057cd3677623 > src/java/main/org/apache/zookeeper/ZooKeeperMain.java > 496e88748cf6aa291c8b71583a28fdb55fdf7761 > src/java/main/org/apache/zookeeper/auth/X509Auth.java PRE-CREATION > src/java/main/org/apache/zookeeper/client/FourLetterWordMain.java > e41465ab93a3a59dbced8294e83b1651ad0dfe69 > src/java/main/org/apache/zookeeper/server/NIOServerCnxn.java > e02753f4fb926a8cc6c7a7c10af42f949c1e210c > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java > acabb33f6c7a000706763ccba94cbaf5aaaca08e > src/java/main/org/apache/zookeeper/server/NettyServerCnxn.java > b4bdc82f8b52f736c6ee3d67bb793a3616c1b436 > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 09a794844978456fc3580adc22b6064e2a12cf77 > src/java/main/org/apache/zookeeper/server/ServerCnxn.java > a47d85662970cc0c219a46b226737a8689f8fe96 > src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java > 14037722c569d560acef56de0b5a7ae13464128c > src/java/main/org/apache/zookeeper/server/ServerConfig.java > f2b8463e871739319bdf40be1f014d5ad0af5602 > src/java/main/org/apache/zookeeper/server/ZooKeeperServer.java > 09469914395c060d954e8ff8e364612e37d77953 > src/java/main/org/apache/zookeeper/server/ZooKeeperServerBean.java > 0eb5c7f979199f2f7dcb9e5cfa011a9b20113713 > src/java/main/org/apache/zookeeper/server/ZooKeeperServerMain.java > b756d349abeb1fc69534100c3633db4c1c18e031 > src/java/main/org/apache/zookeeper/server/auth/ProviderRegistry.java > 406015f84a51e6afcfe704b881f8494bdd687a25 > > src/java/main/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java > PRE-CREATION > src/java/main/org/apache/zookeeper/server/quorum/Leader.java > 9dc0424c2cfa03380147ed6094bfb0f73c877731 > src/java/main/org/apache/zookeeper/server/quorum/Learner.java > 87f4c0627141c2cfee0533aca7ba2e7ff91433e3 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java > 04e84eeee8b64c8eb947fa07911d8839aad17e34 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java > badc8df1f05dea4be337bc8312d7ac22f6c77dc3 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > d17c58d59e0131a78adde1becb5c23ce8c7a16a7 > > src/java/main/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java > 2aab6d09f9bd980ed76f886fb8168aae2ac8f99f > src/java/test/data/auth/test.cert PRE-CREATION > src/java/test/data/auth/testKeyStore.jks PRE-CREATION > src/java/test/data/auth/testTrustStore.jks PRE-CREATION > src/java/test/data/auth/testUntrustedKeyStore.jks PRE-CREATION > src/java/test/org/apache/zookeeper/server/TestServerCnxn.java PRE-CREATION > src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java > 6ce058e48d17410d89d8348ee659dd7752bfd578 > src/java/test/org/apache/zookeeper/test/ClientBase.java > 5225a5099d4a8fc2b04f96e305350422ef5c16dc > src/java/test/org/apache/zookeeper/test/ReconfigTest.java > 8b238ee7463508122010208ebc3e786caa2cf1b1 > src/java/test/org/apache/zookeeper/test/SSLAuthTest.java PRE-CREATION > src/java/test/org/apache/zookeeper/test/X509AuthTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/30753/diff/ > > > Testing > ------- > > > Thanks, > > Hongchao Deng > >
