From 9c882ee7966ed2c890e8079db5a81596cf0cb2c8 Mon Sep 17 00:00:00 2001
From: Simone Bordet <simone.bordet@gmail.com>
Date: Wed, 2 Dec 2020 19:24:19 +0100
Subject: [PATCH] Improved KeyStoreScanner.scan().

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
---
 .../jetty/util/ssl/KeyStoreScanner.java       | 22 ++++++++-----------
 .../jetty/test/KeyStoreScannerTest.java       | 12 +++++-----
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java
index 23b6aa4f3ce..a4676120df7 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java
@@ -20,7 +20,7 @@ package org.eclipse.jetty.util.ssl;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 
@@ -119,23 +119,18 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
     }
 
     @ManagedOperation(value = "Scan for changes in the SSL Keystore", impact = "ACTION")
-    public boolean scan()
+    public boolean scan(long waitMillis)
     {
         if (LOG.isDebugEnabled())
             LOG.debug("scanning");
 
+        CompletableFuture<Boolean> cf = new CompletableFuture<>();
         try
         {
-            CountDownLatch complete = new CountDownLatch(2);
-            Callback callback = Callback.from(complete::countDown, t -> 
-            {
-                LOG.warn("Scan fail", t);
-                complete.countDown();
-            });
-
-            _scanner.scan(callback);
-            _scanner.scan(callback);
-            return complete.await(10, TimeUnit.SECONDS);
+            // Perform 2 scans to be sure that the scan is stable.
+            _scanner.scan(Callback.from(() ->
+                _scanner.scan(Callback.from(() -> cf.complete(true), cf::completeExceptionally)), cf::completeExceptionally));
+            return cf.get(waitMillis, TimeUnit.MILLISECONDS);
         }
         catch (Exception e)
         {
@@ -152,7 +147,8 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr
         try
         {
             sslContextFactory.reload(scf ->
-            {});
+            {
+            });
         }
         catch (Throwable t)
         {
diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java
index 8fb48f94f1d..35fb52af950 100644
--- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java
+++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/KeyStoreScannerTest.java
@@ -126,7 +126,7 @@ public class KeyStoreScannerTest
 
         // Switch to use newKeystore which has a later expiry date.
         useKeystore("newKeystore");
-        assertTrue(keystoreScanner.scan());
+        assertTrue(keystoreScanner.scan(5000));
 
         // The scanner should have detected the updated keystore, expiry should be renewed.
         X509Certificate cert2 = getCertificateFromServer();
@@ -146,7 +146,7 @@ public class KeyStoreScannerTest
         try (StacklessLogging ignored = new StacklessLogging(KeyStoreScanner.class))
         {
             useKeystore("badKeystore");
-            keystoreScanner.scan();
+            keystoreScanner.scan(5000);
         }
 
         // The good keystore is removed, now the bad keystore now causes an exception.
@@ -167,7 +167,7 @@ public class KeyStoreScannerTest
         {
             Path keystorePath = keystoreDir.resolve("keystore");
             assertTrue(Files.deleteIfExists(keystorePath));
-            keystoreScanner.scan();
+            keystoreScanner.scan(5000);
         }
 
         // The good keystore is removed, having no keystore causes an exception.
@@ -175,7 +175,7 @@ public class KeyStoreScannerTest
 
         // Switch to use keystore2 which has a later expiry date.
         useKeystore("newKeystore");
-        keystoreScanner.scan();
+        keystoreScanner.scan(5000);
         X509Certificate cert2 = getCertificateFromServer();
         assertThat(getExpiryYear(cert2), is(2020));
     }
@@ -200,7 +200,7 @@ public class KeyStoreScannerTest
         // Change the symlink to point to the newKeystore file location which has a later expiry date.
         Files.delete(keystorePath);
         Files.createSymbolicLink(keystorePath, useKeystore("newKeystore"));
-        keystoreScanner.scan();
+        keystoreScanner.scan(5000);
 
         // The scanner should have detected the updated keystore, expiry should be renewed.
         X509Certificate cert2 = getCertificateFromServer();
@@ -232,7 +232,7 @@ public class KeyStoreScannerTest
         // Change the target file of the symlink to the newKeystore which has a later expiry date.
         Files.copy(newKeystoreSrc, target, StandardCopyOption.REPLACE_EXISTING);
         System.err.println("### Triggering scan");
-        keystoreScanner.scan();
+        keystoreScanner.scan(5000);
 
         // The scanner should have detected the updated keystore, expiry should be renewed.
         X509Certificate cert2 = getCertificateFromServer();