Validate hostnames on setup

3 files changed, 63 insertions, 37 deletions

diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
index e95044ce6d2..2faa6d86426 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/ApacheGatewayConnection.java
@@ -27,6 +27,8 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -75,7 +77,7 @@ class ApacheGatewayConnection implements GatewayConnection {
             HttpClientFactory httpClientFactory,
             String clientId) {
         SUPPORTED_VERSIONS.add(3);
-        this.endpoint = endpoint;
+        this.endpoint = validate(endpoint);
         this.feedParams = feedParams;
         this.clusterSpecificRoute = clusterSpecificRoute;
         this.httpClientFactory = httpClientFactory;
@@ -90,8 +92,17 @@ class ApacheGatewayConnection implements GatewayConnection {
             endOfFeed = END_OF_FEED_XML;
         }
         this.clientId = clientId;
-        if (this.clientId == null) {
-            throw new RuntimeException("Got no client Id.");
+        if (this.clientId == null)
+            throw new IllegalArgumentException("Got no client Id.");
+    }
+
+    private Endpoint validate(Endpoint endpoint) {
+        try {
+            InetAddress.getByName(endpoint.getHostname());
+            return endpoint;
+        }
+        catch (UnknownHostException e) {
+            throw new IllegalArgumentException("Unknown host: " + endpoint);
         }
     }
 
@@ -390,7 +401,7 @@ class ApacheGatewayConnection implements GatewayConnection {
         final ConnectionParams connectionParams;
         final boolean useSsl;
 
-        public HttpClientFactory(final ConnectionParams connectionParams, final boolean useSsl) {
+        public HttpClientFactory(ConnectionParams connectionParams, boolean useSsl) {
             this.connectionParams = connectionParams;
             this.useSsl = useSsl;
         }
@@ -426,14 +437,12 @@ class ApacheGatewayConnection implements GatewayConnection {
             clientBuilder.disableContentCompression();
             // Try to disable the disabling to see if system tests become stable again.
             // clientBuilder.disableAutomaticRetries();
-            {
-                RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
-                requestConfigBuilder.setSocketTimeout(0);
-                if (connectionParams.getProxyHost() != null) {
-                    requestConfigBuilder.setProxy(new HttpHost(connectionParams.getProxyHost(), connectionParams.getProxyPort()));
-                }
-                clientBuilder.setDefaultRequestConfig(requestConfigBuilder.build());
+            RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
+            requestConfigBuilder.setSocketTimeout(0);
+            if (connectionParams.getProxyHost() != null) {
+                requestConfigBuilder.setProxy(new HttpHost(connectionParams.getProxyHost(), connectionParams.getProxyPort()));
             }
+            clientBuilder.setDefaultRequestConfig(requestConfigBuilder.build());
 
             log.fine("Creating HttpClient: " + " ConnectionTimeout "
                             + " SocketTimeout 0 secs "
diff --git a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
index 643b7641e68..77ed8464284 100644
--- a/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
+++ b/vespa-http-client/src/main/java/com/yahoo/vespa/http/client/core/communication/IOThread.java
@@ -298,6 +298,7 @@ class IOThread implements Runnable, AutoCloseable {
         return processResponse;
     }
 
+    /** Given a current thread state, take the appropriate action and return the resulting new thread state */
     private ThreadState cycle(ThreadState threadState) {
         switch(threadState) {
             case DISCONNECTED:
diff --git a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java
index 78f616d0e36..54293495d5c 100644
--- a/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java
+++ b/vespa-http-client/src/test/java/com/yahoo/vespa/http/client/core/operationProcessor/OperationProcessorTest.java
@@ -19,6 +19,8 @@ import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
@@ -40,10 +42,10 @@ public class OperationProcessorTest {
     @Test
     public void testBasic() {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .build();
 
         OperationProcessor q = new OperationProcessor(
@@ -53,26 +55,26 @@ public class OperationProcessorTest {
 
 
         q.resultReceived(new EndpointResult("foo", new Result.Detail(null)), 0);
-        assertThat(queue.size(), is(0));
+        assertEquals(0, queue.size());
 
 
         q.sendDocument(doc1);
-        assertThat(queue.size(), is(0));
+        assertEquals(0, queue.size());
 
         q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("a"))), 0);
-        assertThat(queue.size(), is(0));
+        assertEquals(0, queue.size());
 
         q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("b"))), 1);
-        assertThat(queue.size(), is(0));
+        assertEquals(0, queue.size());
 
         q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("c"))), 2);
-        assertThat(queue.size(), is(0));
+        assertEquals(0, queue.size());
 
         q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("d"))), 3);
-        assertThat(queue.size(), is(1));
+        assertEquals(1, queue.size());
 
         q.resultReceived(new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("e"))), 0);
-        assertThat(queue.size(), is(1));
+        assertEquals(1, queue.size());
 
         //check a, b, c, d
         Result aggregated = queue.poll();
@@ -121,8 +123,8 @@ public class OperationProcessorTest {
     @Test
     public void testBlockingOfOperationsTwoEndpoints() {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .setConnectionParams(new ConnectionParams.Builder().build())
                 .build();
         OperationProcessor operationProcessor = new OperationProcessor(
@@ -159,7 +161,7 @@ public class OperationProcessorTest {
     @Test
     public void testBlockingOfOperationsToSameDocIdWithTwoOperations() {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .setConnectionParams(new ConnectionParams.Builder().build())
                 .build();
 
@@ -192,7 +194,7 @@ public class OperationProcessorTest {
     @Test
     public void testBlockingOfOperationsToSameDocIdMany() {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .setConnectionParams(new ConnectionParams.Builder().build())
                 .build();
 
@@ -225,7 +227,7 @@ public class OperationProcessorTest {
 
     @Test
     public void testMixOfBlockingAndNonBlocking() {
-        Endpoint endpoint = Endpoint.create("host");
+        Endpoint endpoint = Endpoint.create("localhost");
         SessionParams sessionParams = new SessionParams.Builder()
                 .addCluster(new Cluster.Builder().addEndpoint(endpoint).build())
                 .setConnectionParams(new ConnectionParams.Builder().build())
@@ -266,9 +268,9 @@ public class OperationProcessorTest {
     @Test
     public void assertThatDuplicateResultsFromOneClusterWorks() {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .build();
 
         OperationProcessor q = new OperationProcessor(
@@ -292,9 +294,9 @@ public class OperationProcessorTest {
     @Test
     public void testMultipleDuplicateDocIds() {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .build();
 
         OperationProcessor q = new OperationProcessor(
@@ -354,7 +356,7 @@ public class OperationProcessorTest {
     @Test
     public void testWaitBlocks() throws InterruptedException {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("host")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .build();
 
         OperationProcessor operationProcessor = new OperationProcessor(
@@ -376,17 +378,17 @@ public class OperationProcessorTest {
         started.await();
         // We want the test to pass fast so we only wait 40mS to see that it is blocking. This might lead to
         // some false positives, but that is ok.
-        assertThat(done.await(40, TimeUnit.MILLISECONDS), is(false));
+        assertFalse(done.await(40, TimeUnit.MILLISECONDS));
         operationProcessor.resultReceived(
                 new EndpointResult(doc1.getOperationId(), new Result.Detail(Endpoint.create("d"))), 0);
-        assertThat(done.await(120, TimeUnit.SECONDS), is(true));
+        assertTrue(done.await(120, TimeUnit.SECONDS));
 
     }
 
     @Test
     public void testSendsResponseToQueuedDocumentOnClose() throws InterruptedException {
         SessionParams sessionParams = new SessionParams.Builder()
-                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("#$#")).build())
+                .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
                 .build();
 
         ScheduledThreadPoolExecutor executor = mock(ScheduledThreadPoolExecutor.class);
@@ -410,4 +412,18 @@ public class OperationProcessorTest {
         operationProcessor.close();
         countDownLatch.await();
     }
+
+    @Test
+    public void testUnknownHost() {
+        try {
+            new SessionParams.Builder()
+                    .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("localhost")).build())
+                    .addCluster(new Cluster.Builder().addEndpoint(Endpoint.create("unknown")).build())
+                    .build();
+        }
+        catch (IllegalArgumentException e) {
+            assertEquals("", e.getMessage());
+        }
+    }
+
 }