From b81f28ce6ec29f311075f5bef63d7ffd6ac5d27e Mon Sep 17 00:00:00 2001
From: Jonathan Lennox <jonathan.lennox@8x8.com>
Date: Mon, 21 Sep 2020 17:21:11 -0400
Subject: [PATCH] Fall back properly to automatically-chosen factory classes if
 the chosen one isn't working.

Don't bother benchmarking the chosen factory class; it either works or it doesn't.
---
 src/main/java/org/jitsi/srtp/crypto/Aes.java | 106 ++++++++++++-------
 1 file changed, 65 insertions(+), 41 deletions(-)

diff --git a/src/main/java/org/jitsi/srtp/crypto/Aes.java b/src/main/java/org/jitsi/srtp/crypto/Aes.java
index e59e9a1..b732aa2 100644
--- a/src/main/java/org/jitsi/srtp/crypto/Aes.java
+++ b/src/main/java/org/jitsi/srtp/crypto/Aes.java
@@ -269,8 +269,7 @@ public void run(Cipher cipher) throws Exception
     private static CipherFactory benchmark(
             CipherFactory[] factories,
             int keySize,
-            String transformation,
-            boolean warmup
+            String transformation
         )
     {
         long minTime = Long.MAX_VALUE;
@@ -285,6 +284,14 @@ private static CipherFactory benchmark(
             if (factory == null)
                 continue;
 
+            // The user may have specified a specific CipherFactory class
+            // (name) through setFactoryClassName(String), Practically, FACTORY_CLASS_NAME may override
+            // minFactory and, consequently, it may appear that the benchmark is
+            // unnecessary. Technically though, the specified CipherFactory may
+            // malfunction. That is why FACTORY_CLASS_NAME is selected after it has
+            // proven itself functional.
+            boolean chosenFactoryClass = (factory.getClass().equals(Aes.factoryClass));
+
             try
             {
                 Cipher cipher = factory.createCipher(transformation);
@@ -298,18 +305,24 @@ private static CipherFactory benchmark(
                 }
                 else
                 {
+                    /* If this is the user-chosen factory class, we don't really
+                     * need to benchmark it; just verify that it's working.
+                     */
+                    int numWarmups = chosenFactoryClass ? 0 : NUM_WARMUPS;
+                    int numBenchmarks = chosenFactoryClass ? 1 : NUM_BENCHMARKS;
+
                     BenchmarkOperation benchmark = BenchmarkOperation.getBenchmark(transformation, keySize);
-                    if (warmup)
+                    if (!chosenFactoryClass)
                     {
                         // Let the JVM "warm up" (do JIT compilation and the like)
-                        for (int i = 0; i < NUM_WARMUPS; i++)
+                        for (int i = 0; i < numWarmups; i++)
                         {
                             benchmark.run(cipher);
                         }
                     }
 
                     long startTime = System.nanoTime();
-                    for (int i = 0; i < NUM_BENCHMARKS; i++)
+                    for (int i = 0; i < numBenchmarks; i++)
                     {
                         benchmark.run(cipher);
                     }
@@ -338,6 +351,19 @@ private static CipherFactory benchmark(
                 else if (t instanceof ThreadDeath)
                     throw (ThreadDeath) t;
             }
+
+            if (chosenFactoryClass)
+            {
+                if (minFactory != null)
+                {
+                    return minFactory;
+                }
+                else
+                {
+                    logger.warn("Chosen factory class \"" + FACTORY_CLASS_NAME +
+                        "\" not working for " + transformation);
+                }
+            }
         }
 
         if (log.length() != 0)
@@ -540,26 +566,47 @@ else if (t instanceof ThreadDeath)
 
         // If FACTORY_CLASS_NAME does not specify a well-known Class, add the
         // new Class to FACTORY_CLASSES.
-        if (add && (factoryClass != null))
+        if (factoryClass != null)
         {
-            for (Class<?> clazz : factoryClasses)
+            Class<?>[] newFactoryClasses;
+            if (add)
             {
-                if (factoryClass.equals(clazz))
+                for (Class<?> clazz : factoryClasses)
                 {
-                    add = false;
-                    break;
+                    if (factoryClass.equals(clazz))
+                    {
+                        add = false;
+                        break;
+                    }
                 }
-            }
-            if (add)
-            {
-                Class<?>[] newFactoryClasses
-                    = new Class<?>[1 + factoryClasses.length];
+                if (add)
+                {
+                    newFactoryClasses
+                        = new Class<?>[1 + factoryClasses.length];
 
-                newFactoryClasses[0] = factoryClass;
-                System.arraycopy(
+                    newFactoryClasses[0] = factoryClass;
+                    System.arraycopy(
                         factoryClasses, 0,
                         newFactoryClasses, 1,
                         factoryClasses.length);
+                }
+            }
+            else
+            {
+                /* Otherwise, move the FACTORY_CLASS to the beginning of the list. */
+                newFactoryClasses
+                    = new Class<?>[factoryClasses.length];
+
+                newFactoryClasses[0] = factoryClass;
+                int i = 1;
+                for (Class<?> clazz : factoryClasses)
+                {
+                    if (!factoryClass.equals(clazz))
+                    {
+                        newFactoryClasses[i] = clazz;
+                        i++;
+                    }
+                }
                 factoryClasses = newFactoryClasses;
             }
         }
@@ -642,30 +689,7 @@ private static CipherFactory getCipherFactory(String transformation, boolean war
         // Benchmark the StreamCiphers provided by the available
         // StreamCipherFactories in order to select the fastest-performing
         // CipherFactory.
-        CipherFactory minFactory = benchmark(factories, keySize, transformation, warmup);
-
-        // The user may have specified a specific CipherFactory class
-        // (name) through setFactoryClassName(String), Practically, FACTORY_CLASS_NAME may override
-        // minFactory and, consequently, it may appear that the benchmark is
-        // unnecessary. Technically though, the specified CipherFactory may
-        // malfunction. That is why FACTORY_CLASS_NAME is selected after it has
-        // proven itself functional.
-        {
-            Class<? extends CipherFactory> factoryClass = Aes.factoryClass;
-
-            if (factoryClass != null)
-            {
-                for (CipherFactory factory : factories)
-                {
-                    if ((factory != null)
-                            && factory.getClass().equals(factoryClass))
-                    {
-                        minFactory = factory;
-                        break;
-                    }
-                }
-            }
-        }
+        CipherFactory minFactory = benchmark(factories, keySize, transformation);
 
         return minFactory;
     }