(tinkerpop) 09/48: Fixed failing GremlinServerSslIntegrateTest.shouldEnableSslAndFailIfCiphersDontMatch by reinstating the previous WebSocket channelizer logic that waited for the handshake to complete after the channel connects. If the handshake fails then a ConnectionException is thrown. (#2753)

[ke...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

kenhuuu pushed a commit to branch master-http-final
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit b972aa8140449541b5c14533ac83658e11a2c064
Author: andreachild <an...@improving.com>
AuthorDate: Thu Sep 12 12:41:29 2024 -0700

    Fixed failing GremlinServerSslIntegrateTest.shouldEnableSslAndFailIfCiphersDontMatch by reinstating the previous WebSocket channelizer logic that waited for the handshake to complete after the channel connects. If the handshake fails then a ConnectionException is thrown. (#2753)
---
 .../apache/tinkerpop/gremlin/driver/Channelizer.java | 20 +++++++++++++++++++-
 .../server/GremlinServerSslIntegrateTest.java        |  3 ---
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java
index 8b80cc3457..43c1769c2d 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Channelizer.java
@@ -27,6 +27,7 @@ import io.netty.handler.codec.http.HttpClientCodec;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslHandler;
+import org.apache.tinkerpop.gremlin.driver.exception.ConnectionException;
 import org.apache.tinkerpop.gremlin.driver.handler.GremlinResponseHandler;
 import org.apache.tinkerpop.gremlin.driver.handler.HttpContentDecompressionHandler;
 import org.apache.tinkerpop.gremlin.driver.handler.HttpGremlinRequestEncoder;
@@ -87,6 +88,7 @@ public interface Channelizer extends ChannelHandler {
     abstract class AbstractChannelizer extends ChannelInitializer<SocketChannel> implements Channelizer {
         protected Connection connection;
         protected Cluster cluster;
+        protected SslHandler sslHandler;
         private AtomicReference<ResultQueue> pending;
 
         protected static final String PIPELINE_GREMLIN_HANDLER = "gremlin-handler";
@@ -96,6 +98,10 @@ public interface Channelizer extends ChannelHandler {
         protected static final String PIPELINE_HTTP_ENCODER = "gremlin-encoder";
         protected static final String PIPELINE_HTTP_DECODER = "gremlin-decoder";
         protected static final String PIPELINE_HTTP_DECOMPRESSION_HANDLER = "http-decompression-handler";
+        
+        private static final String HANDSHAKE_ERROR = "Could not complete connection setup to the server. Ensure that SSL is correctly " +
+                "configured at both the client and the server. Ensure that client http handshake " +
+                "protocol matches the server. Ensure that the server is still reachable.";
 
         private static final SslCheckHandler sslCheckHandler = new SslCheckHandler();
 
@@ -136,7 +142,7 @@ public interface Channelizer extends ChannelHandler {
             }
 
             if (sslCtx.isPresent()) {
-                final SslHandler sslHandler = sslCtx.get().newHandler(socketChannel.alloc(), connection.getUri().getHost(), connection.getUri().getPort());
+                sslHandler = sslCtx.get().newHandler(socketChannel.alloc(), connection.getUri().getHost(), connection.getUri().getPort());
                 // TINKERPOP-2814. Remove the SSL handshake timeout so that handshakes that take longer than 10000ms
                 // (Netty default) but less than connectionSetupTimeoutMillis can succeed. This means the SSL handshake
                 // will instead be capped by connectionSetupTimeoutMillis.
@@ -149,6 +155,18 @@ public interface Channelizer extends ChannelHandler {
             configure(pipeline);
             pipeline.addLast(PIPELINE_GREMLIN_HANDLER, new GremlinResponseHandler(pending));
         }
+
+        @Override
+        public void connected() {
+            if (supportsSsl()) {
+                try {
+                    // Block until the handshake is complete either successfully or with an error.
+                    sslHandler.handshakeFuture().sync();
+                } catch (Exception ex) {
+                    throw new ConnectionException(connection.getUri(), HANDSHAKE_ERROR, ex);
+                }
+            }
+        }
     }
 
     /**
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java
index cf2444cb02..8d97dd93fa 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSslIntegrateTest.java
@@ -30,7 +30,6 @@ import org.apache.tinkerpop.gremlin.driver.Cluster;
 import org.apache.tinkerpop.gremlin.driver.simple.SimpleClient;
 import org.apache.tinkerpop.gremlin.util.ExceptionHelper;
 import org.apache.tinkerpop.gremlin.util.message.RequestMessage;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import javax.net.ssl.SSLException;
@@ -302,8 +301,6 @@ public class GremlinServerSslIntegrateTest extends AbstractGremlinServerIntegrat
         }
     }
 
-    // TODO: Add client-side SSL checking.
-    @Ignore("No client side SSL checking")
     @Test
     public void shouldEnableSslAndFailIfCiphersDontMatch() {
         final Cluster cluster = TestClientFactory.build().enableSsl(true).keyStore(JKS_SERVER_KEY).keyStorePassword(KEY_PASS)