----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31277/#review74766 -----------------------------------------------------------
src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java <https://reviews.apache.org/r/31277/#comment121496> synchronized? src/java/main/org/apache/zookeeper/ZooKeeper.java <https://reviews.apache.org/r/31277/#comment121494> i think we can just call this: ``` SECURE_CLIENT ``` i know some static fields in this class have the ZOOKEEPER prefix, but we can probably drop it. this reads clearly enough: ``` ZooKeeper.SECURE_CLIENT ``` src/java/main/org/apache/zookeeper/common/X509Error.java <https://reviews.apache.org/r/31277/#comment121492> i like these exceptions! elegant and well encapsulated! src/java/main/org/apache/zookeeper/common/X509Util.java <https://reviews.apache.org/r/31277/#comment121498> i'd drop ZOOKEEPER from these constants (not from the properties, but from the class constants). i.e.; ``` public static final String SSL_KEYSTORE_LOCATION = "zookeeper.ssl.keyStore.location"; ... ``` is as clear, but also much less verbose and repetitive. src/java/main/org/apache/zookeeper/common/X509Util.java <https://reviews.apache.org/r/31277/#comment121505> But if a user specifies one, location and password are required. src/java/main/org/apache/zookeeper/common/X509Util.java <https://reviews.apache.org/r/31277/#comment121506> do we care? i think this can just be: ```java } catch (IOException e) {} ``` src/java/main/org/apache/zookeeper/common/X509Util.java <https://reviews.apache.org/r/31277/#comment121507> ditto src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121508> nit: simpler: ```java public void configure(InetSocketAddress addr, maxcc, boolean secure) { } ``` src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121509> nit: no need for line wrap src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121510> nit: simpler: ```java public void startup(ZooKeeperServer zks, boolean startServer) { } ``` src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121511> why Exception? can't we throw something a SSL specific exception? src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121513> nit: "SSL handler added" src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121514> boolean secure src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121515> this.secure src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121516> boolean startServer src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121517> startServer src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121518> boolean secure src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121519> int maxcc src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java <https://reviews.apache.org/r/31277/#comment121520> int maxcc, boolean secure - Raul Gutierrez Segales On Feb. 26, 2015, 7:34 p.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31277/ > ----------------------------------------------------------- > > (Updated Feb. 26, 2015, 7:34 p.m.) > > > Review request for zookeeper. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-2125: SSL on Netty client-server communication > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/ClientCnxnSocketNetty.java PRE-CREATION > src/java/main/org/apache/zookeeper/ZooKeeper.java > dd13cc9ba5096312b06999a03ae0057cd3677623 > src/java/main/org/apache/zookeeper/common/X509Error.java PRE-CREATION > src/java/main/org/apache/zookeeper/common/X509Util.java PRE-CREATION > src/java/main/org/apache/zookeeper/server/NIOServerCnxnFactory.java > acabb33f6c7a000706763ccba94cbaf5aaaca08e > src/java/main/org/apache/zookeeper/server/NettyServerCnxnFactory.java > 41268805fe16244aeea4db3f35f13a6987b30187 > 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 > 30a0ed390bb7473ddb36757da97bc7d5f4281887 > 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/quorum/Leader.java > 20589045752a7ba4ae9c9090055a4fcbe86a8eda > src/java/main/org/apache/zookeeper/server/quorum/Learner.java > 4dd1e947357080f3e055f3e7e2a78c979daa6ea7 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java > 388ceeb45bd18c7cb8f0766a96ebd4a54a9e76de > 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/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java > 6ab19b1eb137c8b13b8ad031d474e213267da1ea > src/java/test/org/apache/zookeeper/server/quorum/Zab1_0Test.java > 6ce058e48d17410d89d8348ee659dd7752bfd578 > src/java/test/org/apache/zookeeper/test/ReconfigTest.java > 8b238ee7463508122010208ebc3e786caa2cf1b1 > src/java/test/org/apache/zookeeper/test/SSLTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/31277/diff/ > > > Testing > ------- > > > Thanks, > > Hongchao Deng > >