From aef16549efdde282a4bdcd6576596521316eb3c3 Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Wed, 10 Aug 2016 18:17:17 -0700
Subject: [PATCH] SniHandler reference count leak if pipeline replace fails

Motivation:
The SniHandler attempts to generate a new SslHandler from the selected SslContext in a and insert that SslHandler into the pipeline. However if the underlying channel has been closed or the pipeline has been modified the pipeline.replace(..) operation may fail. Creating the SslHandler may also create a SSLEngine which is of type ReferenceCounted. The SslHandler states that if it is not inserted into a pipeline that it will not take reference count ownership of the SSLEngine. Under these conditions we will leak the SSLEngine if it is reference counted.

Modifications:
- If the pipeline.replace(..) operation fails we should release the SSLEngine object.

Result:
Fixes https://github.com/netty/netty/issues/5678
---
 .../java/io/netty/handler/ssl/SniHandler.java   | 17 +++++++++++++++--
 .../java/io/netty/handler/ssl/SslContext.java   |  2 +-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
index 1a9cd1ca96..54ebcca1ed 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java
@@ -21,6 +21,7 @@ import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.ByteToMessageDecoder;
 import io.netty.util.CharsetUtil;
 import io.netty.util.DomainNameMapping;
+import io.netty.util.ReferenceCountUtil;
 import io.netty.util.internal.logging.InternalLogger;
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
@@ -28,6 +29,8 @@ import java.net.IDN;
 import java.util.List;
 import java.util.Locale;
 
+import javax.net.ssl.SSLEngine;
+
 /**
  * <p>Enables <a href="https://tools.ietf.org/html/rfc3546#section-3.1">SNI
  * (Server Name Indication)</a> extension for server side SSL. For clients
@@ -245,10 +248,20 @@ public class SniHandler extends ByteToMessageDecoder {
     }
 
     private void select(ChannelHandlerContext ctx, String hostname) {
+        SSLEngine sslEngine = null;
         SslContext selectedContext = mapping.map(hostname);
         selection = new Selection(selectedContext, hostname);
-        SslHandler sslHandler = selectedContext.newHandler(ctx.alloc());
-        ctx.pipeline().replace(this, SslHandler.class.getName(), sslHandler);
+        try {
+            sslEngine = selection.context.newEngine(ctx.alloc());
+            ctx.pipeline().replace(this, SslHandler.class.getName(), selection.context.newHandler(sslEngine));
+        } catch (Throwable cause) {
+            selection = EMPTY_SELECTION;
+            // Since the SslHandler was not inserted into the pipeline the ownership of the SSLEngine was not
+            // transferred to the SslHandler.
+            // See https://github.com/netty/netty/issues/5678
+            ReferenceCountUtil.safeRelease(sslEngine);
+            ctx.fireExceptionCaught(cause);
+        }
     }
 
     private static final class Selection {
diff --git a/handler/src/main/java/io/netty/handler/ssl/SslContext.java b/handler/src/main/java/io/netty/handler/ssl/SslContext.java
index 2aa25bc8c2..8d87867e31 100644
--- a/handler/src/main/java/io/netty/handler/ssl/SslContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/SslContext.java
@@ -866,7 +866,7 @@ public abstract class SslContext {
         return newHandler(newEngine(alloc, peerHost, peerPort));
     }
 
-    private static SslHandler newHandler(SSLEngine engine) {
+    static SslHandler newHandler(SSLEngine engine) {
         return new SslHandler(engine);
     }