DefaultNettyConnection.initChannel() does not work on channels that a…

…lready completed a SSL handshake.
apple · Jul 28, 2023 · e6ad6aa · e6ad6aa
1 parent 67308bd
commit e6ad6aa
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 3 deletions.
diff --git a/...k-http-netty/src/main/java/io/servicetalk/http/netty/H2ClientParentConnectionContext.java b/...k-http-netty/src/main/java/io/servicetalk/http/netty/H2ClientParentConnectionContext.java
@@ -142,7 +142,7 @@ protected void handleSubscribe(final Subscriber<? super H2ClientParentConnection
                     initializer.init(channel);
                     pipeline = channel.pipeline();
                     parentChannelInitializer = new DefaultH2ClientParentConnection(connection, subscriber,
-                            delayedCancellable, NettyPipelineSslUtils.isSslEnabled(pipeline),
+                            delayedCancellable, NettyPipelineSslUtils.hasPendingSslHandshake(pipeline),
                             allowDropTrailersReadFromTransport, config.headersFactory(), reqRespFactory, observer);
                 } catch (Throwable cause) {
                     close(channel, cause);

diff --git a/...k-http-netty/src/main/java/io/servicetalk/http/netty/H2ServerParentConnectionContext.java b/...k-http-netty/src/main/java/io/servicetalk/http/netty/H2ServerParentConnectionContext.java
@@ -147,7 +147,7 @@ protected void handleSubscribe(final Subscriber<? super H2ServerParentConnection
                     pipeline = channel.pipeline();
 
                     parentChannelInitializer = new DefaultH2ServerParentConnection(connection, subscriber,
-                            delayedCancellable, NettyPipelineSslUtils.isSslEnabled(pipeline), observer);
+                            delayedCancellable, NettyPipelineSslUtils.hasPendingSslHandshake(pipeline), observer);
 
                     new H2ServerParentChannelInitializer(h2ServerConfig,
                         new io.netty.channel.ChannelInitializer<Http2StreamChannel>() {

diff --git a/...nternal/src/main/java/io/servicetalk/transport/netty/internal/DefaultNettyConnection.java b/...nternal/src/main/java/io/servicetalk/transport/netty/internal/DefaultNettyConnection.java
@@ -506,7 +506,7 @@ protected void handleSubscribe(
                     initializer.init(channel);
                     ChannelPipeline pipeline = connection.channel().pipeline();
                     nettyInboundHandler = new NettyToStChannelHandler<>(connection, subscriber,
-                            delayedCancellable, NettyPipelineSslUtils.isSslEnabled(pipeline), observer);
+                            delayedCancellable, NettyPipelineSslUtils.hasPendingSslHandshake(pipeline), observer);
                 } catch (Throwable cause) {
                     close(channel, cause);
                     deliverErrorFromSource(subscriber, cause);

diff --git a/...internal/src/main/java/io/servicetalk/transport/netty/internal/NettyPipelineSslUtils.java b/...internal/src/main/java/io/servicetalk/transport/netty/internal/NettyPipelineSslUtils.java
@@ -18,10 +18,12 @@
 import io.servicetalk.transport.api.ConnectionObserver.SecurityHandshakeObserver;
 import io.servicetalk.transport.netty.internal.ConnectionObserverInitializer.ConnectionObserverHandler;
 
+import io.netty.channel.Channel;
 import io.netty.channel.ChannelPipeline;
 import io.netty.handler.ssl.SniHandler;
 import io.netty.handler.ssl.SslHandler;
 import io.netty.handler.ssl.SslHandshakeCompletionEvent;
+import io.netty.util.concurrent.Future;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -45,11 +47,36 @@ private NettyPipelineSslUtils() {
      *
      * @param pipeline The pipeline to check.
      * @return {@code true} if the pipeline is configured to use SSL/TLS.
+     *
+     * @deprecated Use {@link #hasPendingSslHandshake(ChannelPipeline)} instead
      */
+    @Deprecated
     public static boolean isSslEnabled(ChannelPipeline pipeline) {
         return pipeline.get(SslHandler.class) != null || pipeline.get(SniHandler.class) != null;
     }
 
+    /**
+     * Determine if the {@link ChannelPipeline} has a pending SSL/TLS handshake.
+     *
+     * @param pipeline The pipeline to check.
+     * @return {@code true} if the pipeline is configured to use SSL/TLS, but the handshake has not completed yet.
+     * @throws Throwable Handshake exception, if the handshake failed.
+     */
+    public static boolean hasPendingSslHandshake(ChannelPipeline pipeline) throws Throwable {
+        final SslHandler sslHandler = pipeline.get(SslHandler.class);
+        if (sslHandler != null) {
+            final Future<Channel> handshakeFuture = sslHandler.handshakeFuture();
+            if (handshakeFuture.isDone()) {
+                if (handshakeFuture.cause() != null) {
+                    throw handshakeFuture.cause();
+                }
+                return false;
+            }
+            return true;
+        }
+        return pipeline.get(SniHandler.class) != null;
+    }
+
     /**
      * Extracts the {@link SSLSession} from the {@link ChannelPipeline} if the {@link SslHandshakeCompletionEvent}
      * is successful and reports the result to {@link SecurityHandshakeObserver} if available.