Revert "Bjorncs/jdisc http service cleanup"

4 files changed, 94 insertions, 42 deletions

diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java
index 5dd5dca1667..e71bd190a37 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/SslContextFactory.java
@@ -1,11 +1,16 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.jdisc.http;
 
-import com.yahoo.jdisc.http.ssl.jks.JksKeyStore;
+import com.yahoo.jdisc.http.ssl.SslKeyStore;
 
 import javax.net.ssl.KeyManagerFactory;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.TrustManagerFactory;
+import java.io.IOException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.UnrecoverableKeyException;
+import java.security.cert.CertificateException;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -27,16 +32,16 @@ public class SslContextFactory {
         return this.sslContext;
     }
 
-    public static SslContextFactory newInstanceFromTrustStore(JksKeyStore trustStore) {
+    public static SslContextFactory newInstanceFromTrustStore(SslKeyStore trustStore) {
         return newInstance(DEFAULT_ALGORITHM, DEFAULT_PROTOCOL, null, trustStore);
     }
 
-    public static SslContextFactory newInstance(JksKeyStore trustStore, JksKeyStore keyStore) {
+    public static SslContextFactory newInstance(SslKeyStore trustStore, SslKeyStore keyStore) {
         return newInstance(DEFAULT_ALGORITHM, DEFAULT_PROTOCOL, keyStore, trustStore);
     }
 
     public static SslContextFactory newInstance(String sslAlgorithm, String sslProtocol,
-                                                JksKeyStore keyStore, JksKeyStore trustStore) {
+                                                SslKeyStore keyStore, SslKeyStore trustStore) {
         log.fine("Configuring SSLContext...");
         log.fine("Using " + sslAlgorithm + " algorithm.");
         try {
@@ -55,14 +60,15 @@ public class SslContextFactory {
     /**
      * Used for the key store, which contains the SSL cert and private key.
      */
-    public static javax.net.ssl.KeyManager[] getKeyManagers(JksKeyStore keyStore,
-                                                            String sslAlgorithm) throws Exception {
+    public static javax.net.ssl.KeyManager[] getKeyManagers(SslKeyStore keyStore,
+                                                            String sslAlgorithm)
+            throws NoSuchAlgorithmException, CertificateException, IOException, UnrecoverableKeyException,
+                   KeyStoreException {
 
         KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(sslAlgorithm);
-        String keyStorePassword = keyStore.getKeyStorePassword();
         keyManagerFactory.init(
                 keyStore.loadJavaKeyStore(),
-                keyStorePassword != null ? keyStorePassword.toCharArray() : null);
+                keyStore.getKeyStorePassword().map(String::toCharArray).orElse(null));
         log.fine("KeyManagerFactory initialized with keystore");
         return keyManagerFactory.getKeyManagers();
     }
@@ -71,9 +77,9 @@ public class SslContextFactory {
      * Used for the trust store, which contains certificates from other parties that you expect to communicate with,
      * or from Certificate Authorities that you trust to identify other parties.
      */
-    public static javax.net.ssl.TrustManager[] getTrustManagers(JksKeyStore trustStore,
+    public static javax.net.ssl.TrustManager[] getTrustManagers(SslKeyStore trustStore,
                                                                 String sslAlgorithm)
-            throws Exception {
+            throws NoSuchAlgorithmException, CertificateException, IOException, KeyStoreException {
 
         TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(sslAlgorithm);
         trustManagerFactory.init(trustStore.loadJavaKeyStore());
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java
index 1380abc03f3..25457c0c6c6 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java
@@ -1,20 +1,33 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.jdisc.http.server.jetty;
 
+import com.google.common.collect.ImmutableMap;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.http.ConnectorConfig;
 import com.yahoo.jdisc.http.SecretStore;
+import com.yahoo.jdisc.http.ssl.ReaderForPath;
+import com.yahoo.jdisc.http.SslContextFactory;
+import com.yahoo.jdisc.http.ssl.SslKeyStore;
+import com.yahoo.jdisc.http.ssl.pem.PemKeyStore;
+import com.yahoo.jdisc.http.ssl.pem.PemSslKeyStore;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.testng.annotations.Test;
 
+import javax.net.ssl.SSLContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.net.InetSocketAddress;
+import java.nio.channels.FileChannel;
 import java.nio.channels.ServerSocketChannel;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Collections;
 import java.util.Map;
 
 import static com.yahoo.jdisc.http.ConnectorConfig.Ssl;
@@ -60,7 +73,7 @@ public class ConnectorFactoryTest {
             ConnectorFactory factory = new ConnectorFactory(new ConnectorConfig(new ConnectorConfig.Builder()),
                                                             new ThrowingSecretStore());
             ConnectorFactory.JDiscServerConnector connector =
-                    (ConnectorFactory.JDiscServerConnector)factory.createConnector(new DummyMetric(), server, null);
+                    (ConnectorFactory.JDiscServerConnector)factory.createConnector(new DummyMetric(), server, null, Collections.emptyMap());
             server.addConnector(connector);
             server.setHandler(new HelloWorldHandler());
             server.start();
@@ -86,7 +99,7 @@ public class ConnectorFactoryTest {
             serverChannel.socket().bind(new InetSocketAddress(0));
 
             ConnectorFactory factory = new ConnectorFactory(new ConnectorConfig(new ConnectorConfig.Builder()), new ThrowingSecretStore());
-            ConnectorFactory.JDiscServerConnector connector = (ConnectorFactory.JDiscServerConnector) factory.createConnector(new DummyMetric(), server, serverChannel);
+            ConnectorFactory.JDiscServerConnector connector = (ConnectorFactory.JDiscServerConnector) factory.createConnector(new DummyMetric(), server, serverChannel, Collections.emptyMap());
             server.addConnector(connector);
             server.setHandler(new HelloWorldHandler());
             server.start();
@@ -104,6 +117,63 @@ public class ConnectorFactoryTest {
         }
     }
 
+    @Test
+    public void pre_bound_keystore_file_channels_are_used() throws Exception {
+        Path pemKeyStoreDirectory = Paths.get("src/test/resources/pem/");
+
+        Path certificateFile = pemKeyStoreDirectory.resolve("test.crt");
+        Path privateKeyFile = pemKeyStoreDirectory.resolve("test.key");
+
+        Server server = new Server();
+        try {
+            ServerSocketChannel serverChannel = ServerSocketChannel.open();
+            serverChannel.socket().bind(new InetSocketAddress(0));
+
+            String fakeCertificatePath = "ensure-certificate-path-is-not-used-to-open-the-file";
+            String fakeKeyPath = "ensure-key-path-is-not-used-to-open-the-file";
+
+            ConnectorConfig.Builder builder = new ConnectorConfig.Builder();
+            builder.ssl(
+                    new Ssl.Builder().
+                            enabled(true).
+                            keyStoreType(PEM).
+                            pemKeyStore(new Ssl.PemKeyStore.Builder().
+                                    certificatePath(fakeCertificatePath).
+                                    keyPath(fakeKeyPath)));
+
+            FileChannel certificateChannel = FileChannel.open(certificateFile, StandardOpenOption.READ);
+            FileChannel privateKeyChannel = FileChannel.open(privateKeyFile, StandardOpenOption.READ);
+
+            Map<Path, FileChannel> keyStoreChannels = ImmutableMap.<Path, FileChannel>builder().
+                    put(Paths.get(fakeCertificatePath), certificateChannel).
+                    put(Paths.get(fakeKeyPath), privateKeyChannel).
+                    build();
+
+
+            ConnectorFactory factory = new ConnectorFactory(new ConnectorConfig(builder), new ThrowingSecretStore());
+            ConnectorFactory.JDiscServerConnector connector = (ConnectorFactory.JDiscServerConnector) factory.createConnector(new DummyMetric(), server, serverChannel, keyStoreChannels);
+            server.addConnector(connector);
+            server.setHandler(new HelloWorldHandler());
+            server.start();
+
+            SslKeyStore trustStore = new PemSslKeyStore(
+                    new PemKeyStore.TrustStoreLoadParameter(
+                            new ReaderForPath(Files.newBufferedReader(certificateFile), certificateFile)));
+
+            SSLContext clientSslContext = SslContextFactory.newInstanceFromTrustStore(trustStore).getServerSSLContext();
+            SimpleHttpClient client = new SimpleHttpClient(clientSslContext, connector.getLocalPort(), false);
+            SimpleHttpClient.RequestExecutor ex = client.newGet("/ignored");
+            SimpleHttpClient.ResponseValidator val = ex.execute();
+            val.expectContent(equalTo("Hello world"));
+        } finally {
+            try {
+                server.stop();
+            } catch (Exception e) {
+                //ignore
+            }
+        }
+    }
+
     private static class HelloWorldHandler extends AbstractHandler {
         @Override
         public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException {
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java
index cc7095dadda..d588ace8268 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerConformanceTest.java
@@ -24,7 +24,6 @@ import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 import org.hamcrest.TypeSafeMatcher;
 import org.testng.annotations.AfterClass;
-import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
 import java.net.URI;
@@ -34,8 +33,6 @@ import java.util.Collections;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
 import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR;
@@ -52,34 +49,13 @@ import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 
 /**
- * @author Simon Thoresen Hult
+ * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a>
  */
 public class HttpServerConformanceTest extends ServerProviderConformanceTest {
 
-    private static final Logger log = Logger.getLogger(HttpServerConformanceTest.class.getName());
-
     private static final String REQUEST_CONTENT = "myRequestContent";
     private static final String RESPONSE_CONTENT = "myResponseContent";
 
-    @SuppressWarnings("LoggerInitializedWithForeignClass")
-    private static Logger httpRequestDispatchLogger = Logger.getLogger(HttpRequestDispatch.class.getName());
-    private static Level httpRequestDispatchLoggerOriginalLevel;
-
-    /*
-     * Reduce logging of every stack trace for {@link ServerProviderConformanceTest.ConformanceException} thrown.
-     * This makes the log more readable and the test faster as well.
-     */
-    @BeforeClass
-    public static void reduceExcessiveLogging() {
-        httpRequestDispatchLoggerOriginalLevel = httpRequestDispatchLogger.getLevel();
-        httpRequestDispatchLogger.setLevel(Level.SEVERE);
-    }
-
-    @AfterClass
-    public static void restoreExcessiveLogging() {
-        httpRequestDispatchLogger.setLevel(httpRequestDispatchLoggerOriginalLevel);
-    }
-
     @AfterClass
     public static void reportDiagnostics() {
         System.out.println(
@@ -808,7 +784,7 @@ public class HttpServerConformanceTest extends ServerProviderConformanceTest {
                 post.setProtocolVersion(client.requestVersion);
                 request = post;
             }
-            log.fine(() -> "executorService:"
+            System.out.println("executorService:"
                     + " .isShutDown()=" + executorService.isShutdown()
                     + " .isTerminated()=" + executorService.isTerminated());
             return executorService.submit(() -> client.delegate.execute(request));
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java
index 525cde9d8b3..8ddcd7f03ac 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDriver.java
@@ -6,8 +6,9 @@ import com.google.inject.Module;
 import com.yahoo.jdisc.application.ContainerBuilder;
 import com.yahoo.jdisc.handler.RequestHandler;
 import com.yahoo.jdisc.http.ConnectorConfig;
+import com.yahoo.jdisc.http.ssl.jks.JKSKeyStore;
 import com.yahoo.jdisc.http.SslContextFactory;
-import com.yahoo.jdisc.http.ssl.jks.JksKeyStore;
+import com.yahoo.jdisc.http.ssl.SslKeyStore;
 
 import javax.net.ssl.SSLContext;
 import java.io.IOException;
@@ -75,9 +76,8 @@ public class TestDriver {
         ConnectorConfig.Ssl sslConfig = builder.getInstance(ConnectorConfig.class).ssl();
         if (!sslConfig.enabled()) return null;
 
-        JksKeyStore keyStore = new JksKeyStore(
-                Paths.get(sslConfig.keyStorePath()),
-                builder.getInstance(Key.get(String.class, named("keyStorePassword"))));
+        SslKeyStore keyStore = new JKSKeyStore(Paths.get(sslConfig.keyStorePath()));
+        keyStore.setKeyStorePassword(builder.getInstance(Key.get(String.class, named("keyStorePassword"))));
         return SslContextFactory.newInstanceFromTrustStore(keyStore).getServerSSLContext();
     }