Fix Java9SslEngine implementation of ApplicationProtocolAccessor and …

…so fix ApplicationProtocolNegationHandler

Motivation:

Java9SslEngine did not correctly implement ApplicationProtocolAccessor and so returned an empty String when no extension was used while the interface contract is to return null. This lead to the situation that ApplicationProtocolNegationHandler did not correct work.

Modifications:

- Rename ApplicationProtocolAccessor.getApplicationProtocol() to getNegotiatedApplicationProtocol() which resolves the clash with the method exposed by Java9s SSLEngine.
- Correctly implement getNegotiatedApplicationProtocol() for Java9Sslengine
- Add delegate in Java9Sslengine.getApplicationProtocol() which is provided by Java9
- Adjust tests to test the correct behaviour.

Result:

Fixes [#7251].

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

@@ -26,5 +26,5 @@ interface ApplicationProtocolAccessor {
      * @return the application-level protocol name or
      *         {@code null} if the negotiation failed or the client does not have ALPN/NPN extension
      */
-    String getApplicationProtocol();
+    String getNegotiatedApplicationProtocol();
 }

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

@@ -150,17 +150,25 @@ public SSLEngineResult unwrap(ByteBuffer src, ByteBuffer[] dst, int offset, int
     }
 
     @Override
-    void setApplicationProtocol(String applicationProtocol) {
+    void setNegotiatedApplicationProtocol(String applicationProtocol) {
         // Do nothing as this is handled internally by the Java9 implementation of SSLEngine.
     }
 
     @Override
-    public String getApplicationProtocol() {
-        return Java9SslUtils.getApplicationProtocol(getWrappedEngine());
+    public String getNegotiatedApplicationProtocol() {
+        String protocol = getApplicationProtocol();
+        if (protocol != null) {
+            return protocol.isEmpty() ? null : protocol;
+        }
+        return protocol;
     }
 
     // These methods will override the methods defined by Java 9. As we compile with Java8 we can not add
     // @Override annotations here.
+    public String getApplicationProtocol() {
+        return Java9SslUtils.getApplicationProtocol(getWrappedEngine());
+    }
+
     public String getHandshakeApplicationProtocol() {
         return Java9SslUtils.getHandshakeApplicationProtocol(getWrappedEngine());
     }

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

@@ -137,22 +137,22 @@ static class NoFailProtocolSelector implements ProtocolSelector {
 
         @Override
         public void unsupported() {
-            engineWrapper.setApplicationProtocol(null);
+            engineWrapper.setNegotiatedApplicationProtocol(null);
         }
 
         @Override
         public String select(List<String> protocols) throws Exception {
             for (String p : supportedProtocols) {
                 if (protocols.contains(p)) {
-                    engineWrapper.setApplicationProtocol(p);
+                    engineWrapper.setNegotiatedApplicationProtocol(p);
                     return p;
                 }
             }
             return noSelectMatchFound();
         }
 
         public String noSelectMatchFound() throws Exception {
-            engineWrapper.setApplicationProtocol(null);
+            engineWrapper.setNegotiatedApplicationProtocol(null);
             return null;
         }
     }
@@ -179,13 +179,13 @@ private static class NoFailProtocolSelectionListener implements ProtocolSelectio
 
         @Override
         public void unsupported() {
-            engineWrapper.setApplicationProtocol(null);
+            engineWrapper.setNegotiatedApplicationProtocol(null);
         }
 
         @Override
         public void selected(String protocol) throws Exception {
             if (supportedProtocols.contains(protocol)) {
-                engineWrapper.setApplicationProtocol(protocol);
+                engineWrapper.setNegotiatedApplicationProtocol(protocol);
             } else {
                 noSelectedMatchFound(protocol);
             }

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

@@ -33,11 +33,11 @@ class JdkSslEngine extends SSLEngine implements ApplicationProtocolAccessor {
     }
 
     @Override
-    public String getApplicationProtocol() {
+    public String getNegotiatedApplicationProtocol() {
         return applicationProtocol;
     }
 
-    void setApplicationProtocol(String applicationProtocol) {
+    void setNegotiatedApplicationProtocol(String applicationProtocol) {
         this.applicationProtocol = applicationProtocol;
     }
 

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

@@ -1829,7 +1829,7 @@ final boolean checkSniHostnameMatch(String hostname) {
     }
 
     @Override
-    public String getApplicationProtocol() {
+    public String getNegotiatedApplicationProtocol() {
         return applicationProtocol;
     }
 

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

@@ -66,7 +66,6 @@
 import javax.net.ssl.SSLEngineResult.HandshakeStatus;
 import javax.net.ssl.SSLEngineResult.Status;
 import javax.net.ssl.SSLException;
-import javax.net.ssl.SSLSession;
 
 import static io.netty.buffer.ByteBufUtil.ensureWritableSuccess;
 import static io.netty.handler.ssl.SslUtils.getEncryptedPacketLength;
@@ -578,7 +577,7 @@ public String applicationProtocol() {
             return null;
         }
 
-        return ((ApplicationProtocolAccessor) engine).getApplicationProtocol();
+        return ((ApplicationProtocolAccessor) engine).getNegotiatedApplicationProtocol();
     }
 
     /**

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

@@ -42,6 +42,7 @@
 import io.netty.util.concurrent.Promise;
 import io.netty.util.internal.EmptyArrays;
 import io.netty.util.internal.PlatformDependent;
+import io.netty.util.internal.StringUtil;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -996,10 +997,8 @@ protected void runTest(String expectedApplicationProtocol) throws Exception {
         try {
             writeAndVerifyReceived(clientMessage.retain(), clientChannel, serverLatch, serverReceiver);
             writeAndVerifyReceived(serverMessage.retain(), serverConnectedChannel, clientLatch, clientReceiver);
-            if (expectedApplicationProtocol != null) {
-                verifyApplicationLevelProtocol(clientChannel, expectedApplicationProtocol);
-                verifyApplicationLevelProtocol(serverConnectedChannel, expectedApplicationProtocol);
-            }
+            verifyApplicationLevelProtocol(clientChannel, expectedApplicationProtocol);
+            verifyApplicationLevelProtocol(serverConnectedChannel, expectedApplicationProtocol);
         } finally {
             clientMessage.release();
             serverMessage.release();
@@ -1011,6 +1010,14 @@ private static void verifyApplicationLevelProtocol(Channel channel, String expec
         assertNotNull(handler);
         String appProto = handler.applicationProtocol();
         assertEquals(appProto, expectedApplicationProtocol);
+
+        SSLEngine engine = handler.engine();
+        if (engine instanceof Java9SslEngine) {
+            // Also verify the Java9 exposed method.
+            Java9SslEngine java9SslEngine = (Java9SslEngine) engine;
+            assertEquals(expectedApplicationProtocol == null ? StringUtil.EMPTY_STRING : expectedApplicationProtocol,
+                    java9SslEngine.getApplicationProtocol());
+        }
     }
 
     private static void writeAndVerifyReceived(ByteBuf message, Channel sendChannel, CountDownLatch receiverLatch,