zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ivmaykov <...@git.apache.org>
Subject [GitHub] zookeeper pull request #184: ZOOKEEPER-236: SSL Support for Atomic Broadcast...
Date Thu, 14 Jun 2018 18:03:10 GMT
Github user ivmaykov commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/184#discussion_r195514675
  
    --- Diff: src/java/main/org/apache/zookeeper/common/X509Util.java ---
    @@ -339,4 +351,20 @@ private void configureSSLServerSocket(SSLServerSocket sslServerSocket)
{
     
             sslServerSocket.setSSLParameters(sslParameters);
         }
    +
    +    private String[] getDefaultCipherSuites() {
    +        String javaVersion = System.getProperty("java.version");
    --- End diff --
    
    Couple minor suggestions:
    - use the "java.specification.version" property instead, it returns a string w/o the minor
version (such as "1.8" or "9"), easier to deal with.
    - Add some comments that explain why we branch the ciphers based on the java version.
Something like "perf testing done by Facebook engineers shows that on Intel x86_64 machines,
Java9 performs better with GCM and Java8 performs better with CBC, so these seem like reasonable
defaults."


---

Mime
View raw message