ZOOKEEPER-236. Nit code review fixes

src/java/main/org/apache/zookeeper/common/X509Util.java

@@ -195,7 +195,8 @@ public SSLContext createSSLContext(ZKConfig config) throws SSLContextException {
 
         boolean sslCrlEnabled = config.getBoolean(this.sslCrlEnabledProperty);
         boolean sslOcspEnabled = config.getBoolean(this.sslOcspEnabledProperty);
-        boolean sslServerHostnameVerificationEnabled = config.getBoolean(this.getSslHostnameVerificationEnabledProperty(), true);
+        boolean sslServerHostnameVerificationEnabled =
+                config.getBoolean(this.getSslHostnameVerificationEnabledProperty(),true);
         boolean sslClientHostnameVerificationEnabled = sslServerHostnameVerificationEnabled && shouldVerifyClientHostname();
 
         if (trustStoreLocationProp == null) {
@@ -278,7 +279,6 @@ public static X509TrustManager createTrustManager(String trustStoreLocation, Str
                 if (ocspEnabled) {
                     Security.setProperty("ocsp.enable", "true");
                 }
-
             } else {
                 pbParams.setRevocationEnabled(false);
             }
@@ -323,12 +323,10 @@ public SSLSocket createSSLSocket(Socket socket) throws X509Exception, IOExceptio
     }
 
     private void configureSSLSocket(SSLSocket sslSocket) {
-        if (cipherSuites != null) {
-            SSLParameters sslParameters = sslSocket.getSSLParameters();
-            LOG.debug("Setup cipher suites for client socket: {}", Arrays.toString(cipherSuites));
-            sslParameters.setCipherSuites(cipherSuites);
-            sslSocket.setSSLParameters(sslParameters);
-        }
+        SSLParameters sslParameters = sslSocket.getSSLParameters();
+        LOG.debug("Setup cipher suites for client socket: {}", Arrays.toString(cipherSuites));
+        sslParameters.setCipherSuites(cipherSuites);
+        sslSocket.setSSLParameters(sslParameters);
     }
 
     public SSLServerSocket createSSLServerSocket() throws X509Exception, IOException {
@@ -348,11 +346,8 @@ public SSLServerSocket createSSLServerSocket(int port) throws X509Exception, IOE
     private void configureSSLServerSocket(SSLServerSocket sslServerSocket) {
         SSLParameters sslParameters = sslServerSocket.getSSLParameters();
         sslParameters.setNeedClientAuth(true);
-        if (cipherSuites != null) {
-            LOG.debug("Setup cipher suites for server socket: {}", Arrays.toString(cipherSuites));
-            sslParameters.setCipherSuites(cipherSuites);
-        }
-
+        LOG.debug("Setup cipher suites for server socket: {}", Arrays.toString(cipherSuites));
+        sslParameters.setCipherSuites(cipherSuites);
         sslServerSocket.setSSLParameters(sslParameters);
     }
 

src/java/main/org/apache/zookeeper/common/ZKTrustManager.java

@@ -94,19 +94,20 @@ public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngi
             try {
                 performHostVerification(InetAddress.getByName(engine.getPeerHost()), chain[0]);
             } catch (UnknownHostException e) {
-                throw new CertificateException("failed to verify host", e);
+                throw new CertificateException("Failed to verify host", e);
             }
         }
     }
 
     @Override
-    public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) throws CertificateException {
+    public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
+            throws CertificateException {
         x509ExtendedTrustManager.checkServerTrusted(chain, authType, engine);
         if (serverHostnameVerificationEnabled) {
             try {
                 performHostVerification(InetAddress.getByName(engine.getPeerHost()), chain[0]);
             } catch (UnknownHostException e) {
-                throw new CertificateException("failed to verify host", e);
+                throw new CertificateException("Failed to verify host", e);
             }
         }
     }
@@ -129,22 +130,24 @@ public void checkServerTrusted(X509Certificate[] chain, String authType) throws
      * @param certificate Peer's certificate
      * @throws CertificateException Thrown if the provided certificate doesn't match the peer hostname.
      */
-    private void performHostVerification(InetAddress inetAddress, X509Certificate certificate) throws CertificateException {
+    private void performHostVerification(InetAddress inetAddress, X509Certificate certificate)
+            throws CertificateException {
         String hostAddress = "";
         String hostName = "";
         try {
             hostAddress = inetAddress.getHostAddress();
             hostnameVerifier.verify(hostAddress, certificate);
         } catch (SSLException addressVerificationException) {
             try {
-                LOG.debug("Failed to verify host address: " + hostAddress +
-                        ", attempting to verify host name with reverse dns lookup", addressVerificationException);
+                LOG.debug("Failed to verify host address: {} attempting to verify host name with reverse dns lookup",
+                        hostAddress, addressVerificationException);
                 hostName = inetAddress.getHostName();
                 hostnameVerifier.verify(hostName, certificate);
             } catch (SSLException hostnameVerificationException) {
-                LOG.error("Failed to verify host address: " + hostAddress, addressVerificationException);
-                LOG.error("Failed to verify hostname: " + hostName, hostnameVerificationException);
-                throw new CertificateException("Failed to verify both host address and host name", hostnameVerificationException);
+                LOG.error("Failed to verify host address: {}", hostAddress, addressVerificationException);
+                LOG.error("Failed to verify hostname: {}", hostName, hostnameVerificationException);
+                throw new CertificateException("Failed to verify both host address and host name",
+                        hostnameVerificationException);
             }
         }
     }

src/java/main/org/apache/zookeeper/server/quorum/Leader.java

@@ -255,7 +255,7 @@ public boolean isQuorumSynced(QuorumVerifier qv) {
                 ss.bind(self.getQuorumAddress());
             }
         } catch (X509Exception e) {
-            LOG.error("failed to setup ssl server socket", e);
+            LOG.error("Failed to setup ssl server socket", e);
             throw e;
         } catch (BindException e) {
             if (self.getQuorumListenOnAllIPs()) {

src/java/main/org/apache/zookeeper/server/quorum/Learner.java

@@ -247,7 +247,7 @@ protected void sockConnect(Socket sock, InetSocketAddress addr, int timeout)
      * @throws InterruptedException
      */
     protected void connectToLeader(InetSocketAddress addr, String hostname)
-    throws IOException, InterruptedException, X509Exception {
+            throws IOException, InterruptedException, X509Exception {
         this.sock = createSocket();
 
         int initLimitTime = self.tickTime * self.initLimit;

src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java

@@ -547,6 +547,7 @@ private void handleConnection(Socket sock, DataInputStream din)
             closeSocket(sock);
             return;
         }
+
         // do authenticating learner
         authServer.authenticate(sock, din);
         //If wins the challenge, then close the new connection.
@@ -639,49 +640,50 @@ synchronized private boolean connectOne(long sid, InetSocketAddress electionAddr
 
         Socket sock = null;
         try {
-             LOG.debug("Opening channel to server " + sid);
-             if (self.isSslQuorum()) {
-                 SSLSocket sslSock = x509Util.createSSLSocket();
-                 setSockOpts(sslSock);
-                 sslSock.connect(electionAddr, cnxTO);
-                 sslSock.startHandshake();
-                 sock = sslSock;
-             } else {
-                 sock = new Socket();
-                 setSockOpts(sock);
-                 sock.connect(electionAddr, cnxTO);
-
-             }
-             LOG.debug("Connected to server " + sid);
+            LOG.debug("Opening channel to server " + sid);
+            if (self.isSslQuorum()) {
+                SSLSocket sslSock = x509Util.createSSLSocket();
+                setSockOpts(sslSock);
+                sslSock.connect(electionAddr, cnxTO);
+                sslSock.startHandshake();
+                sock = sslSock;
+            } else {
+                sock = new Socket();
+                setSockOpts(sock);
+                sock.connect(electionAddr, cnxTO);
+            }
+            LOG.debug("Connected to server " + sid);
             // Sends connection request asynchronously if the quorum
             // sasl authentication is enabled. This is required because
             // sasl server authentication process may take few seconds to
             // finish, this may delay next peer connection requests.
             if (quorumSaslAuthEnabled) {
                 initiateConnectionAsync(sock, sid);
-            } else { initiateConnection(sock, sid);
-            } return true;
-         } catch (UnresolvedAddressException e) {
-             // Sun doesn't include the address that causes this
-             // exception to be thrown, also UAE cannot be wrapped cleanly
-             // so we log the exception in order to capture this critical
-             // detail.
-             LOG.warn("Cannot open channel to " + sid
-                     + " at election address " + electionAddr, e);
-             closeSocket(sock);
-             throw e;} catch (X509Exception e) {
+            } else {
+                initiateConnection(sock, sid);
+            }
+            return true;
+        } catch (UnresolvedAddressException e) {
+            // Sun doesn't include the address that causes this
+            // exception to be thrown, also UAE cannot be wrapped cleanly
+            // so we log the exception in order to capture this critical
+            // detail.
+            LOG.warn("Cannot open channel to " + sid
+                    + " at election address " + electionAddr, e);
+            closeSocket(sock);
+            throw e;
+        } catch (X509Exception e) {
             LOG.warn("Cannot open secure channel to " + sid
-              + " at election address " + electionAddr, e);
+                    + " at election address " + electionAddr, e);
             closeSocket(sock);
             return false;
-         } catch (IOException e) {
-             LOG.warn("Cannot open channel to " + sid
-                     + " at election address " + electionAddr,
-                     e);
-             closeSocket(sock);
-             return false;
-         }
-
+        } catch (IOException e) {
+            LOG.warn("Cannot open channel to " + sid
+                            + " at election address " + electionAddr,
+                    e);
+            closeSocket(sock);
+            return false;
+        }
     }
 
     /**

src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerConfigTest.java

@@ -27,8 +27,6 @@
 import java.util.Properties;
 
 import org.apache.zookeeper.common.ClientX509Util;
-import org.apache.zookeeper.common.X509Util;
-import org.apache.zookeeper.common.ZKConfig;
 import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.junit.Test;