Use httpclient5 in SimpleHttpClient

5 files changed, 98 insertions, 78 deletions

diff --git a/container-core/pom.xml b/container-core/pom.xml
index 35bb62879fb..8334dcf1f60 100644
--- a/container-core/pom.xml
+++ b/container-core/pom.xml
@@ -313,6 +313,17 @@
       <artifactId>mockito-core</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.apache.httpcomponents.client5</groupId>
+      <artifactId>httpclient5</artifactId>
+      <scope>test</scope>
+      <exclusions>
+        <exclusion>
+          <groupId>org.slf4j</groupId>
+          <artifactId>slf4j-api</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
   </dependencies>
   <build>
     <plugins>
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java
index a67656dd5ca..e117ef7f723 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java
@@ -99,7 +99,7 @@ public class FilterTestCase {
         final MyRequestHandler requestHandler = new MyRequestHandler();
         final TestDriver testDriver = newDriver(requestHandler, filterBindings);
 
-        testDriver.client().get("status.html");
+        testDriver.client().get("/status.html");
 
         assertThat(requestHandler.awaitInvocation(), is(true));
         assertThat(requestHandler.getHeaderMap().get("foo").get(0), is("bar"));
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
index 791a42c86bf..cbf807b311f 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
@@ -35,10 +35,11 @@ import com.yahoo.security.SslContextBuilder;
 import com.yahoo.security.X509CertificateBuilder;
 import com.yahoo.security.X509CertificateUtils;
 import com.yahoo.security.tls.TlsContext;
-import org.apache.http.conn.ssl.NoopHostnameVerifier;
-import org.apache.http.entity.ContentType;
-import org.apache.http.entity.mime.FormBodyPart;
-import org.apache.http.entity.mime.content.StringBody;
+import org.apache.hc.client5.http.entity.mime.FormBodyPart;
+import org.apache.hc.client5.http.entity.mime.FormBodyPartBuilder;
+import org.apache.hc.client5.http.entity.mime.StringBody;
+import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
+import org.apache.hc.core5.http.ContentType;
 import org.assertj.core.api.Assertions;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V1;
@@ -400,8 +401,8 @@ public class HttpServerTest {
         final ResponseValidator response =
                 driver.client().newPost("/status.html")
                         .setMultipartContent(
-                                newFileBody("", "start.txt", startTxtContent),
-                                newFileBody("", "updater.conf", updaterConfContent))
+                                newFileBody("start.txt", startTxtContent),
+                                newFileBody("updater.conf", updaterConfContent))
                         .execute();
         response.expectStatusCode(is(OK))
                 .expectContent(containsString(startTxtContent))
@@ -505,6 +506,16 @@ public class HttpServerTest {
     }
 
     @Test
+    public void requireThatServerCanRespondToHttp2Request() throws Exception {
+        Path privateKeyFile = tmpFolder.newFile().toPath();
+        Path certificateFile = tmpFolder.newFile().toPath();
+        generatePrivateKeyAndCertificate(privateKeyFile, certificateFile);
+
+        TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT);
+        assertTrue(driver.close());
+    }
+
+    @Test
     public void requireThatTlsClientAuthenticationEnforcerRejectsRequestsForNonWhitelistedPaths() throws IOException {
         Path privateKeyFile = tmpFolder.newFile().toPath();
         Path certificateFile = tmpFolder.newFile().toPath();
@@ -1068,30 +1079,16 @@ public class HttpServerTest {
                 new ConnectorConfig.Builder());
     }
 
-    private static FormBodyPart newFileBody(final String parameterName, final String fileName, final String fileContent) {
-        return new FormBodyPart(
-                parameterName,
-                new StringBody(fileContent, ContentType.TEXT_PLAIN) {
-                    @Override
-                    public String getFilename() {
-                        return fileName;
-                    }
-
-                    @Override
-                    public String getTransferEncoding() {
-                        return "binary";
-                    }
-
-                    @Override
-                    public String getMimeType() {
-                        return "";
-                    }
-
-                    @Override
-                    public String getCharset() {
-                        return null;
-                    }
-                });
+    private static FormBodyPart newFileBody(final String fileName, final String fileContent) {
+        return FormBodyPartBuilder.create()
+                .setBody(
+                        new StringBody(fileContent, ContentType.TEXT_PLAIN) {
+                            @Override public String getFilename() { return fileName; }
+                            @Override public String getMimeType() { return ""; }
+                            @Override public String getCharset() { return null; }
+                        })
+                .setName(fileName)
+                .build();
     }
 
     private static class ConnectedAtRequestHandler extends AbstractRequestHandler {
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java
index 230f59cbb34..23c229e2ec5 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServletTest.java
@@ -7,15 +7,15 @@ import com.yahoo.jdisc.handler.AbstractRequestHandler;
 import com.yahoo.jdisc.handler.ContentChannel;
 import com.yahoo.jdisc.handler.RequestHandler;
 import com.yahoo.jdisc.handler.ResponseHandler;
-import org.apache.http.client.methods.HttpDelete;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpHead;
-import org.apache.http.client.methods.HttpOptions;
-import org.apache.http.client.methods.HttpPatch;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.client.methods.HttpPut;
-import org.apache.http.client.methods.HttpRequestBase;
-import org.apache.http.client.methods.HttpTrace;
+import org.apache.hc.client5.http.classic.methods.HttpDelete;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.client5.http.classic.methods.HttpHead;
+import org.apache.hc.client5.http.classic.methods.HttpOptions;
+import org.apache.hc.client5.http.classic.methods.HttpPatch;
+import org.apache.hc.client5.http.classic.methods.HttpPost;
+import org.apache.hc.client5.http.classic.methods.HttpPut;
+import org.apache.hc.client5.http.classic.methods.HttpTrace;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
 import org.junit.Test;
 
 import java.io.IOException;
@@ -73,8 +73,7 @@ public class JDiscHttpServletTest {
         };
     }
 
-    private static class UnknownMethodHttpRequest extends HttpRequestBase {
-        UnknownMethodHttpRequest(URI uri) { setURI(uri); }
-        @Override public String getMethod() { return "UNKNOWN_METHOD"; }
+    private static class UnknownMethodHttpRequest extends HttpUriRequestBase {
+        UnknownMethodHttpRequest(URI uri) { super("UNKNOWN_METHOD", uri); }
     }
 }
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java
index eea8d7e3072..161f48d847d 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java
@@ -1,33 +1,36 @@
 // 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 org.apache.http.Header;
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.entity.GzipCompressingEntity;
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.config.Registry;
-import org.apache.http.config.RegistryBuilder;
-import org.apache.http.conn.socket.ConnectionSocketFactory;
-import org.apache.http.conn.ssl.DefaultHostnameVerifier;
-import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
-import org.apache.http.entity.ByteArrayEntity;
-import org.apache.http.entity.StringEntity;
-import org.apache.http.entity.mime.FormBodyPart;
-import org.apache.http.entity.mime.MultipartEntityBuilder;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClientBuilder;
-import org.apache.http.impl.conn.BasicHttpClientConnectionManager;
-import org.apache.http.util.EntityUtils;
+import org.apache.hc.client5.http.SystemDefaultDnsResolver;
+import org.apache.hc.client5.http.classic.methods.HttpGet;
+import org.apache.hc.client5.http.classic.methods.HttpPost;
+import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
+import org.apache.hc.client5.http.entity.GzipCompressingEntity;
+import org.apache.hc.client5.http.entity.mime.FormBodyPart;
+import org.apache.hc.client5.http.entity.mime.MultipartEntityBuilder;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
+import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
+import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
+import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
+import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
+import org.apache.hc.client5.http.ssl.DefaultHostnameVerifier;
+import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
+import org.apache.hc.core5.http.ContentType;
+import org.apache.hc.core5.http.Header;
+import org.apache.hc.core5.http.HttpEntity;
+import org.apache.hc.core5.http.HttpResponse;
+import org.apache.hc.core5.http.ParseException;
+import org.apache.hc.core5.http.io.entity.ByteArrayEntity;
+import org.apache.hc.core5.http.io.entity.EntityUtils;
+import org.apache.hc.core5.http.io.entity.StringEntity;
 import org.hamcrest.Matcher;
 import org.hamcrest.MatcherAssert;
 
 import javax.net.ssl.SSLContext;
 import java.io.IOException;
+import java.net.InetAddress;
 import java.net.URI;
+import java.net.UnknownHostException;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.List;
@@ -55,8 +58,9 @@ public class SimpleHttpClient implements AutoCloseable {
 
     public SimpleHttpClient(SSLContext sslContext, List<String> enabledProtocols, List<String> enabledCiphers,
                             int listenPort, boolean useCompression) {
-        HttpClientBuilder builder = HttpClientBuilder.create();
-        builder.disableConnectionState(); // Reuse SSL connection when client authentication is enabled
+        HttpClientBuilder builder = HttpClientBuilder.create()
+                .disableAutomaticRetries()
+                .disableConnectionState(); // Reuse SSL connection when client authentication is enabled
         if (!useCompression) {
             builder.disableContentCompression();
         }
@@ -66,12 +70,17 @@ public class SimpleHttpClient implements AutoCloseable {
                     toArray(enabledProtocols),
                     toArray(enabledCiphers),
                     new DefaultHostnameVerifier());
-            builder.setSSLSocketFactory(sslConnectionFactory);
-
-            Registry<ConnectionSocketFactory> registry = RegistryBuilder.<ConnectionSocketFactory>create()
-                    .register("https", sslConnectionFactory)
+            PoolingHttpClientConnectionManager connManager = PoolingHttpClientConnectionManagerBuilder.create()
+                    .setSSLSocketFactory(sslConnectionFactory)
+                    .setDnsResolver(new SystemDefaultDnsResolver() {
+                        @Override
+                        public InetAddress[] resolve(String host) throws UnknownHostException {
+                            // Returns single address instead of multiple (to avoid multiple connection attempts)
+                            return new InetAddress[] { InetAddress.getByName(host) };
+                        }
+                    })
                     .build();
-            builder.setConnectionManager(new BasicHttpClientConnectionManager(registry));
+            builder.setConnectionManager(connManager);
             scheme = "https";
         } else {
             scheme = "http";
@@ -139,7 +148,7 @@ public class SimpleHttpClient implements AutoCloseable {
         }
 
         public RequestExecutor setBinaryContent(final byte[] content) {
-            this.entity = new ByteArrayEntity(content);
+            this.entity = new ByteArrayEntity(content, ContentType.DEFAULT_BINARY);
             return this;
         }
 
@@ -152,7 +161,7 @@ public class SimpleHttpClient implements AutoCloseable {
 
         public ResponseValidator execute() throws IOException {
             if (entity != null) {
-                ((HttpPost)request).setEntity(entity);
+                request.setEntity(entity);
             }
             try (CloseableHttpResponse response = delegate.execute(request)){
                 return new ResponseValidator(response);
@@ -165,15 +174,19 @@ public class SimpleHttpClient implements AutoCloseable {
         private final HttpResponse response;
         private final String content;
 
-        public ResponseValidator(HttpResponse response) throws IOException {
-            this.response = response;
+        public ResponseValidator(CloseableHttpResponse response) throws IOException {
+            try {
+                this.response = response;
 
-            HttpEntity entity = response.getEntity();
-            this.content = entity == null ? null : EntityUtils.toString(entity, StandardCharsets.UTF_8);
+                HttpEntity entity = response.getEntity();
+                this.content = entity == null ? null : EntityUtils.toString(entity, StandardCharsets.UTF_8);
+            } catch (ParseException e) {
+                throw new IOException(e);
+            }
         }
 
         public ResponseValidator expectStatusCode(Matcher<Integer> matcher) {
-            MatcherAssert.assertThat(response.getStatusLine().getStatusCode(), matcher);
+            MatcherAssert.assertThat(response.getCode(), matcher);
             return this;
         }