From 9b5b43a2fd7e90b69d4f575a61d3174d816bbe70 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 12 Aug 2020 15:50:11 -0500 Subject: [PATCH] Fixing KeyStoreScannerTest Signed-off-by: Joakim Erdfelt --- .../jetty/util/resource/PathResource.java | 8 ++--- .../jetty/util/ssl/KeyStoreScanner.java | 18 +++++++++-- .../jetty/test/KeyStoreScannerTest.java | 30 ++++++++++++------- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java index 925a012c307..e9931f548e4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/PathResource.java @@ -230,15 +230,15 @@ public class PathResource extends Resource Path absPath = path; try { - absPath = path.toAbsolutePath(); + absPath = path.toRealPath(NO_FOLLOW_LINKS); } - catch (IOError ioError) + catch (IOError | IOException e) { - // Not able to resolve absolute path from provided path + // Not able to resolve real/canonical path from provided path // This could be due to a glob reference, or a reference // to a path that doesn't exist (yet) if (LOG.isDebugEnabled()) - LOG.debug("Unable to get absolute path for {}", path, ioError); + LOG.debug("Unable to get real/canonical path for {}", path, e); } // cleanup any lingering relative path nonsense (like "/./" and "/../") 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 e7e817780e8..89f8552fa91 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 @@ -29,6 +29,7 @@ import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.resource.Resource; /** *

The {@link KeyStoreScanner} is used to monitor the KeyStore file used by the {@link SslContextFactory}. @@ -49,11 +50,22 @@ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.Discr this.sslContextFactory = sslContextFactory; try { - keystoreFile = sslContextFactory.getKeyStoreResource().getFile(); - if (keystoreFile == null || !keystoreFile.exists()) + Resource keystoreResource = sslContextFactory.getKeyStoreResource(); + File monitoredFile = keystoreResource.getFile(); + if (monitoredFile == null || !monitoredFile.exists()) throw new IllegalArgumentException("keystore file does not exist"); - if (keystoreFile.isDirectory()) + if (monitoredFile.isDirectory()) throw new IllegalArgumentException("expected keystore file not directory"); + + if (keystoreResource.getAlias() != null) + { + // this resource has an alias, use the alias, as that's what's returned in the Scanner + monitoredFile = new File(keystoreResource.getAlias()); + } + + keystoreFile = monitoredFile; + if (LOG.isDebugEnabled()) + LOG.debug("Monitored Keystore File: {}", monitoredFile); } catch (IOException e) { 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 d7aba199434..2f84f66d4f6 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 @@ -21,6 +21,7 @@ package org.eclipse.jetty.test; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.security.SecureRandom; import java.security.cert.Certificate; import java.security.cert.X509Certificate; @@ -47,11 +48,14 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledOnOs; import org.junit.jupiter.api.extension.ExtendWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.condition.OS.WINDOWS; @ExtendWith(WorkDirExtension.class) public class KeyStoreScannerTest @@ -161,7 +165,8 @@ public class KeyStoreScannerTest // Delete the keystore. try (StacklessLogging ignored = new StacklessLogging(KeyStoreScanner.class)) { - useKeystore(null); + Path keystorePath = keystoreDir.resolve("keystore"); + assertTrue(Files.deleteIfExists(keystorePath)); keystoreScanner.scan(); } @@ -176,6 +181,7 @@ public class KeyStoreScannerTest } @Test + @DisabledOnOs(WINDOWS) // does not support symbolic link public void testReloadChangingSymbolicLink() throws Exception { Path keystorePath = keystoreDir.resolve("symlinkKeystore"); @@ -202,13 +208,19 @@ public class KeyStoreScannerTest } @Test + @DisabledOnOs(WINDOWS) // does not support symbolic link public void testReloadChangingTargetOfSymbolicLink() throws Exception { + Path keystoreLink = keystoreDir.resolve("symlinkKeystore"); + Path oldKeystoreSrc = MavenTestingUtils.getTestResourcePathFile("oldKeystore"); + Path newKeystoreSrc = MavenTestingUtils.getTestResourcePathFile("newKeystore"); + Path target = keystoreDir.resolve("keystore"); + start(sslContextFactory -> { - Path keystorePath = keystoreDir.resolve("symlinkKeystore"); - Files.createSymbolicLink(keystorePath, useKeystore("oldKeystore")); - sslContextFactory.setKeyStorePath(keystorePath.toString()); + Files.copy(oldKeystoreSrc, target); + Files.createSymbolicLink(keystoreLink, target); + sslContextFactory.setKeyStorePath(keystoreLink.toString()); sslContextFactory.setKeyStorePassword("storepwd"); sslContextFactory.setKeyManagerPassword("keypwd"); }); @@ -218,7 +230,8 @@ public class KeyStoreScannerTest assertThat(getExpiryYear(cert1), is(2015)); // Change the target file of the symlink to the newKeystore which has a later expiry date. - useKeystore("newKeystore"); + Files.copy(newKeystoreSrc, target, StandardCopyOption.REPLACE_EXISTING); + System.err.println("### Triggering scan"); keystoreScanner.scan(); // The scanner should have detected the updated keystore, expiry should be renewed. @@ -232,11 +245,7 @@ public class KeyStoreScannerTest if (Files.exists(keystorePath)) Files.delete(keystorePath); - if (keystore == null) - return null; - - Files.copy(MavenTestingUtils.getTestResourceFile(keystore).toPath(), keystorePath); - keystorePath.toFile().deleteOnExit(); + Files.copy(MavenTestingUtils.getTestResourcePath(keystore), keystorePath); if (!Files.exists(keystorePath)) throw new IllegalStateException("keystore file was not created"); @@ -260,6 +269,7 @@ public class KeyStoreScannerTest HttpsURLConnection connection = (HttpsURLConnection)serverUrl.openConnection(); connection.setHostnameVerifier((a, b) -> true); + connection.setRequestProperty("Connection", "close"); connection.connect(); Certificate[] certs = connection.getServerCertificates(); connection.disconnect();