From 876818a7d262b562559295311c3453c157cec33d Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 11 Jan 2017 14:31:53 +0100 Subject: [PATCH] Ensure SslHandler.sslCloseFuture() is notified in all cases. Motivation: The SslHandler.sslCloseFuture() may not be notified when the Channel is closed before a closify_notify is received. Modifications: Ensure we try to fail the sslCloseFuture() when the Channel is closed. Result: Correctly notify the ssl close future. --- .../main/java/io/netty/handler/ssl/SslHandler.java | 11 +++++++---- .../java/io/netty/handler/ssl/SslHandlerTest.java | 13 +++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index 8eecce6c10..5cae86e3d1 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -277,7 +277,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH private PendingWriteQueue pendingUnencryptedWrites; private Promise handshakePromise = new LazyChannelPromise(); - private final LazyChannelPromise sslCloseFuture = new LazyChannelPromise(); + private final LazyChannelPromise sslClosePromise = new LazyChannelPromise(); /** * Set by wrap*() methods when something is produced. @@ -456,7 +456,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH * @see SSLEngine */ public Future sslCloseFuture() { - return sslCloseFuture; + return sslClosePromise; } @Override @@ -770,6 +770,9 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH // Make sure to release SSLEngine, // and notify the handshake future if the connection has been closed during handshake. setHandshakeFailure(ctx, CHANNEL_CLOSED, !outboundClosed); + + // Ensure we always notify the sslClosePromise as well + sslClosePromise.tryFailure(CHANNEL_CLOSED); super.channelInactive(ctx); } @@ -804,7 +807,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH * */ private boolean ignoreException(Throwable t) { - if (!(t instanceof SSLException) && t instanceof IOException && sslCloseFuture.isDone()) { + if (!(t instanceof SSLException) && t instanceof IOException && sslClosePromise.isDone()) { String message = String.valueOf(t.getMessage()).toLowerCase(); // first try to match connection reset / broke peer based on the regex. This is the fastest way @@ -1129,7 +1132,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH } if (notifyClosure) { - sslCloseFuture.trySuccess(ctx.channel()); + sslClosePromise.trySuccess(ctx.channel()); } } finally { if (decodeOut != null) { diff --git a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java index 7ec85717c2..73fb014a3c 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java @@ -65,6 +65,7 @@ import io.netty.util.ReferenceCounted; import java.io.File; import java.net.InetSocketAddress; +import java.nio.channels.ClosedChannelException; import java.security.KeyStore; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; @@ -374,4 +375,16 @@ public class SslHandlerTest { ReferenceCountUtil.release(sslClientCtx); } } + + @Test + public void testCloseFutureNotified() throws Exception { + SslHandler handler = new SslHandler(SSLContext.getDefault().createSSLEngine()); + EmbeddedChannel ch = new EmbeddedChannel(handler); + + // Closing the Channel will also produce a close_notify so it is expected to return true. + assertTrue(ch.finishAndReleaseAll()); + + assertTrue(handler.handshakeFuture().cause() instanceof ClosedChannelException); + assertTrue(handler.sslCloseFuture().cause() instanceof ClosedChannelException); + } }