385925: make SslContextFactory.setProtocols and SslContextFactory.setCipherSuites preserve the order of the given parameters

jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java

@@ -1215,8 +1215,8 @@ public class SslContextFactory extends AbstractLifeCycle
         if (_includeProtocols!=null)
         {
             // Use only the supported included protocols
-            for (String protocol : supportedProtocols)
+            for (String protocol : _includeProtocols)
-                if (_includeProtocols.contains(protocol))
+                if(Arrays.asList(supportedProtocols).contains(protocol))
                     selected_protocols.add(protocol);
         }
         else
@@ -1247,8 +1247,8 @@ public class SslContextFactory extends AbstractLifeCycle
         if (_includeCipherSuites!=null)
         {
             // Use only the supported included ciphers
-            for (String cipherSuite : supportedCipherSuites)
+            for (String cipherSuite : _includeCipherSuites)
-                if (_includeCipherSuites.contains(cipherSuite))
+                if(Arrays.asList(supportedCipherSuites).contains(cipherSuite))
                     selected_ciphers.add(cipherSuite);
         }
         else

jetty-util/src/test/java/org/eclipse/jetty/util/ssl/SslContextFactoryTest.java

@@ -12,10 +12,8 @@ package org.eclipse.jetty.util.ssl;
 //You may elect to redistribute this code under either of these licenses. 
 //========================================================================
 
-import static junit.framework.Assert.assertTrue;
-
-import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.security.KeyStore;
 
 import org.eclipse.jetty.util.component.AbstractLifeCycle;
@@ -23,16 +21,30 @@ import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.StdErrLog;
 import org.eclipse.jetty.util.resource.Resource;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
+import static junit.framework.Assert.assertTrue;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
 
 public class SslContextFactoryTest
 {
+
+    private SslContextFactory cf;
+
+    @Before
+    public void setUp() throws Exception
+    {
+        cf = new SslContextFactory();
+    }
+
     @Test
     public void testNoTsFileKs() throws Exception
     {
         String keystorePath = System.getProperty("basedir",".") + "/src/test/resources/keystore";
-        SslContextFactory cf = new SslContextFactory(keystorePath);
         cf.setKeyStorePassword("storepwd");
         cf.setKeyManagerPassword("keypwd");
         
@@ -44,11 +56,9 @@ public class SslContextFactoryTest
     @Test
     public void testNoTsStreamKs() throws Exception
     {
-        String keystorePath = System.getProperty("basedir",".") + "/src/test/resources/keystore";
+        InputStream keystoreInputStream = this.getClass().getResourceAsStream("keystore");
-        
+
-        SslContextFactory cf = new SslContextFactory();
+        cf.setKeyStoreInputStream(keystoreInputStream);
-        
-        cf.setKeyStoreInputStream(new FileInputStream(keystorePath));
         cf.setKeyStorePassword("storepwd");
         cf.setKeyManagerPassword("keypwd");
         
@@ -60,12 +70,11 @@ public class SslContextFactoryTest
     @Test
     public void testNoTsSetKs() throws Exception
     {
-        String keystorePath = System.getProperty("basedir",".") + "/src/test/resources/keystore";
+        InputStream keystoreInputStream = this.getClass().getResourceAsStream("keystore");
-        
+
         KeyStore ks = KeyStore.getInstance("JKS");
-        ks.load(new FileInputStream(keystorePath),"storepwd".toCharArray());
+        ks.load(keystoreInputStream, "storepwd".toCharArray());
-        
+
-        SslContextFactory cf = new SslContextFactory();
         cf.setKeyStore(ks);
         cf.setKeyManagerPassword("keypwd");
         
@@ -77,7 +86,6 @@ public class SslContextFactoryTest
     @Test
     public void testNoTsNoKs() throws Exception
     {
-        SslContextFactory cf = new SslContextFactory();
         cf.start();
         assertTrue(cf.getSslContext()!=null);
     }
@@ -85,7 +93,6 @@ public class SslContextFactoryTest
     @Test
     public void testTrustAll() throws Exception
     {
-        SslContextFactory cf = new SslContextFactory();
         cf.start();
         assertTrue(cf.getSslContext()!=null);
     }
@@ -95,7 +102,6 @@ public class SslContextFactoryTest
     {
         Resource keystoreResource = Resource.newSystemResource("keystore");
 
-        SslContextFactory cf = new SslContextFactory();
         cf.setKeyStoreResource(keystoreResource);
         cf.setKeyStorePassword("storepwd");
         cf.setKeyManagerPassword("keypwd");
@@ -103,7 +109,6 @@ public class SslContextFactoryTest
         cf.start();
 
         assertTrue(cf.getSslContext()!=null);
-
     }
 
     @Test
@@ -112,7 +117,6 @@ public class SslContextFactoryTest
         Resource keystoreResource = Resource.newSystemResource("keystore");
         Resource truststoreResource = Resource.newSystemResource("keystore");
 
-        SslContextFactory cf = new SslContextFactory();
         cf.setKeyStoreResource(keystoreResource);
         cf.setTrustStoreResource(truststoreResource);
         cf.setKeyStorePassword("storepwd");
@@ -130,7 +134,6 @@ public class SslContextFactoryTest
         Resource keystoreResource = Resource.newSystemResource("keystore");
         Resource truststoreResource = Resource.newSystemResource("keystore");
 
-        SslContextFactory cf = new SslContextFactory();
         cf.setKeyStoreResource(keystoreResource);
         cf.setTrustStoreResource(truststoreResource);
         cf.setKeyStorePassword("storepwd");
@@ -154,7 +157,6 @@ public class SslContextFactoryTest
         Resource keystoreResource = Resource.newSystemResource("keystore");
         Resource truststoreResource = Resource.newSystemResource("keystore");
 
-        SslContextFactory cf = new SslContextFactory();
         cf.setKeyStoreResource(keystoreResource);
         cf.setTrustStoreResource(truststoreResource);
         cf.setKeyStorePassword("storepwd");
@@ -175,7 +177,6 @@ public class SslContextFactoryTest
     @Test
     public void testNoKeyConfig() throws Exception
     {
-        SslContextFactory cf = new SslContextFactory();
         try
         {
             ((StdErrLog)Log.getLogger(AbstractLifeCycle.class)).setHideStacks(true);
@@ -192,4 +193,36 @@ public class SslContextFactoryTest
             Assert.fail("Unexpected exception");
         }
     }
+
+    @Test
+    public void testSetIncludeCipherSuitesPreservesOrder()
+    {
+        String[] supportedCipherSuites = new String[]{"cipher4", "cipher2", "cipher1", "cipher3"};
+        String[] includeCipherSuites = {"cipher1", "cipher3", "cipher4"};
+
+        cf.setIncludeCipherSuites(includeCipherSuites);
+        String[] selectedCipherSuites = cf.selectCipherSuites(null, supportedCipherSuites);
+
+        assertSelectedMatchesIncluded(includeCipherSuites, selectedCipherSuites);
+    }
+
+    @Test
+    public void testSetIncludeProtocolsPreservesOrder()
+    {
+        String[] supportedProtocol = new String[]{"cipher4", "cipher2", "cipher1", "cipher3"};
+        String[] includeProtocol = {"cipher1", "cipher3", "cipher4"};
+
+        cf.setIncludeProtocols(includeProtocol);
+        String[] selectedProtocol = cf.selectProtocols(null, supportedProtocol);
+
+        assertSelectedMatchesIncluded(includeProtocol, selectedProtocol);
+    }
+
+    private void assertSelectedMatchesIncluded(String[] includeStrings, String[] selectedStrings)
+    {
+        assertThat(includeStrings.length + " strings are selected", selectedStrings.length, is(includeStrings.length));
+        assertThat("order from includeStrings is preserved", selectedStrings[0], equalTo(includeStrings[0]));
+        assertThat("order from includeStrings is preserved", selectedStrings[1], equalTo(includeStrings[1]));
+        assertThat("order from includeStrings is preserved", selectedStrings[2], equalTo(includeStrings[2]));
+    }
 }