diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkAlpnSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/JdkAlpnSslEngine.java index 580b3f23f5..012b1676bf 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkAlpnSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkAlpnSslEngine.java @@ -20,7 +20,7 @@ import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectionLi import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector; import io.netty.util.internal.PlatformDependent; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import javax.net.ssl.SSLEngine; @@ -64,7 +64,8 @@ final class JdkAlpnSslEngine extends JdkSslEngine { if (server) { final ProtocolSelector protocolSelector = checkNotNull(applicationNegotiator.protocolSelectorFactory() - .newSelector(this, new HashSet(applicationNegotiator.protocols())), "protocolSelector"); + .newSelector(this, new LinkedHashSet(applicationNegotiator.protocols())), + "protocolSelector"); ALPN.put(engine, new ServerProvider() { @Override public String select(List protocols) { diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkBaseApplicationProtocolNegotiator.java b/handler/src/main/java/io/netty/handler/ssl/JdkBaseApplicationProtocolNegotiator.java index aacff48426..d9d0d83f20 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkBaseApplicationProtocolNegotiator.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkBaseApplicationProtocolNegotiator.java @@ -142,9 +142,8 @@ class JdkBaseApplicationProtocolNegotiator implements JdkApplicationProtocolNego @Override public String select(List protocols) throws Exception { - for (int i = 0; i < protocols.size(); ++i) { - String p = protocols.get(i); - if (supportedProtocols.contains(p)) { + for (String p : supportedProtocols) { + if (protocols.contains(p)) { jettyWrapper.getSession().setApplicationProtocol(p); return p; } diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkNpnSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/JdkNpnSslEngine.java index 4864c6b568..43d5bb584f 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkNpnSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkNpnSslEngine.java @@ -21,7 +21,7 @@ import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectionLi import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector; import io.netty.util.internal.PlatformDependent; -import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import javax.net.ssl.SSLEngine; @@ -88,7 +88,8 @@ final class JdkNpnSslEngine extends JdkSslEngine { }); } else { final ProtocolSelector protocolSelector = checkNotNull(applicationNegotiator.protocolSelectorFactory() - .newSelector(this, new HashSet(applicationNegotiator.protocols())), "protocolSelector"); + .newSelector(this, new LinkedHashSet(applicationNegotiator.protocols())), + "protocolSelector"); NextProtoNego.put(engine, new ClientProvider() { @Override public boolean supports() { diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java index 41b2da7436..b81883bff0 100644 --- a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java @@ -17,6 +17,7 @@ package io.netty.handler.ssl; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeNoException; import static org.mockito.Mockito.verify; @@ -61,7 +62,8 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; public class JdkSslEngineTest { - private static final String APPLICATION_LEVEL_PROTOCOL = "my-protocol"; + private static final String PREFERRED_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http2"; + private static final String FALLBACK_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http1_1"; private static final String APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE = "my-protocol-FOO"; @Mock @@ -123,6 +125,7 @@ public class JdkSslEngineTest { clientChannel = null; serverChannel = null; serverConnectedChannel = null; + serverException = null; } @Test @@ -135,7 +138,7 @@ public class JdkSslEngineTest { throw new RuntimeException("NPN not on classpath"); } JdkApplicationProtocolNegotiator apn = new JdkNpnApplicationProtocolNegotiator(true, true, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); mySetup(apn); runTest(); } catch (RuntimeException e) { @@ -155,7 +158,7 @@ public class JdkSslEngineTest { throw new RuntimeException("NPN not on classpath"); } JdkApplicationProtocolNegotiator clientApn = new JdkNpnApplicationProtocolNegotiator(false, false, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); JdkApplicationProtocolNegotiator serverApn = new JdkNpnApplicationProtocolNegotiator(false, false, APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE); mySetup(serverApn, clientApn); @@ -177,7 +180,7 @@ public class JdkSslEngineTest { throw new RuntimeException("NPN not on classpath"); } JdkApplicationProtocolNegotiator clientApn = new JdkNpnApplicationProtocolNegotiator(true, true, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); JdkApplicationProtocolNegotiator serverApn = new JdkNpnApplicationProtocolNegotiator(false, false, APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE); mySetup(serverApn, clientApn); @@ -200,7 +203,7 @@ public class JdkSslEngineTest { throw new RuntimeException("NPN not on classpath"); } JdkApplicationProtocolNegotiator clientApn = new JdkNpnApplicationProtocolNegotiator(false, false, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); JdkApplicationProtocolNegotiator serverApn = new JdkNpnApplicationProtocolNegotiator(true, true, APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE); mySetup(serverApn, clientApn); @@ -223,7 +226,7 @@ public class JdkSslEngineTest { throw new RuntimeException("ALPN not on classpath"); } JdkApplicationProtocolNegotiator apn = new JdkAlpnApplicationProtocolNegotiator(true, true, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); mySetup(apn); runTest(); } catch (Exception e) { @@ -243,7 +246,7 @@ public class JdkSslEngineTest { throw new RuntimeException("ALPN not on classpath"); } JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(false, false, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator(false, false, APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE); mySetup(serverApn, clientApn); @@ -265,7 +268,7 @@ public class JdkSslEngineTest { throw new RuntimeException("ALPN not on classpath"); } JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(false, false, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator(true, true, APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE); mySetup(serverApn, clientApn); @@ -278,6 +281,31 @@ public class JdkSslEngineTest { } } + @Test + public void testAlpnCompatibleProtocolsDifferentClientOrder() throws Exception { + try { + // Typical code will not have to check this, but will get a initialization error on class load. + // Check in this test just in case we have multiple tests that just the class and we already ignored the + // initialization error. + if (!JdkAlpnSslEngine.isAvailable()) { + throw new RuntimeException("ALPN not on classpath"); + } + // Even the preferred application protocol appears second in the client's list, it will be picked + // because it's the first one on server's list. + JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(false, false, + FALLBACK_APPLICATION_LEVEL_PROTOCOL, PREFERRED_APPLICATION_LEVEL_PROTOCOL); + JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator(true, true, + PREFERRED_APPLICATION_LEVEL_PROTOCOL, FALLBACK_APPLICATION_LEVEL_PROTOCOL); + mySetup(serverApn, clientApn); + assertNull(serverException); + runTest(PREFERRED_APPLICATION_LEVEL_PROTOCOL); + } catch (Exception e) { + // ALPN availability is dependent on the java version. If ALPN is not available because of + // java version incompatibility don't fail the test, but instead just skip the test + assumeNoException(e); + } + } + @Test public void testAlpnNoCompatibleProtocolsClientHandshakeFailure() throws Exception { try { @@ -288,7 +316,7 @@ public class JdkSslEngineTest { throw new RuntimeException("ALPN not on classpath"); } JdkApplicationProtocolNegotiator clientApn = new JdkAlpnApplicationProtocolNegotiator(true, true, - APPLICATION_LEVEL_PROTOCOL); + PREFERRED_APPLICATION_LEVEL_PROTOCOL); JdkApplicationProtocolNegotiator serverApn = new JdkAlpnApplicationProtocolNegotiator( new ProtocolSelectorFactory() { @Override @@ -514,7 +542,7 @@ public class JdkSslEngineTest { } private void runTest() throws Exception { - runTest(APPLICATION_LEVEL_PROTOCOL); + runTest(PREFERRED_APPLICATION_LEVEL_PROTOCOL); } private void runTest(String expectedApplicationProtocol) throws Exception {