SCBC-382 Throw exception instead of ignoring connection string scheme…

… or parameters Motivation ---------- The connection string scheme and parameters can specify important security settings that should not be ignored. Modifications ------------- If a pre-built cluster environment is incompatible with the connection string, fail with IllegalArgumentException (instead of silently ignoring bits of the connection string). Change-Id: I73b742b83b8703fe281cc52ca008998f6cffca23 Reviewed-on: https://review.couchbase.org/c/couchbase-jvm-clients/+/183574 Tested-by: Build Bot <build@couchbase.com> Reviewed-by: Graham Pople <graham.pople@couchbase.com>
couchbase · Dec 2, 2022 · 15793f0 · 15793f0
1 parent 5fe9e18
commit 15793f0
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 14 deletions.
diff --git a/scala-client/src/integrationTest/scala/com/couchbase/client/scala/ConnectionSpec.scala b/scala-client/src/integrationTest/scala/com/couchbase/client/scala/ConnectionSpec.scala
@@ -16,24 +16,57 @@ package com.couchbase.client.scala
  * limitations under the License.
  */
 
+import com.couchbase.client.core.util.ConnectionStringUtil
 import com.couchbase.client.scala.env.{ClusterEnvironment, SecurityConfig}
 import com.couchbase.client.scala.json.JsonObject
 import com.couchbase.client.scala.kv.UpsertOptions
 import com.couchbase.client.scala.util.ScalaIntegrationTest
 import com.couchbase.client.test.{Capabilities, ClusterType, IgnoreWhen}
 import io.netty.handler.ssl.util.InsecureTrustManagerFactory
 import org.junit.Assert.fail
+import org.junit.jupiter.api.Assertions.{assertEquals, assertThrows}
 import org.junit.jupiter.api.TestInstance.Lifecycle
-import org.junit.jupiter.api.{Disabled, Test, TestInstance}
-import scala.collection.JavaConverters;
+import org.junit.jupiter.api.{Test, TestInstance}
 
-import java.nio.file.Path
 import java.util.concurrent.TimeUnit
+import scala.collection.JavaConverters
 import scala.concurrent.duration.Duration
 
 @TestInstance(Lifecycle.PER_CLASS)
 class ConnectionSpec extends ScalaIntegrationTest {
 
+  @Test
+  def failsOnIncompatibleConnectionStringScheme(): Unit = {
+    assertIncompatibleConnectionString(
+      "couchbases://example.com",
+      ConnectionStringUtil.INCOMPATIBLE_CONNECTION_STRING_SCHEME
+    )
+  }
+
+  @Test
+  def failsOnIncompatibleConnectionStringParams(): Unit = {
+    assertIncompatibleConnectionString(
+      "couchbase://example.com?foo=bar",
+      ConnectionStringUtil.INCOMPATIBLE_CONNECTION_STRING_PARAMS
+    )
+  }
+
+  def assertIncompatibleConnectionString(
+      connectionString: String,
+      expectedErrorMessage: String
+  ): Unit = {
+    val env = ClusterEnvironment.builder.build.get
+    try {
+      val e = assertThrows(
+        classOf[IllegalArgumentException],
+        () => Cluster.connect(connectionString, ClusterOptions(authenticator, Some(env))).get
+      )
+      assertEquals(expectedErrorMessage, e.getMessage)
+    } finally {
+      env.shutdown()
+    }
+  }
+
   @Test
   def performsKeyValueIgnoringServerCert(): Unit = {
     val env = ClusterEnvironment.builder

diff --git a/scala-client/src/main/scala/com/couchbase/client/scala/AsyncCluster.scala b/scala-client/src/main/scala/com/couchbase/client/scala/AsyncCluster.scala
@@ -21,18 +21,12 @@ import com.couchbase.client.core.diagnostics._
 import com.couchbase.client.core.env.Authenticator
 import com.couchbase.client.core.msg.search.SearchRequest
 import com.couchbase.client.core.service.ServiceType
-import com.couchbase.client.core.util.ConnectionStringUtil
+import com.couchbase.client.core.util.ConnectionStringUtil.checkConnectionString
+import com.couchbase.client.core.util.{ConnectionString, ConnectionStringUtil}
 import com.couchbase.client.scala.analytics._
-import com.couchbase.client.scala.diagnostics.{
-  DiagnosticsOptions,
-  PingOptions,
-  WaitUntilReadyOptions
-}
+import com.couchbase.client.scala.diagnostics.{DiagnosticsOptions, PingOptions, WaitUntilReadyOptions}
 import com.couchbase.client.scala.env.{ClusterEnvironment, PasswordAuthenticator, SeedNode}
-import com.couchbase.client.scala.manager.analytics.{
-  AsyncAnalyticsIndexManager,
-  ReactiveAnalyticsIndexManager
-}
+import com.couchbase.client.scala.manager.analytics.{AsyncAnalyticsIndexManager, ReactiveAnalyticsIndexManager}
 import com.couchbase.client.scala.manager.bucket.{AsyncBucketManager, ReactiveBucketManager}
 import com.couchbase.client.scala.manager.eventing.AsyncEventingFunctionManager
 import com.couchbase.client.scala.manager.query.AsyncQueryIndexManager
@@ -507,10 +501,16 @@ object AsyncCluster {
       connectionString: String,
       opts: ClusterOptions
   ): Try[ClusterEnvironment] = {
-    opts.environment match {
+    val result = opts.environment match {
       case Some(env) => Success(env)
       case _         => ClusterEnvironment.Builder(owned = true).connectionString(connectionString).build
     }
+
+    if (result.isFailure) result else Try {
+      val env = result.get
+      checkConnectionString(env.core, env.owned, ConnectionString.create(connectionString))
+      env
+    }
   }
 
   private[client] def extractClusterEnvironment(opts: ClusterOptions): Try[ClusterEnvironment] = {