Updated to the latest version of httpclient (4.5.1)

To safely rework the code to remove the deprecation warnings, I added unit tests which leverage the WireMock HTTP server mocking framework.  By doing this, I was able to capture the functionality from how the old httpclient APIs were being used, and duplicate that functionality using the 4.5.1 APIs.

By having these unit tests, we can keep up with changes to httpclient in the future, and be confident the existing functionality still works.

pom.xml

@@ -62,7 +62,7 @@
             <properties>
                 <maven.javadoc.skip>true</maven.javadoc.skip>
                 <source.skip>true</source.skip>
-				<jspc.skip>true</jspc.skip>
+                <jspc.skip>true</jspc.skip>
             </properties>
         </profile>
     </profiles>
@@ -210,6 +210,19 @@
             <version>1.10.19</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.assertj</groupId>
+            <artifactId>assertj-core</artifactId>
+            <!-- use 3.3.0 for Java 8 projects -->
+            <version>2.3.0</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>com.github.tomakehurst</groupId>
+            <artifactId>wiremock</artifactId>
+            <version>1.58</version>
+            <scope>test</scope>
+        </dependency>
 
         <!-- container dependencies -->
         <dependency>
@@ -282,7 +295,7 @@
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpclient</artifactId>
-            <version>4.2.2</version>
+            <version>4.5.1</version>
         </dependency>
         <dependency>
             <groupId>com.googlecode.concurrentlinkedhashmap</groupId>

src/main/java/password/pwm/http/client/PwmHttpClient.java

@@ -22,24 +22,49 @@
 
 package password.pwm.http.client;
 
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.security.KeyManagementException;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.util.Date;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+
 import org.apache.http.Header;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
 import org.apache.http.auth.AuthScope;
 import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
 import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.*;
-import org.apache.http.conn.ClientConnectionManager;
-import org.apache.http.conn.params.ConnRoutePNames;
-import org.apache.http.conn.scheme.Scheme;
-import org.apache.http.conn.scheme.SchemeRegistry;
-import org.apache.http.conn.ssl.SSLSocketFactory;
-import org.apache.http.conn.ssl.X509HostnameVerifier;
+import org.apache.http.client.methods.HttpDelete;
+import org.apache.http.client.methods.HttpGet;
+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.config.Registry;
+import org.apache.http.config.RegistryBuilder;
+import org.apache.http.conn.HttpClientConnectionManager;
+import org.apache.http.conn.socket.ConnectionSocketFactory;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.entity.StringEntity;
-import org.apache.http.impl.client.DefaultHttpClient;
-import org.apache.http.impl.conn.SingleClientConnManager;
-import org.apache.http.params.HttpProtocolParams;
+import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.client.ProxyAuthenticationStrategy;
+import org.apache.http.impl.conn.BasicHttpClientConnectionManager;
+import org.apache.http.ssl.SSLContextBuilder;
+import org.apache.http.ssl.TrustStrategy;
 import org.apache.http.util.EntityUtils;
+
 import password.pwm.AppProperty;
 import password.pwm.PwmApplication;
 import password.pwm.PwmConstants;
@@ -53,19 +78,6 @@ import password.pwm.util.TimeDuration;
 import password.pwm.util.X509Utils;
 import password.pwm.util.logging.PwmLogger;
 
-import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.SSLContext;
-import javax.net.ssl.TrustManager;
-import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
-import java.security.KeyManagementException;
-import java.security.NoSuchAlgorithmException;
-import java.security.SecureRandom;
-import java.util.Date;
-import java.util.LinkedHashMap;
-import java.util.Map;
-
 public class PwmHttpClient {
     private static final PwmLogger LOGGER = PwmLogger.forClass(PwmHttpClient.class);
 
@@ -94,39 +106,54 @@ public class PwmHttpClient {
     }
 
     public static HttpClient getHttpClient(final Configuration configuration, final PwmHttpClientConfiguration pwmHttpClientConfiguration)
-            throws PwmUnrecoverableException
+    throws PwmUnrecoverableException
     {
-        final DefaultHttpClient httpClient;
+        HttpClientBuilder clientBuilder = HttpClientBuilder.create();
+        clientBuilder.setUserAgent(PwmConstants.PWM_APP_NAME + " " + PwmConstants.SERVLET_VERSION);
+
         try {
             if (Boolean.parseBoolean(configuration.readAppProperty(AppProperty.SECURITY_HTTP_PROMISCUOUS_ENABLE))) {
-                httpClient = new DefaultHttpClient(makeConnectionManager(new X509Utils.PromiscuousTrustManager()));
+                clientBuilder.setSSLContext(promiscuousSSLContext());
+                clientBuilder.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE);
             } else if (pwmHttpClientConfiguration != null && pwmHttpClientConfiguration.getCertificates() != null) {
-                final TrustManager trustManager = new X509Utils.CertMatchingTrustManager(configuration,pwmHttpClientConfiguration.getCertificates());
-                httpClient = new DefaultHttpClient(makeConnectionManager(trustManager));
-            } else {
-                httpClient = new DefaultHttpClient();
+                SSLContext sslContext = SSLContext.getInstance("SSL");
+                TrustManager trustManager = new X509Utils.CertMatchingTrustManager(configuration, pwmHttpClientConfiguration.getCertificates());
+                sslContext.init(null, new TrustManager[]{ trustManager }, new SecureRandom());
+
+                SSLConnectionSocketFactory sslConnectionFactory = new SSLConnectionSocketFactory(sslContext, NoopHostnameVerifier.INSTANCE);
+                Registry<ConnectionSocketFactory> registry = RegistryBuilder.<ConnectionSocketFactory>create().register("https", sslConnectionFactory).build();
+                HttpClientConnectionManager ccm = new BasicHttpClientConnectionManager(registry);
+
+                clientBuilder.setSSLSocketFactory(sslConnectionFactory);
+                clientBuilder.setConnectionManager(ccm);
             }
         } catch (Exception e) {
             throw new PwmUnrecoverableException(new ErrorInformation(PwmError.ERROR_UNKNOWN,"unexpected error creating promiscuous https client: " + e.getMessage()));
         }
-        final String strValue = configuration.readSettingAsString(PwmSetting.HTTP_PROXY_URL);
-        if (strValue != null && strValue.length() > 0) {
-            final URI proxyURI = URI.create(strValue);
-
-            final String host = proxyURI.getHost();
-            final int port = proxyURI.getPort();
-            final HttpHost proxy = new HttpHost(host, port);
-            httpClient.getParams().setParameter(ConnRoutePNames.DEFAULT_PROXY, proxy);
-
-            final String username = proxyURI.getUserInfo();
-            if (username != null && username.length() > 0) {
-                final String password = (username.contains(":")) ? username.split(":")[1] : "";
-                final UsernamePasswordCredentials passwordCredentials = new UsernamePasswordCredentials(username, password);
-                httpClient.getCredentialsProvider().setCredentials(new AuthScope(host, port), passwordCredentials);
+
+        final String proxyUrl = configuration.readSettingAsString(PwmSetting.HTTP_PROXY_URL);
+        if (proxyUrl != null && proxyUrl.length() > 0) {
+            URI proxyURI = URI.create(proxyUrl);
+            String host = proxyURI.getHost();
+            int port = proxyURI.getPort();
+
+            clientBuilder.setProxy(new HttpHost(host, port));
+
+            final String userInfo = proxyURI.getUserInfo();
+            if (userInfo != null && userInfo.length() > 0) {
+                String[] parts = userInfo.split(":");
+
+                String username = parts[0];
+                String password = (parts.length > 1) ? parts[1] : "";
+
+                CredentialsProvider credsProvider = new BasicCredentialsProvider();
+                credsProvider.setCredentials(new AuthScope(host, port), new UsernamePasswordCredentials(username, password));
+                clientBuilder.setDefaultCredentialsProvider(credsProvider);
+                clientBuilder.setProxyAuthenticationStrategy(new ProxyAuthenticationStrategy());
             }
         }
-        final String userAgent = PwmConstants.PWM_APP_NAME + " " + PwmConstants.SERVLET_VERSION;
-        httpClient.getParams().setParameter(HttpProtocolParams.USER_AGENT, userAgent);
+
+        HttpClient httpClient = clientBuilder.build();
         return httpClient;
     }
 
@@ -225,22 +252,12 @@ public class PwmHttpClient {
         return httpClientResponse;
     }
 
-    private static ClientConnectionManager makeConnectionManager(TrustManager trustManager)
-            throws NoSuchAlgorithmException, KeyManagementException
-    {
-        final SSLContext sslContext = SSLContext.getInstance("SSL");
-
-        sslContext.init(null, new TrustManager[]{trustManager}, new SecureRandom());
-
-        final SSLSocketFactory sf = new SSLSocketFactory(sslContext);
-        final HostnameVerifier hostnameVerifier = org.apache.http.conn.ssl.SSLSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER;
-
-        sf.setHostnameVerifier((X509HostnameVerifier) hostnameVerifier);
-        final Scheme httpsScheme = new Scheme("https", 443, sf);
-        final SchemeRegistry schemeRegistry = new SchemeRegistry();
-        schemeRegistry.register(httpsScheme);
-
-        return new SingleClientConnManager(schemeRegistry);
+    protected static SSLContext promiscuousSSLContext() throws NoSuchAlgorithmException, KeyManagementException, KeyStoreException {
+        return new SSLContextBuilder().loadTrustMaterial(null, new TrustStrategy() {
+            public boolean isTrusted(X509Certificate[] arg0, String arg1) throws CertificateException {
+                return true;
+            }
+        }).build();
     }
 }
 

src/test/java/password/pwm/http/client/PwmHttpClientTest.java

@@ -0,0 +1,183 @@
+package password.pwm.http.client;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.*;
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.*;
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import java.io.InputStream;
+import java.security.KeyStore;
+import java.security.cert.X509Certificate;
+
+import javax.net.ssl.SSLHandshakeException;
+
+import org.apache.commons.io.IOUtils;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.junit.Rule;
+import org.junit.Test;
+
+import password.pwm.AppProperty;
+import password.pwm.PwmConstants;
+import password.pwm.config.Configuration;
+import password.pwm.config.PwmSetting;
+
+import com.github.tomakehurst.wiremock.client.WireMock;
+import com.github.tomakehurst.wiremock.junit.WireMockRule;
+
+public class PwmHttpClientTest {
+    @Rule public WireMockRule wm = new WireMockRule(wireMockConfig()
+        .dynamicPort()
+        .dynamicHttpsPort());
+
+    // Create a few mock objects, in case they're needed by the tests
+    private Configuration configuration = mock(Configuration.class);
+    private PwmHttpClientConfiguration pwmHttpClientConfiguration = mock(PwmHttpClientConfiguration.class);
+
+    /**
+     * Test making a simple HTTP request from the client returned by PwmHttpClient.getHttpClient(...)
+     */
+    @Test
+    public void testGetHttpClient_simpleHello() throws Exception {
+        // Stub out our local HTTP server
+        wm.stubFor(get(urlEqualTo("/simpleHello"))
+            .willReturn(aResponse()
+                .withHeader("Content-Type", "text/plain")
+                .withBody("Hello from the local mock server")));
+
+        // Obtain the HTTP client from PWM
+        HttpClient httpClient = PwmHttpClient.getHttpClient(configuration, pwmHttpClientConfiguration);
+
+        // Execute the HTTP request
+        HttpGet httpGet = new HttpGet(String.format("http://localhost:%d/simpleHello", wm.port()));
+        HttpResponse response = httpClient.execute(httpGet);
+
+        // Verify the response
+        int responseStatusCode = response.getStatusLine().getStatusCode();
+        assertThat(responseStatusCode).isEqualTo(200);
+
+        String responseContent = IOUtils.toString(response.getEntity().getContent());
+        assertThat(responseContent).startsWith("Hello");
+
+        // Verify the HTTP server got called as expected
+        wm.verify(getRequestedFor(urlEqualTo("/simpleHello"))
+            .withHeader("User-Agent", equalTo(PwmConstants.PWM_APP_NAME + " " + PwmConstants.SERVLET_VERSION)));
+    }
+
+    /**
+     * Test making an SSL request without setting SECURITY_HTTP_PROMISCUOUS_ENABLE to true, or supplying any certificates
+     */
+    @Test(expected = SSLHandshakeException.class)
+    public void testGetHttpClient_sslHelloFail() throws Exception {
+        // Stub out our local HTTP server
+        wm.stubFor(get(urlEqualTo("/simpleHello"))
+            .willReturn(aResponse()
+                .withHeader("Content-Type", "text/plain")
+                .withBody("Hello from the local mock server")));
+
+        HttpClient httpClient = PwmHttpClient.getHttpClient(configuration, pwmHttpClientConfiguration);
+
+        HttpGet httpGet = new HttpGet(String.format("https://localhost:%d/simpleHello", wm.httpsPort()));
+
+        // This should throw an exception, since we're doing https without setting SECURITY_HTTP_PROMISCUOUS_ENABLE, or setting certificates
+        httpClient.execute(httpGet);
+    }
+
+    /**
+     * Test making an SSL request in promiscuous mode (no certificates needed)
+     */
+    @Test
+    public void testGetHttpClient_sslHello() throws Exception {
+        // Stub out our local HTTP server
+        wm.stubFor(get(urlEqualTo("/simpleHello"))
+            .willReturn(aResponse()
+                .withHeader("Content-Type", "text/plain")
+                .withBody("Hello from the local mock server")));
+
+        // Stub out some mock object behavior
+        when(configuration.readAppProperty(AppProperty.SECURITY_HTTP_PROMISCUOUS_ENABLE)).thenReturn("true");
+
+        HttpClient httpClient = PwmHttpClient.getHttpClient(configuration, pwmHttpClientConfiguration);
+
+        HttpGet httpGet = new HttpGet(String.format("https://localhost:%d/simpleHello", wm.httpsPort()));
+        HttpResponse response = httpClient.execute(httpGet);
+
+        // Verify the response
+        int responseStatusCode = response.getStatusLine().getStatusCode();
+        assertThat(responseStatusCode).isEqualTo(200);
+
+        String responseContent = IOUtils.toString(response.getEntity().getContent());
+        assertThat(responseContent).startsWith("Hello");
+    }
+
+    /**
+     * Test making an SSL request using the server's certificate
+     */
+    @Test
+    public void testGetHttpClient_sslWithCertificates() throws Exception {
+     // Stub out our local HTTP server
+        wm.stubFor(get(urlEqualTo("/simpleHello"))
+            .willReturn(aResponse()
+                .withHeader("Content-Type", "text/plain")
+                .withBody("Hello from the local mock server")));
+
+        // Stub out some mock object behavior
+        X509Certificate[] certificates = new X509Certificate[] { getWireMockSelfSignedCertificate() };
+        when(pwmHttpClientConfiguration.getCertificates()).thenReturn(certificates);
+
+        HttpClient httpClient = PwmHttpClient.getHttpClient(configuration, pwmHttpClientConfiguration);
+
+        HttpGet httpGet = new HttpGet(String.format("https://localhost:%d/simpleHello", wm.httpsPort()));
+        HttpResponse response = httpClient.execute(httpGet);
+
+        // Verify the response
+        int responseStatusCode = response.getStatusLine().getStatusCode();
+        assertThat(responseStatusCode).isEqualTo(200);
+
+        String responseContent = IOUtils.toString(response.getEntity().getContent());
+        assertThat(responseContent).startsWith("Hello");
+    }
+
+    /**
+     * Test making a request through a proxy
+     */
+    @Test
+    public void testGetHttpClient_proxyHello() throws Exception {
+        // Stub out our local HTTP server
+        wm.stubFor(get(urlEqualTo("/simpleHello"))
+            .willReturn(aResponse()
+                .withHeader("Content-Type", "text/plain")
+                .withBody("Hello from the local mock server")));
+
+        // Stub out some mock object behavior
+        when(configuration.readSettingAsString(PwmSetting.HTTP_PROXY_URL)).thenReturn(String.format("http://localhost:%d/simpleHello", wm.port()));
+
+        HttpClient httpClient = PwmHttpClient.getHttpClient(configuration, pwmHttpClientConfiguration);
+
+        // We are making a request to www.microfocus.com, but our server on localhost will receive it
+        HttpGet httpGet = new HttpGet(String.format("http://www.microfocus.com/simpleHello"));
+        HttpResponse response = httpClient.execute(httpGet);
+
+        // Verify the response
+        int responseStatusCode = response.getStatusLine().getStatusCode();
+        assertThat(responseStatusCode).isEqualTo(200);
+
+        String responseContent = IOUtils.toString(response.getEntity().getContent());
+        assertThat(responseContent).startsWith("Hello");
+    }
+
+    private X509Certificate getWireMockSelfSignedCertificate() {
+        InputStream keystoreInputStream = WireMock.class.getResourceAsStream("/keystore");
+
+        try {
+            KeyStore keyStore  = KeyStore.getInstance(KeyStore.getDefaultType());
+            keyStore.load(keystoreInputStream, "password".toCharArray());
+            return (X509Certificate) keyStore.getCertificate("wiremock");
+        } catch (Exception e) {
+            fail("Unable to load wiremock self-signed certificate", e);
+        }
+
+        return null;
+    }
+}