Do not reuse a HttpClient, when using a socket factory,

it leads to mixing up of connections and not changing host
cloudfoundry · Aug 11, 2015 · 0eaec93 · 0eaec93
1 parent e7888d8
commit 0eaec93
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/.../src/main/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderConfigurator.java b/.../src/main/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderConfigurator.java
@@ -13,6 +13,8 @@
 package org.cloudfoundry.identity.uaa.login.saml;
 
 import org.apache.commons.httpclient.HttpClient;
+import org.apache.commons.httpclient.SimpleHttpConnectionManager;
+import org.apache.commons.httpclient.params.HttpClientParams;
 import org.apache.commons.httpclient.protocol.ProtocolSocketFactory;
 import org.apache.http.client.utils.URIBuilder;
 import org.cloudfoundry.identity.uaa.login.util.FileLocator;
@@ -46,7 +48,7 @@ public class IdentityProviderConfigurator implements InitializingBean {
     private boolean legacyShowSamlLink = true;
     private List<IdentityProviderDefinition> identityProviders = new LinkedList<>();
     private Timer metadataFetchingHttpClientTimer;
-    private HttpClient httpClient;
+    private HttpClientParams clientParams;
     private BasicParserPool parserPool;
 
     public List<IdentityProviderDefinition> getIdentityProviderDefinitions() {
@@ -230,7 +232,10 @@ protected ExtendedMetadataDelegate configureURLMetadata(IdentityProviderDefiniti
             socketFactory = (Class<ProtocolSocketFactory>) Class.forName(def.getSocketFactoryClassName());
             ExtendedMetadata extendedMetadata = new ExtendedMetadata();
             extendedMetadata.setAlias(def.getIdpEntityAlias());
-            FixedHttpMetaDataProvider fixedHttpMetaDataProvider = new FixedHttpMetaDataProvider(def.getZoneId(), def.getIdpEntityAlias(), getMetadataFetchingHttpClientTimer(), getHttpClient(), adjustURIForPort(def.getMetaDataLocation()));
+            SimpleHttpConnectionManager connectionManager = new SimpleHttpConnectionManager(true);
+            connectionManager.getParams().setDefaults(getClientParams());
+            HttpClient client = new HttpClient(connectionManager);
+            FixedHttpMetaDataProvider fixedHttpMetaDataProvider = new FixedHttpMetaDataProvider(def.getZoneId(), def.getIdpEntityAlias(), getMetadataFetchingHttpClientTimer(), client, adjustURIForPort(def.getMetaDataLocation()));
             fixedHttpMetaDataProvider.setParserPool(getParserPool());
             //TODO - we have no way of actually instantiating this object unless it has a zero arg constructor
             fixedHttpMetaDataProvider.setSocketFactory(socketFactory.newInstance());
@@ -358,12 +363,12 @@ public void setMetadataFetchingHttpClientTimer(Timer metadataFetchingHttpClientT
         this.metadataFetchingHttpClientTimer = metadataFetchingHttpClientTimer;
     }
 
-    public HttpClient getHttpClient() {
+    public HttpClientParams getClientParams() {
-        return httpClient;
+        return clientParams;
     }
 
-    public void setHttpClient(HttpClient httpClient) {
+    public void setClientParams(HttpClientParams clientParams) {
-        this.httpClient = httpClient;
+        this.clientParams = clientParams;
     }
 
     public BasicParserPool getParserPool() {

diff --git a/...test/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderConfiguratorTests.java b/...test/java/org/cloudfoundry/identity/uaa/login/saml/IdentityProviderConfiguratorTests.java
@@ -13,6 +13,7 @@
 package org.cloudfoundry.identity.uaa.login.saml;
 
 import org.apache.commons.httpclient.HttpClient;
+import org.apache.commons.httpclient.params.HttpClientParams;
 import org.cloudfoundry.identity.uaa.client.ClientConstants;
 import org.cloudfoundry.identity.uaa.config.YamlMapFactoryBean;
 import org.cloudfoundry.identity.uaa.config.YamlProcessor;
@@ -346,7 +347,7 @@ public void testGetIdentityProviders() throws Exception {
         conf.setLegacyIdpIdentityAlias("vsphere.local.legacy");
         conf.setLegacyNameId("urn:oasis:names:tc:SAML:2.0:nameid-format:persistent");
         conf.setMetadataFetchingHttpClientTimer(new Timer());
-        conf.setHttpClient(new HttpClient());
+        conf.setClientParams(new HttpClientParams());
         testGetIdentityProviderDefinitions(6);
         conf.getIdentityProviders();
     }

diff --git a/uaa/src/main/webapp/WEB-INF/spring/saml-providers.xml b/uaa/src/main/webapp/WEB-INF/spring/saml-providers.xml
@@ -121,10 +121,6 @@
             <constructor-arg value="true" />
         </bean>
 
-        <bean name="metadataFetchingHttpClient" class="org.apache.commons.httpclient.HttpClient">
-            <constructor-arg ref="httpClientParams" />
-        </bean>
-
         <bean name="httpClientParams" class="org.apache.commons.httpclient.params.HttpClientParams">
             <property name="connectionManagerTimeout" value="${login.saml.socket.connectionManagerTimeout:10000}" />
             <property name="soTimeout" value="${login.saml.socket.soTimeout:10000}" />
@@ -269,7 +265,7 @@
             <property name="legacyMetadataTrustCheck" value="${login.saml.metadataTrustCheck:true}"/>
             <property name="legacyShowSamlLink" value="${login.showSamlLoginLink:true}"/>
             <property name="metadataFetchingHttpClientTimer" ref="metadataFetchingHttpClientTimer" />
-            <property name="httpClient" ref="metadataFetchingHttpClient" />
+            <property name="clientParams" ref="httpClientParams" />
             <property name="parserPool" ref="parserPool"/>
         </bean>