Fix IndexOutOfBoundsException from SslHandler on JDK 8

Motivation:

When SslHandler.unwrap() copies SSL records into a heap buffer, it does
not update the start offset, causing IndexOutOfBoundsException.

Modifications:

- Copy to a heap buffer before calling unwrap() for simplicity
- Do not copy an empty buffer to a heap buffer.
  - unwrap(... EMPTY_BUFFER ...) never involves copying now.
- Use better parameter names for unwrap()
- Clean-up log messages

Result:

- Bugs fixed
- Cleaner code

handler/src/main/java/io/netty/handler/ssl/SslHandler.java

@@ -376,7 +376,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                     flush(ctx);
                 } catch (Exception e) {
                     if (!future.tryFailure(e)) {
-                        logger.warn("flush() raised a masked exception.", e);
+                        logger.warn("{} flush() raised a masked exception.", ctx.channel(), e);
                     }
                 }
             }
@@ -524,7 +524,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                 }
             }
         } catch (SSLException e) {
-            setHandshakeFailure(e);
+            setHandshakeFailure(ctx, e);
             throw e;
         } finally {
             finishWrap(ctx, out, promise, inUnwrap);
@@ -599,7 +599,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                 }
             }
         } catch (SSLException e) {
-            setHandshakeFailure(e);
+            setHandshakeFailure(ctx, e);
             throw e;
         }  finally {
             if (out != null) {
@@ -669,7 +669,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
     public void channelInactive(ChannelHandlerContext ctx) throws Exception {
         // Make sure to release SSLEngine,
         // and notify the handshake future if the connection has been closed during handshake.
-        setHandshakeFailure(CHANNEL_CLOSED);
+        setHandshakeFailure(ctx, CHANNEL_CLOSED);
         super.channelInactive(ctx);
     }
 
@@ -680,8 +680,8 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
             // 'broken pipe' error after sending close_notify.
             if (logger.isDebugEnabled()) {
                 logger.debug(
-                        "Swallowing a harmless 'connection reset by peer / broken pipe' error that occurred " +
+                        "{} Swallowing a harmless 'connection reset by peer / broken pipe' error that occurred " +
-                                "while writing close_notify in response to the peer's close_notify", cause);
+                        "while writing close_notify in response to the peer's close_notify", ctx.channel(), cause);
             }
 
             // Close the connection explicitly just in case the transport
@@ -916,7 +916,19 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
             // See https://github.com/netty/netty/issues/1534
 
             in.skipBytes(totalLength);
-            unwrap(ctx, in, startOffset, totalLength);
+
+            // If SSLEngine expects a heap buffer for unwrapping, do the conversion.
+            if (in.isDirect() && wantsInboundHeapBuffer) {
+                ByteBuf copy = ctx.alloc().heapBuffer(totalLength);
+                try {
+                    copy.writeBytes(in, startOffset, totalLength);
+                    unwrap(ctx, copy, 0, totalLength);
+                } finally {
+                    copy.release();
+                }
+            } else {
+                unwrap(ctx, in, startOffset, totalLength);
+            }
         }
 
         if (nonSslRecord) {
@@ -925,7 +937,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                     "not an SSL/TLS record: " + ByteBufUtil.hexDump(in));
             in.skipBytes(in.readableBytes());
             ctx.fireExceptionCaught(e);
-            setHandshakeFailure(e);
+            setHandshakeFailure(ctx, e);
         }
     }
 
@@ -954,37 +966,23 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
     /**
      * Unwraps inbound SSL records.
      */
-    private void unwrap(ChannelHandlerContext ctx, ByteBuf packet,
+    private void unwrap(
-            int readerIndex, int initialOutAppBufCapacity) throws SSLException {
+            ChannelHandlerContext ctx, ByteBuf packet, int offset, int length) throws SSLException {
-
-        int len = initialOutAppBufCapacity;
-        // If SSLEngine expects a heap buffer for unwrapping, do the conversion.
-        final ByteBuf oldPacket;
-        final ByteBuf newPacket;
-        if (wantsInboundHeapBuffer && packet.isDirect()) {
-            newPacket = ctx.alloc().heapBuffer(packet.readableBytes());
-            newPacket.writeBytes(packet, readerIndex, len);
-            oldPacket = packet;
-            packet = newPacket;
-        } else {
-            oldPacket = null;
-            newPacket = null;
-        }
 
         boolean wrapLater = false;
         boolean notifyClosure = false;
-        ByteBuf decodeOut = allocate(ctx, initialOutAppBufCapacity);
+        ByteBuf decodeOut = allocate(ctx, length);
         try {
             for (;;) {
-                final SSLEngineResult result = unwrap(engine, packet, readerIndex, len, decodeOut);
+                final SSLEngineResult result = unwrap(engine, packet, offset, length, decodeOut);
                 final Status status = result.getStatus();
                 final HandshakeStatus handshakeStatus = result.getHandshakeStatus();
                 final int produced = result.bytesProduced();
                 final int consumed = result.bytesConsumed();
 
                 // Update indexes for the next iteration
-                readerIndex += consumed;
+                offset += consumed;
-                len -= consumed;
+                length -= consumed;
 
                 if (status == Status.CLOSED) {
                     // notify about the CLOSED state of the SSLEngine. See #137
@@ -1019,7 +1017,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
 
                         break;
                     default:
-                        throw new IllegalStateException("Unknown handshake status: " + handshakeStatus);
+                        throw new IllegalStateException("unknown handshake status: " + handshakeStatus);
                 }
 
                 if (status == Status.BUFFER_UNDERFLOW || consumed == 0 && produced == 0) {
@@ -1035,16 +1033,9 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                 sslCloseFuture.trySuccess(ctx.channel());
             }
         } catch (SSLException e) {
-            setHandshakeFailure(e);
+            setHandshakeFailure(ctx, e);
             throw e;
         } finally {
-            // If we converted packet into a heap buffer at the beginning of this method,
-            // we should synchronize the position of the original buffer.
-            if (newPacket != null) {
-                oldPacket.readerIndex(readerIndex + packet.readerIndex());
-                newPacket.release();
-            }
-
             if (decodeOut.isReadable()) {
                 ctx.fireChannelRead(decodeOut);
             } else {
@@ -1227,7 +1218,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
         handshakePromise.trySuccess(ctx.channel());
 
         if (logger.isDebugEnabled()) {
-            logger.debug(ctx.channel() + " HANDSHAKEN: " + engine.getSession().getCipherSuite());
+            logger.debug("{} HANDSHAKEN: {}", ctx.channel(), engine.getSession().getCipherSuite());
         }
         ctx.fireUserEventTriggered(SslHandshakeCompletionEvent.SUCCESS);
 
@@ -1240,7 +1231,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
     /**
      * Notify all the handshake futures about the failure during the handshake.
      */
-    private void setHandshakeFailure(Throwable cause) {
+    private void setHandshakeFailure(ChannelHandlerContext ctx, Throwable cause) {
         // Release all resources such as internal buffers that SSLEngine
         // is managing.
         engine.closeOutbound();
@@ -1254,7 +1245,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
             // See https://github.com/netty/netty/issues/1340
             String msg = e.getMessage();
             if (msg == null || !msg.contains("possible truncation attack")) {
-                logger.debug("SSLEngine.closeInbound() raised an exception.", e);
+                logger.debug("{} SSLEngine.closeInbound() raised an exception.", ctx.channel(), e);
             }
         }
         notifyHandshakeFailure(cause);
@@ -1438,9 +1429,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
             timeoutFuture = ctx.executor().schedule(new Runnable() {
                 @Override
                 public void run() {
-                    logger.warn(
+                    logger.warn("{} Last write attempt timed out; force-closing the connection.", ctx.channel());
-                            ctx.channel() + " last write attempt timed out." +
-                                    " Force-closing the connection.");
                     ctx.close(promise);
                 }
             }, closeNotifyTimeoutMillis, TimeUnit.MILLISECONDS);