From f3ba5961bdac254496b91d58abc4b56425f43daa Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Wed, 27 Sep 2023 17:00:53 -0700 Subject: [PATCH] Trigger channel read for TLS handshake only on the server-side Motivation: `SslHandler` automatically initiates the handshake and manages reads on the client-side. We need to initiate read only on the server-side. Modifications: - `DefaultNettyConnection.doChannelActive` read only if `!isClient`; - Assert that channel is active when `AlpnChannelHandler` or `SniCompleteChannelHandler` are added and the read is forced; Result: No unnecessary read on client-side channels. --- .../servicetalk/http/netty/SniCompleteChannelSingle.java | 1 + .../transport/netty/internal/DefaultNettyConnection.java | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/SniCompleteChannelSingle.java b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/SniCompleteChannelSingle.java index 2f46541524..bdb4ec7d70 100644 --- a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/SniCompleteChannelSingle.java +++ b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/SniCompleteChannelSingle.java @@ -51,6 +51,7 @@ private static final class SniCompleteChannelHandler extends ChannelInboundHandl @Override public void handlerAdded(final ChannelHandlerContext ctx) throws Exception { super.handlerAdded(ctx); + assert ctx.channel().isActive(); // Force a read to get the SSL handshake started. We initialize pipeline before // SslHandshakeCompletionEvent will complete, therefore, no data will be propagated before we finish // initialization. diff --git a/servicetalk-transport-netty-internal/src/main/java/io/servicetalk/transport/netty/internal/DefaultNettyConnection.java b/servicetalk-transport-netty-internal/src/main/java/io/servicetalk/transport/netty/internal/DefaultNettyConnection.java index 2099d5ea52..c45f3a1537 100644 --- a/servicetalk-transport-netty-internal/src/main/java/io/servicetalk/transport/netty/internal/DefaultNettyConnection.java +++ b/servicetalk-transport-netty-internal/src/main/java/io/servicetalk/transport/netty/internal/DefaultNettyConnection.java @@ -981,9 +981,11 @@ public void channelInactive(ChannelHandlerContext ctx) { private void doChannelActive(ChannelHandlerContext ctx) { if (waitForSslHandshake) { - // Force a read to get the SSL handshake started, any application data that makes it past the SslHandler - // will be queued in the NettyChannelPublisher. - ctx.read(); + if (!connection.isClient) { + // Force a read to get the SSL handshake started, any application data that makes it past the + // SslHandler will be queued in the NettyChannelPublisher. + ctx.read(); + } } else if (subscriber != null) { completeSubscriber(); }