Skip to content

Commit

Permalink
Trigger channel read for TLS handshake only on the server-side (#2713)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
idelpivnitskiy authored Oct 2, 2023
1 parent 6d38417 commit ee7db1e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down

0 comments on commit ee7db1e

Please sign in to comment.