JAVA-2229: Separate the sslEnabled and socketFactory properties in MongoClientOptions.

   The socketFactory should only get a default value based on the sslEnabled property if it wasn't already explicitly set.
   Consider it an error if the socket factory doesn't produce an instance of SSLSocket when SSL is set to enabled

Author: jyemin

driver-core/src/main/com/mongodb/connection/SocketStreamHelper.java

@@ -40,7 +40,7 @@ static void initialize(final Socket socket, final ServerAddress address, final S
         if (settings.getSendBufferSize() > 0) {
             socket.setSendBufferSize(settings.getSendBufferSize());
         }
-        if (sslSettings.isEnabled()) {
+        if (sslSettings.isEnabled() || socket instanceof SSLSocket) {
             if (!(socket instanceof SSLSocket)) {
                 throw new MongoInternalException("SSL is enabled but the socket is not an instance of javax.net.ssl.SSLSocket");
             }

driver-core/src/test/functional/com/mongodb/connection/SocketStreamHelperSpecification.groovy

@@ -0,0 +1,104 @@
+/*
+ * Copyright 2016 MongoDB, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ */
+
+package com.mongodb.connection
+
+import com.mongodb.ClusterFixture
+import com.mongodb.MongoInternalException
+import spock.lang.IgnoreIf
+import spock.lang.Specification
+
+import javax.net.SocketFactory
+import javax.net.ssl.SNIHostName
+import javax.net.ssl.SSLSocket
+import javax.net.ssl.SSLSocketFactory
+
+import static com.mongodb.ClusterFixture.getPrimary
+import static java.util.concurrent.TimeUnit.MILLISECONDS
+import static java.util.concurrent.TimeUnit.SECONDS
+
+class SocketStreamHelperSpecification extends Specification {
+
+    def 'should configure socket with settings()'() {
+        given:
+        Socket socket = SocketFactory.default.createSocket()
+        def socketSettings = SocketSettings.builder()
+                .readTimeout(10, SECONDS)
+                .keepAlive(true)
+                .build()
+
+        when:
+        SocketStreamHelper.initialize(socket, getPrimary(), socketSettings, SslSettings.builder().build())
+
+        then:
+        socket.getTcpNoDelay()
+        socket.getKeepAlive()
+        socket.getSoTimeout() == socketSettings.getReadTimeout(MILLISECONDS)
+
+        cleanup:
+        socket?.close()
+    }
+
+    def 'should connect socket()'() {
+        given:
+        Socket socket = SocketFactory.default.createSocket()
+
+        when:
+        SocketStreamHelper.initialize(socket, getPrimary(), SocketSettings.builder().build(), SslSettings.builder().build())
+
+        then:
+        socket.isConnected()
+
+        cleanup:
+        socket?.close()
+    }
+
+    @IgnoreIf({ !ClusterFixture.sslSettings.enabled || System.getProperty('java.version').startsWith('1.6.') })
+    def 'should enable SSL options if socket is an instance of SSLSocket'() {
+        given:
+        SSLSocket socket = SSLSocketFactory.default.createSocket()
+
+        when:
+        SocketStreamHelper.initialize(socket, getPrimary(), SocketSettings.builder().build(), sslSettings)
+
+        then:
+        socket.getSSLParameters().endpointIdentificationAlgorithm == (sslSettings.invalidHostNameAllowed ? null : 'HTTPS')
+        socket.getSSLParameters().getServerNames() == [new SNIHostName(getPrimary().getHost())]
+
+        cleanup:
+        socket?.close()
+
+        where:
+        sslSettings << [SslSettings.builder().enabled(true).build(),
+                        SslSettings.builder().enabled(false).build(),
+                        SslSettings.builder().enabled(true).invalidHostNameAllowed(true).build()]
+    }
+
+    def 'should throw MongoInternalException is ssl is enabled and the socket is not an instance of SSLSocket'() {
+        given:
+        Socket socket = SocketFactory.default.createSocket()
+
+        when:
+        SocketStreamHelper.initialize(socket, getPrimary(), SocketSettings.builder().build(), SslSettings.builder().enabled(true).build())
+
+        then:
+        thrown(MongoInternalException)
+
+        cleanup:
+        socket?.close()
+    }
+}

driver/src/main/com/mongodb/MongoClientOptions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008-2015 MongoDB, Inc.
+ * Copyright (c) 2008-2016 MongoDB, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -51,6 +51,9 @@
 @Immutable
 public class MongoClientOptions {
 
+    private static final SocketFactory DEFAULT_SSL_SOCKET_FACTORY = SSLSocketFactory.getDefault();
+    private static final SocketFactory DEFAULT_SOCKET_FACTORY = SocketFactory.getDefault();
+
     private final String description;
     private final String applicationName;
     private final ReadPreference readPreference;
@@ -572,7 +575,13 @@ public boolean isAlwaysUseMBeans() {
      * @return the socket factory
      */
     public SocketFactory getSocketFactory() {
-        return socketFactory;
+        if (socketFactory != null) {
+            return socketFactory;
+        } else if (getSslSettings().isEnabled()) {
+            return DEFAULT_SSL_SOCKET_FACTORY;
+        } else {
+            return DEFAULT_SOCKET_FACTORY;
+        }
     }
 
     /**
@@ -717,7 +726,7 @@ public boolean equals(final Object o) {
                                            : that.requiredReplicaSetName != null) {
             return false;
         }
-        if (!socketFactory.getClass().equals(that.socketFactory.getClass())) {
+        if (socketFactory != null ? !socketFactory.equals(that.socketFactory) : that.socketFactory != null) {
             return false;
         }
 
@@ -758,7 +767,7 @@ public int hashCode() {
         result = 31 * result + (dbDecoderFactory != null ? dbDecoderFactory.hashCode() : 0);
         result = 31 * result + (dbEncoderFactory != null ? dbEncoderFactory.hashCode() : 0);
         result = 31 * result + (cursorFinalizerEnabled ? 1 : 0);
-        result = 31 * result + socketFactory.getClass().hashCode();
+        result = 31 * result + (socketFactory != null ? socketFactory.hashCode() : 0);
         return result;
     }
 
@@ -846,7 +855,7 @@ public static class Builder {
         private String requiredReplicaSetName;
         private DBDecoderFactory dbDecoderFactory = DefaultDBDecoder.FACTORY;
         private DBEncoderFactory dbEncoderFactory = DefaultDBEncoder.FACTORY;
-        private SocketFactory socketFactory = SocketFactory.getDefault();
+        private SocketFactory socketFactory;
         private boolean cursorFinalizerEnabled = true;
 
         /**
@@ -893,7 +902,7 @@ public Builder(final MongoClientOptions options) {
             requiredReplicaSetName = options.getRequiredReplicaSetName();
             dbDecoderFactory = options.getDbDecoderFactory();
             dbEncoderFactory = options.getDbEncoderFactory();
-            socketFactory = options.getSocketFactory();
+            socketFactory = options.socketFactory;
             cursorFinalizerEnabled = options.isCursorFinalizerEnabled();
             commandListeners.addAll(options.getCommandListeners());
             clusterListeners.addAll(options.getClusterListeners());
@@ -1072,17 +1081,23 @@ public Builder socketKeepAlive(final boolean socketKeepAlive) {
         }
 
         /**
-         * Sets whether to use SSL.  Setting this to true will also set the socketFactory to {@code SSLSocketFactory.getDefault()} and
-         * setting it to false will set the socketFactory to {@code SocketFactory.getDefault()}
+         * Sets whether to use SSL.
+         *
+         * <p>If the socketFactory is unset, setting this to true will also set the socketFactory to
+         * {@link SSLSocketFactory#getDefault()} and setting it to false will set the socketFactory to
+         * {@link SocketFactory#getDefault()}</p>
+         *
+         * <p>If the socket factory is set and sslEnabled is also set, the socket factory must create instances of
+         * {@link javax.net.ssl.SSLSocket}. Otherwise, MongoClient will refuse to connect.</p>
          *
          * @param sslEnabled set to true if using SSL
          * @return {@code this}
          * @see MongoClientOptions#isSslEnabled()
+         * @see MongoClientOptions#getSocketFactory()
          * @since 3.0
          */
         public Builder sslEnabled(final boolean sslEnabled) {
             this.sslEnabled = sslEnabled;
-            this.socketFactory(sslEnabled ? SSLSocketFactory.getDefault() : SocketFactory.getDefault());
             return this;
         }
 
@@ -1216,9 +1231,6 @@ public Builder addServerMonitorListener(final ServerMonitorListener serverMonito
          * @see MongoClientOptions#getSocketFactory()
          */
         public Builder socketFactory(final SocketFactory socketFactory) {
-            if (socketFactory == null) {
-                throw new IllegalArgumentException("null is not a legal value");
-            }
             this.socketFactory = socketFactory;
             return this;
         }

driver/src/test/unit/com/mongodb/MongoClientOptionsSpecification.groovy

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008-2015 MongoDB, Inc.
+ * Copyright (c) 2008-2016 MongoDB, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the 'License');
  * you may not use this file except in compliance with the License.
@@ -142,6 +142,7 @@ class MongoClientOptionsSpecification extends Specification {
     def 'should build with set options'() {
         given:
         def encoderFactory = new MyDBEncoderFactory()
+        def socketFactory = SSLSocketFactory.getDefault()
         def options = MongoClientOptions.builder()
                                         .description('test')
                                         .applicationName('appName')
@@ -157,6 +158,7 @@ class MongoClientOptionsSpecification extends Specification {
                                         .maxConnectionLifeTime(400)
                                         .threadsAllowedToBlockForConnectionMultiplier(2)
                                         .socketKeepAlive(true)
+                                        .socketFactory(socketFactory)
                                         .sslEnabled(true)
                                         .sslInvalidHostNameAllowed(true)
                                         .dbDecoderFactory(LazyDBDecoder.FACTORY)
@@ -185,6 +187,7 @@ class MongoClientOptionsSpecification extends Specification {
         options.getSocketTimeout() == 700
         options.getThreadsAllowedToBlockForConnectionMultiplier() == 2
         options.isSocketKeepAlive()
+        options.socketFactory == socketFactory
         options.isSslEnabled()
         options.isSslInvalidHostNameAllowed()
         options.getDbDecoderFactory() == LazyDBDecoder.FACTORY
@@ -212,23 +215,25 @@ class MongoClientOptionsSpecification extends Specification {
     }
 
     @IgnoreIf({ System.getProperty('java.version').startsWith('1.6.') })
-    def 'should set sslEnabled based on socketFactory'() {
-        given:
+    def 'should get socketFactory based on sslEnabled'() {
+        when:
         MongoClientOptions.Builder builder = MongoClientOptions.builder()
-        SocketFactory socketFactory = SSLSocketFactory.getDefault()
 
-        when:
-        builder.socketFactory(socketFactory)
         then:
-        builder.build().getSocketFactory() == socketFactory
-        builder.sslEnabled(false)
-        builder.build().getSocketFactory() != null
-        !(builder.build().getSocketFactory() instanceof SSLSocketFactory)
+        builder.build().getSocketFactory() == MongoClientOptions.DEFAULT_SOCKET_FACTORY
 
         when:
         builder.sslEnabled(true)
+
         then:
-        builder.build().getSocketFactory() instanceof SSLSocketFactory
+        builder.build().getSocketFactory() == MongoClientOptions.DEFAULT_SSL_SOCKET_FACTORY
+
+        when:
+        def socketFactory = Mock(SocketFactory)
+        builder.socketFactory(socketFactory)
+
+        then:
+        builder.build().getSocketFactory() == socketFactory
     }
 
     def 'should be easy to create new options from existing'() {
@@ -493,48 +498,44 @@ class MongoClientOptionsSpecification extends Specification {
 
     def 'builder should copy all values from the existing MongoClientOptions'() {
         given:
-        def options = Mock(MongoClientOptions)
-
-
-        when:
-        MongoClientOptions.builder(options)
-
-        then:
-        1 * options.isAlwaysUseMBeans()
-        1 * options.getCodecRegistry()
-        1 * options.getConnectionsPerHost()
-        1 * options.getConnectTimeout()
-        1 * options.isCursorFinalizerEnabled()
-        1 * options.getDbDecoderFactory()
-        1 * options.getDbEncoderFactory()
-        1 * options.getDescription()
-        1 * options.getApplicationName()
-        1 * options.getHeartbeatConnectTimeout()
-        1 * options.getHeartbeatFrequency()
-        1 * options.getHeartbeatSocketTimeout()
-        1 * options.getLocalThreshold()
-        1 * options.getMaxConnectionIdleTime()
-        1 * options.getMaxConnectionLifeTime()
-        1 * options.getMaxWaitTime()
-        1 * options.getMinConnectionsPerHost()
-        1 * options.getMinHeartbeatFrequency()
-        1 * options.getReadPreference()
-        1 * options.getRequiredReplicaSetName()
-        1 * options.getServerSelectionTimeout()
-        1 * options.getSocketFactory()
-        1 * options.isSocketKeepAlive()
-        1 * options.getSocketTimeout()
-        1 * options.isSslEnabled()
-        1 * options.isSslInvalidHostNameAllowed()
-        1 * options.getThreadsAllowedToBlockForConnectionMultiplier()
-        1 * options.getWriteConcern()
-        1 * options.getReadConcern()
-        1 * options.getCommandListeners() >> Collections.unmodifiableList(Collections.emptyList())
-        1 * options.getClusterListeners() >> Collections.unmodifiableList(Collections.emptyList())
-        1 * options.getServerListeners() >> Collections.unmodifiableList(Collections.emptyList())
-        1 * options.getServerMonitorListeners() >> Collections.unmodifiableList(Collections.emptyList())
-
-        0 * options._ // Ensure no other interactions
+        def options = MongoClientOptions.builder()
+                .description('test')
+                .applicationName('appName')
+                .readPreference(ReadPreference.secondary())
+                .writeConcern(WriteConcern.JOURNALED)
+                .minConnectionsPerHost(30)
+                .connectionsPerHost(500)
+                .connectTimeout(100)
+                .socketTimeout(700)
+                .serverSelectionTimeout(150)
+                .maxWaitTime(200)
+                .maxConnectionIdleTime(300)
+                .maxConnectionLifeTime(400)
+                .threadsAllowedToBlockForConnectionMultiplier(2)
+                .socketKeepAlive(true)
+                .sslEnabled(true)
+                .sslInvalidHostNameAllowed(true)
+                .socketFactory(SSLSocketFactory.getDefault())
+                .dbDecoderFactory(LazyDBDecoder.FACTORY)
+                .heartbeatFrequency(5)
+                .minHeartbeatFrequency(11)
+                .heartbeatConnectTimeout(15)
+                .heartbeatSocketTimeout(20)
+                .localThreshold(25)
+                .requiredReplicaSetName('test')
+                .cursorFinalizerEnabled(false)
+                .dbEncoderFactory(new MyDBEncoderFactory())
+                .addCommandListener(Mock(CommandListener))
+                .addClusterListener(Mock(ClusterListener))
+                .addServerListener(Mock(ServerListener))
+                .addServerMonitorListener(Mock(ServerMonitorListener))
+                .build()
+
+        when:
+        def copy = MongoClientOptions.builder(options).build()
+
+        then:
+        copy == options
     }
 
     def 'should only have the following fields in the builder'() {