Improve the API design of Http2OrHttpChooser and SpdyOrHttpChooser

Related: #3641 and #3813

Motivation:

When setting up an HTTP/1 or HTTP/2 (or SPDY) pipeline, a user usually
ends up with adding arbitrary set of handlers.

Http2OrHttpChooser and SpdyOrHttpChooser have two abstract methods
(create*Handler()) that expect a user to return a single handler, and
also have add*Handlers() methods that add the handler returned by
create*Handler() to the pipeline as well as the pre-defined set of
handlers.

The problem is, some users (read: I) don't need all of them or the
user wants to add more than one handler. For example, take a look at
io.netty.example.http2.tiles.Http2OrHttpHandler, which works around
this issue by overriding addHttp2Handlers() and making
createHttp2RequestHandler() a no-op.

Modifications:

- Replace add*Handlers() and create*Handler() with configure*()
- Rename getProtocol() to selectProtocol() to make what it does clear
- Provide the default implementation of selectProtocol()
- Remove SelectedProtocol.UNKNOWN and use null instead, because
  'UNKNOWN' is not a protocol
- Proper exception handling in the *OrHttpChooser so that the
  exception is logged and the connection is closed when failed to
  select a protocol
- Make SpdyClient example always use SSL. It was always using SSL
  anyway.
- Implement SslHandshakeCompletionEvent.toString() for debuggability
- Remove an orphaned class: JettyNpnSslSession
- Add SslHandler.applicationProtocol() to get the name of the
  application protocol
  - SSLSession.getProtocol() now returns transport-layer protocol name
    only, so that it conforms to its contract.

Result:

- *OrHttpChooser have better API.
- *OrHttpChooser handle protocol selection failure properly.
- SSLSession.getProtocol() now conforms to its contract.
- SpdyClient example works with SpdyServer example out of the box

codec-http/src/main/java/io/netty/handler/codec/spdy/SpdyOrHttpChooser.java

@@ -16,18 +16,16 @@
 package io.netty.handler.codec.spdy;
 
 import io.netty.buffer.ByteBuf;
-import io.netty.channel.ChannelHandler;
+import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandler;
 import io.netty.channel.ChannelPipeline;
 import io.netty.handler.codec.ByteToMessageDecoder;
-import io.netty.handler.codec.http.HttpObjectAggregator;
+import io.netty.handler.codec.http.HttpServerCodec;
-import io.netty.handler.codec.http.HttpRequestDecoder;
-import io.netty.handler.codec.http.HttpResponseEncoder;
 import io.netty.handler.ssl.SslHandler;
-import io.netty.util.internal.StringUtil;
+import io.netty.util.internal.logging.InternalLogger;
+import io.netty.util.internal.logging.InternalLoggerFactory;
 
-import javax.net.ssl.SSLEngine;
 import java.util.List;
 
 /**
@@ -37,13 +35,14 @@ import java.util.List;
  */
 public abstract class SpdyOrHttpChooser extends ByteToMessageDecoder {
 
+    private static final InternalLogger logger = InternalLoggerFactory.getInstance(SpdyOrHttpChooser.class);
+
     // TODO: Replace with generic NPN handler
 
     public enum SelectedProtocol {
         SPDY_3_1("spdy/3.1"),
         HTTP_1_1("http/1.1"),
-        HTTP_1_0("http/1.0"),
+        HTTP_1_0("http/1.0");
-        UNKNOWN("Unknown");
 
         private final String name;
 
@@ -58,9 +57,8 @@ public abstract class SpdyOrHttpChooser extends ByteToMessageDecoder {
         /**
          * Get an instance of this enum based on the protocol name returned by the NPN server provider
          *
-         * @param name
+         * @param name the protocol name
-         *            the protocol name
+         * @return the selected protocol or {@code null} if there is no match
-         * @return the SelectedProtocol instance
          */
         public static SelectedProtocol protocol(String name) {
             for (SelectedProtocol protocol : SelectedProtocol.values()) {
@@ -68,36 +66,15 @@ public abstract class SpdyOrHttpChooser extends ByteToMessageDecoder {
                     return protocol;
                 }
             }
-            return UNKNOWN;
+            return null;
         }
     }
 
-    private final int maxSpdyContentLength;
+    protected SpdyOrHttpChooser() { }
-    private final int maxHttpContentLength;
-
-    protected SpdyOrHttpChooser(int maxSpdyContentLength, int maxHttpContentLength) {
-        this.maxSpdyContentLength = maxSpdyContentLength;
-        this.maxHttpContentLength = maxHttpContentLength;
-    }
-
-    /**
-     * Return the {@link SelectedProtocol} for the {@link SSLEngine}. If its not known yet implementations MUST return
-     * {@link SelectedProtocol#UNKNOWN}.
-     *
-     */
-    protected SelectedProtocol getProtocol(SSLEngine engine) {
-        String[] protocol = StringUtil.split(engine.getSession().getProtocol(), ':');
-        if (protocol.length < 2) {
-            // Use HTTP/1.1 as default
-            return SelectedProtocol.HTTP_1_1;
-        }
-        SelectedProtocol selectedProtocol = SelectedProtocol.protocol(protocol[1]);
-        return selectedProtocol;
-    }
 
     @Override
     protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) throws Exception {
-        if (initPipeline(ctx)) {
+        if (configurePipeline(ctx)) {
             // When we reached here we can remove this handler as its now clear
             // what protocol we want to use
             // from this point on. This will also take care of forward all
@@ -106,72 +83,95 @@ public abstract class SpdyOrHttpChooser extends ByteToMessageDecoder {
         }
     }
 
-    private boolean initPipeline(ChannelHandlerContext ctx) {
+    private boolean configurePipeline(ChannelHandlerContext ctx) {
         // Get the SslHandler from the ChannelPipeline so we can obtain the
         // SslEngine from it.
         SslHandler handler = ctx.pipeline().get(SslHandler.class);
         if (handler == null) {
             // SslHandler is needed by SPDY by design.
-            throw new IllegalStateException("SslHandler is needed for SPDY");
+            throw new IllegalStateException("cannot find a SslHandler in the pipeline (required for SPDY)");
         }
 
-        SelectedProtocol protocol = getProtocol(handler.engine());
+        if (!handler.handshakeFuture().isDone()) {
-        switch (protocol) {
-        case UNKNOWN:
-            // Not done with choosing the protocol, so just return here for now,
             return false;
+        }
+
+        SelectedProtocol protocol;
+        try {
+            protocol = selectProtocol(handler);
+        } catch (Exception e) {
+            throw new IllegalStateException("failed to get the selected protocol", e);
+        }
+
+        if (protocol == null) {
+            throw new IllegalStateException("unknown protocol");
+        }
+
+        switch (protocol) {
         case SPDY_3_1:
-            addSpdyHandlers(ctx, SpdyVersion.SPDY_3_1);
+            try {
+                configureSpdy(ctx, SpdyVersion.SPDY_3_1);
+            } catch (Exception e) {
+                throw new IllegalStateException("failed to configure a SPDY pipeline", e);
+            }
             break;
         case HTTP_1_0:
         case HTTP_1_1:
-            addHttpHandlers(ctx);
+            try {
+                configureHttp1(ctx);
+            } catch (Exception e) {
+                throw new IllegalStateException("failed to configure a HTTP/1 pipeline", e);
+            }
             break;
-        default:
-            throw new IllegalStateException("Unknown SelectedProtocol");
         }
         return true;
     }
 
     /**
-     * Add all {@link ChannelHandler}'s that are needed for SPDY with the given version.
+     * Returns the {@link SelectedProtocol} for the current SSL session.  By default, this method returns the first
-     */
+     * known protocol.
-    protected void addSpdyHandlers(ChannelHandlerContext ctx, SpdyVersion version) {
-        ChannelPipeline pipeline = ctx.pipeline();
-        pipeline.addLast("spdyFrameCodec", new SpdyFrameCodec(version));
-        pipeline.addLast("spdySessionHandler", new SpdySessionHandler(version, true));
-        pipeline.addLast("spdyHttpEncoder", new SpdyHttpEncoder(version));
-        pipeline.addLast("spdyHttpDecoder", new SpdyHttpDecoder(version, maxSpdyContentLength));
-        pipeline.addLast("spdyStreamIdHandler", new SpdyHttpResponseStreamIdHandler());
-        pipeline.addLast("httpRequestHandler", createHttpRequestHandlerForSpdy());
-    }
-
-    /**
-     * Add all {@link ChannelHandler}'s that are needed for HTTP.
-     */
-    protected void addHttpHandlers(ChannelHandlerContext ctx) {
-        ChannelPipeline pipeline = ctx.pipeline();
-        pipeline.addLast("httpRequestDecoder", new HttpRequestDecoder());
-        pipeline.addLast("httpResponseEncoder", new HttpResponseEncoder());
-        pipeline.addLast("httpChunkAggregator", new HttpObjectAggregator(maxHttpContentLength));
-        pipeline.addLast("httpRequestHandler", createHttpRequestHandlerForHttp());
-    }
-
-    /**
-     * Create the {@link ChannelInboundHandler} that is responsible for handling the http requests
-     * when the {@link SelectedProtocol} was {@link SelectedProtocol#HTTP_1_0} or
-     * {@link SelectedProtocol#HTTP_1_1}
-     */
-    protected abstract ChannelInboundHandler createHttpRequestHandlerForHttp();
-
-    /**
-     * Create the {@link ChannelInboundHandler} that is responsible for handling the http responses
-     * when the {@link SelectedProtocol} was {@link SelectedProtocol#SPDY_3_1}.
      *
-     * By default this getMethod will just delecate to {@link #createHttpRequestHandlerForHttp()}, but sub-classes may
+     * @return the selected application-level protocol, or {@code null} if the application-level protocol name of
-     * override this to change the behaviour.
+     *         the specified {@code sslHandler} is neither {@code "http/1.1"}, {@code "http/1.0"} nor {@code "spdy/3.1"}
      */
-    protected ChannelInboundHandler createHttpRequestHandlerForSpdy() {
+    protected SelectedProtocol selectProtocol(SslHandler sslHandler) throws Exception {
-        return createHttpRequestHandlerForHttp();
+        final String appProto = sslHandler.applicationProtocol();
+        return appProto != null? SelectedProtocol.protocol(appProto) : SelectedProtocol.HTTP_1_1;
+    }
+
+    /**
+     * Configures the {@link Channel} of the specified {@code ctx} for HTTP/2.
+     * <p>
+     * A typical implementation of this method will look like the following:
+     * <pre>
+     * {@link ChannelPipeline} p = ctx.pipeline();
+     * p.addLast(new {@link SpdyFrameCodec}(version));
+     * p.addLast(new {@link SpdySessionHandler}(version, true));
+     * p.addLast(new {@link SpdyHttpEncoder}(version));
+     * p.addLast(new {@link SpdyHttpDecoder}(version, <i>maxSpdyContentLength</i>));
+     * p.addLast(new {@link SpdyHttpResponseStreamIdHandler}());
+     * p.addLast(new <i>YourHttpRequestHandler</i>());
+     * </pre>
+     * </p>
+     */
+    protected abstract void configureSpdy(ChannelHandlerContext ctx, SpdyVersion version) throws Exception;
+
+    /**
+     * Configures the {@link Channel} of the specified {@code ctx} for HTTP/1.
+     * <p>
+     * A typical implementation of this method will look like the following:
+     * <pre>
+     * {@link ChannelPipeline} p = ctx.pipeline();
+     * p.addLast(new {@link HttpServerCodec}());
+     * p.addLast(new <i>YourHttpRequestHandler</i>());
+     * </pre>
+     * </p>
+     */
+    protected abstract void configureHttp1(ChannelHandlerContext ctx) throws Exception;
+
+    @Override
+    public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
+        logger.warn("{} Failed to select the application-level protocol:", ctx.channel(), cause);
+        ctx.close();
     }
 }

example/src/main/java/io/netty/example/spdy/client/SpdyClient.java

@@ -50,9 +50,8 @@ import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
  */
 public final class SpdyClient {
 
-    static final boolean SSL = System.getProperty("ssl") != null;
     static final String HOST = System.getProperty("host", "127.0.0.1");
-    static final int PORT = Integer.parseInt(System.getProperty("port", SSL? "8443" : "8080"));
+    static final int PORT = Integer.parseInt(System.getProperty("port", "8443"));
 
     public static void main(String[] args) throws Exception {
         // Configure SSL.
@@ -98,9 +97,7 @@ public final class SpdyClient {
             // Wait until the connection is closed.
             channel.close().syncUninterruptibly();
         } finally {
-            if (workerGroup != null) {
+            workerGroup.shutdownGracefully();
-                workerGroup.shutdownGracefully();
-            }
         }
     }
 }

example/src/main/java/io/netty/example/spdy/server/SpdyOrHttpHandler.java

@@ -15,8 +15,17 @@
  */
 package io.netty.example.spdy.server;
 
-import io.netty.channel.ChannelInboundHandler;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelPipeline;
+import io.netty.handler.codec.http.HttpObjectAggregator;
+import io.netty.handler.codec.http.HttpServerCodec;
+import io.netty.handler.codec.spdy.SpdyFrameCodec;
+import io.netty.handler.codec.spdy.SpdyHttpDecoder;
+import io.netty.handler.codec.spdy.SpdyHttpEncoder;
+import io.netty.handler.codec.spdy.SpdyHttpResponseStreamIdHandler;
 import io.netty.handler.codec.spdy.SpdyOrHttpChooser;
+import io.netty.handler.codec.spdy.SpdySessionHandler;
+import io.netty.handler.codec.spdy.SpdyVersion;
 
 /**
  * Negotiates with the browser if SPDY or HTTP is going to be used. Once decided, the Netty pipeline is setup with
@@ -26,17 +35,22 @@ public class SpdyOrHttpHandler extends SpdyOrHttpChooser {
 
     private static final int MAX_CONTENT_LENGTH = 1024 * 100;
 
-    public SpdyOrHttpHandler() {
+    @Override
-        this(MAX_CONTENT_LENGTH, MAX_CONTENT_LENGTH);
+    protected void configureSpdy(ChannelHandlerContext ctx, SpdyVersion version) throws Exception {
-    }
+        ChannelPipeline p = ctx.pipeline();
-
+        p.addLast(new SpdyFrameCodec(version));
-    public SpdyOrHttpHandler(int maxSpdyContentLength, int maxHttpContentLength) {
+        p.addLast(new SpdySessionHandler(version, true));
-        super(maxSpdyContentLength, maxHttpContentLength);
+        p.addLast(new SpdyHttpEncoder(version));
+        p.addLast(new SpdyHttpDecoder(version, MAX_CONTENT_LENGTH));
+        p.addLast(new SpdyHttpResponseStreamIdHandler());
+        p.addLast(new SpdyServerHandler());
     }
 
     @Override
-    protected ChannelInboundHandler createHttpRequestHandlerForHttp() {
+    protected void configureHttp1(ChannelHandlerContext ctx) throws Exception {
-        return new SpdyServerHandler();
+        ChannelPipeline p = ctx.pipeline();
+        p.addLast(new HttpServerCodec());
+        p.addLast(new HttpObjectAggregator(MAX_CONTENT_LENGTH));
+        p.addLast(new SpdyServerHandler());
     }
-
 }

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

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2015 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package io.netty.handler.ssl;
+
+/**
+ * Provides a way to get the application-level protocol name from ALPN or NPN.
+ */
+interface ApplicationProtocolAccessor {
+    String getApplicationProtocol();
+}

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

@@ -22,6 +22,7 @@ import java.util.List;
  * Utility class for application protocol common operations.
  */
 final class ApplicationProtocolUtil {
+
     private static final int DEFAULT_LIST_SIZE = 2;
 
     private ApplicationProtocolUtil() {

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

@@ -24,7 +24,7 @@ import javax.security.cert.X509Certificate;
 import java.security.Principal;
 import java.security.cert.Certificate;
 
-final class JdkSslSession implements SSLSession {
+final class JdkSslSession implements SSLSession, ApplicationProtocolAccessor {
     private final SSLEngine engine;
     private volatile String applicationProtocol;
 
@@ -32,39 +32,22 @@ final class JdkSslSession implements SSLSession {
         this.engine = engine;
     }
 
-    void setApplicationProtocol(String applicationProtocol) {
+    private SSLSession unwrap() {
-        if (applicationProtocol != null) {
+        return engine.getSession();
-            applicationProtocol = applicationProtocol.replace(':', '_');
-        }
-        this.applicationProtocol = applicationProtocol;
     }
 
     @Override
     public String getProtocol() {
-        final String protocol = unwrap().getProtocol();
+        return unwrap().getProtocol();
-        final String applicationProtocol = this.applicationProtocol;
-
-        if (applicationProtocol == null) {
-            if (protocol != null) {
-                return protocol.replace(':', '_');
-            } else {
-                return null;
-            }
-        }
-
-        final StringBuilder buf = new StringBuilder(32);
-        if (protocol != null) {
-            buf.append(protocol.replace(':', '_'));
-            buf.append(':');
-        } else {
-            buf.append("null:");
-        }
-        buf.append(applicationProtocol);
-        return buf.toString();
     }
 
-    private SSLSession unwrap() {
+    @Override
-        return engine.getSession();
+    public String getApplicationProtocol() {
+        return applicationProtocol;
+    }
+
+    void setApplicationProtocol(String applicationProtocol) {
+        this.applicationProtocol = applicationProtocol;
     }
 
     @Override

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

@@ -1,170 +0,0 @@
-/*
- * Copyright 2014 The Netty Project
- *
- * The Netty Project licenses this file to you under the Apache License,
- * version 2.0 (the "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at:
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations
- * under the License.
- */
-
-package io.netty.handler.ssl;
-
-import javax.net.ssl.SSLEngine;
-import javax.net.ssl.SSLPeerUnverifiedException;
-import javax.net.ssl.SSLSession;
-import javax.net.ssl.SSLSessionContext;
-import javax.security.cert.X509Certificate;
-import java.security.Principal;
-import java.security.cert.Certificate;
-
-final class JettyNpnSslSession implements SSLSession {
-
-    private final SSLEngine engine;
-    private volatile String applicationProtocol;
-
-    JettyNpnSslSession(SSLEngine engine) {
-        this.engine = engine;
-    }
-
-    void setApplicationProtocol(String applicationProtocol) {
-        if (applicationProtocol != null) {
-            applicationProtocol = applicationProtocol.replace(':', '_');
-        }
-        this.applicationProtocol = applicationProtocol;
-    }
-
-    @Override
-    public String getProtocol() {
-        final String protocol = unwrap().getProtocol();
-        final String applicationProtocol = this.applicationProtocol;
-
-        if (applicationProtocol == null) {
-            if (protocol != null) {
-                return protocol.replace(':', '_');
-            } else {
-                return null;
-            }
-        }
-
-        final StringBuilder buf = new StringBuilder(32);
-        if (protocol != null) {
-            buf.append(protocol.replace(':', '_'));
-            buf.append(':');
-        } else {
-            buf.append("null:");
-        }
-        buf.append(applicationProtocol);
-        return buf.toString();
-    }
-
-    private SSLSession unwrap() {
-        return engine.getSession();
-    }
-
-    @Override
-    public byte[] getId() {
-        return unwrap().getId();
-    }
-
-    @Override
-    public SSLSessionContext getSessionContext() {
-        return unwrap().getSessionContext();
-    }
-
-    @Override
-    public long getCreationTime() {
-        return unwrap().getCreationTime();
-    }
-
-    @Override
-    public long getLastAccessedTime() {
-        return unwrap().getLastAccessedTime();
-    }
-
-    @Override
-    public void invalidate() {
-        unwrap().invalidate();
-    }
-
-    @Override
-    public boolean isValid() {
-        return unwrap().isValid();
-    }
-
-    @Override
-    public void putValue(String s, Object o) {
-        unwrap().putValue(s, o);
-    }
-
-    @Override
-    public Object getValue(String s) {
-        return unwrap().getValue(s);
-    }
-
-    @Override
-    public void removeValue(String s) {
-        unwrap().removeValue(s);
-    }
-
-    @Override
-    public String[] getValueNames() {
-        return unwrap().getValueNames();
-    }
-
-    @Override
-    public Certificate[] getPeerCertificates() throws SSLPeerUnverifiedException {
-        return unwrap().getPeerCertificates();
-    }
-
-    @Override
-    public Certificate[] getLocalCertificates() {
-        return unwrap().getLocalCertificates();
-    }
-
-    @Override
-    public X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException {
-        return unwrap().getPeerCertificateChain();
-    }
-
-    @Override
-    public Principal getPeerPrincipal() throws SSLPeerUnverifiedException {
-        return unwrap().getPeerPrincipal();
-    }
-
-    @Override
-    public Principal getLocalPrincipal() {
-        return unwrap().getLocalPrincipal();
-    }
-
-    @Override
-    public String getCipherSuite() {
-        return unwrap().getCipherSuite();
-    }
-
-    @Override
-    public String getPeerHost() {
-        return unwrap().getPeerHost();
-    }
-
-    @Override
-    public int getPeerPort() {
-        return unwrap().getPeerPort();
-    }
-
-    @Override
-    public int getPacketBufferSize() {
-        return unwrap().getPacketBufferSize();
-    }
-
-    @Override
-    public int getApplicationBufferSize() {
-        return unwrap().getApplicationBufferSize();
-    }
-}

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

@@ -949,7 +949,7 @@ public final class OpenSslEngine extends SSLEngine {
                     SSL.setOptions(ssl, SSL.SSL_OP_NO_TLSv1_2);
                 }
             } else {
-                throw new IllegalStateException("failed to enable protocols: " + protocols);
+                throw new IllegalStateException("failed to enable protocols: " + Arrays.asList(protocols));
             }
         }
     }
@@ -1067,7 +1067,6 @@ public final class OpenSslEngine extends SSLEngine {
     private static String selectApplicationProtocol(List<String> protocols,
                                              SelectedListenerFailureBehavior behavior,
                                              String applicationProtocol) throws SSLException {
-        applicationProtocol = applicationProtocol.replace(':', '_');
         if (behavior == SelectedListenerFailureBehavior.ACCEPT) {
             return applicationProtocol;
         } else {
@@ -1079,7 +1078,7 @@ public final class OpenSslEngine extends SSLEngine {
                 if (behavior == SelectedListenerFailureBehavior.CHOOSE_MY_LAST_PROTOCOL) {
                     return protocols.get(size - 1);
                 } else {
-                    throw new SSLException("Unknown protocol " + applicationProtocol);
+                    throw new SSLException("unknown protocol " + applicationProtocol);
                 }
             }
         }
@@ -1267,7 +1266,7 @@ public final class OpenSslEngine extends SSLEngine {
         shutdown();
     }
 
-    private final class OpenSslSession implements SSLSession {
+    private final class OpenSslSession implements SSLSession, ApplicationProtocolAccessor {
         // SSLSession implementation seems to not need to be thread-safe so no need for volatile etc.
         private X509Certificate[] x509PeerCerts;
 
@@ -1486,20 +1485,18 @@ public final class OpenSslEngine extends SSLEngine {
 
         @Override
         public String getProtocol() {
-            String applicationProtocol = OpenSslEngine.this.applicationProtocol;
-            final String version;
             synchronized (OpenSslEngine.this) {
                 if (destroyed == 0) {
-                    version = SSL.getVersion(ssl);
+                    return SSL.getVersion(ssl);
                 } else {
                     return StringUtil.EMPTY_STRING;
                 }
             }
-            if (applicationProtocol == null || applicationProtocol.isEmpty()) {
+        }
-                return version;
+
-            } else {
+        @Override
-                return version + ':' + applicationProtocol;
+        public String getApplicationProtocol() {
-            }
+            return applicationProtocol;
         }
 
         @Override

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

@@ -50,6 +50,7 @@ import javax.net.ssl.SSLEngineResult;
 import javax.net.ssl.SSLEngineResult.HandshakeStatus;
 import javax.net.ssl.SSLEngineResult.Status;
 import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLSession;
 import java.io.IOException;
 import java.net.SocketAddress;
 import java.nio.ByteBuffer;
@@ -352,6 +353,20 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
         return engine;
     }
 
+    /**
+     * Returns the name of the current application-level protocol.
+     *
+     * @return the protocol name or {@code null} if application-level protocol has not been negotiated
+     */
+    public String applicationProtocol() {
+        SSLSession sess = engine().getSession();
+        if (!(sess instanceof ApplicationProtocolAccessor)) {
+            return null;
+        }
+
+        return ((ApplicationProtocolAccessor) sess).getApplicationProtocol();
+    }
+
     /**
      * Returns a {@link Future} that will get notified once the current TLS handshake completes.
      *

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

@@ -58,4 +58,14 @@ public final class SslHandshakeCompletionEvent {
     public Throwable cause() {
         return cause;
     }
+
+    @Override
+    public String toString() {
+        Throwable cause = cause();
+        if (cause == null) {
+            return "SslHandshakeCompletionEvent(SUCCESS)";
+        } else {
+            return "SslHandshakeCompletionEvent(" + cause + ')';
+        }
+    }
 }

handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java

@@ -262,14 +262,8 @@ public abstract class SSLEngineTest {
     private static void verifyApplicationLevelProtocol(Channel channel, String expectedApplicationProtocol) {
         SslHandler handler = channel.pipeline().get(SslHandler.class);
         assertNotNull(handler);
-        String[] protocol = handler.engine().getSession().getProtocol().split(":");
+        String appProto = handler.applicationProtocol();
-        assertNotNull(protocol);
+        assertEquals(appProto, expectedApplicationProtocol);
-        if (expectedApplicationProtocol != null && !expectedApplicationProtocol.isEmpty()) {
-            assertTrue("protocol.length must be greater than 1 but is " + protocol.length, protocol.length > 1);
-            assertEquals(expectedApplicationProtocol, protocol[1]);
-        } else {
-            assertEquals(1, protocol.length);
-        }
     }
 
     private static void writeAndVerifyReceived(ByteBuf message, Channel sendChannel, CountDownLatch receiverLatch,