From ae4e9ddc2d66f23c3cad6f8adeed72392aae67da Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Mon, 18 Jan 2016 13:00:25 +0100 Subject: [PATCH] Ensure we flush out all pending data on SslException. Related to [#3900] Motivation: We need to ensure we flush out all pending data when an SslException accours so the remote peer receives all alerts. Modifications: Ensure we call ctx.flush() when needed. Result: Correctly receive alerts in all cases on the remote peer. --- .../java/io/netty/handler/ssl/SslHandler.java | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 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 7dde9d0445..b581749ae2 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -503,8 +503,13 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH if (!handshakePromise.isDone()) { flushedBeforeHandshake = true; } - wrap(ctx, false); - ctx.flush(); + try { + wrap(ctx, false); + } finally { + // We may have written some parts of data before an exception was thrown so ensure we always flush. + // See https://github.com/netty/netty/issues/3900#issuecomment-172481830 + ctx.flush(); + } } private void wrap(ChannelHandlerContext ctx, boolean inUnwrap) throws SSLException { @@ -642,6 +647,10 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH } } catch (SSLException e) { setHandshakeFailure(ctx, e); + + // We may have written some parts of data before an exception was thrown so ensure we always flush. + // See https://github.com/netty/netty/issues/3900#issuecomment-172481830 + flushIfNeeded(ctx); throw e; } finally { if (out != null) { @@ -925,10 +934,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH // Discard bytes of the cumulation buffer if needed. discardSomeReadBytes(); - if (needsFlush) { - needsFlush = false; - ctx.flush(); - } + flushIfNeeded(ctx); // If handshake is not finished yet, we need more data. if (!ctx.channel().config().isAutoRead() && (!firedChannelRead || !handshakePromise.isDone())) { @@ -941,6 +947,13 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH ctx.fireChannelReadComplete(); } + private void flushIfNeeded(ChannelHandlerContext ctx) { + if (needsFlush) { + needsFlush = false; + ctx.flush(); + } + } + /** * Calls {@link SSLEngine#unwrap(ByteBuffer, ByteBuffer)} with an empty buffer to handle handshakes, etc. */ @@ -1349,9 +1362,12 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH try { engine.beginHandshake(); wrapNonAppData(ctx, false); - ctx.flush(); } catch (Exception e) { notifyHandshakeFailure(e); + } finally { + // We have may haven written some parts of data before an exception was thrown so ensure we always flush. + // See https://github.com/netty/netty/issues/3900#issuecomment-172481830 + ctx.flush(); } // Set timeout if necessary.