consistent "connectTimeout" and "readTimeout" properties (SPR-8615)

org.springframework.web/src/main/java/org/springframework/http/client/CommonsClientHttpRequestFactory.java

@@ -90,14 +90,24 @@ public class CommonsClientHttpRequestFactory implements ClientHttpRequestFactory
 	}
 
 	/**
-	 * Set the socket read timeout for the underlying HttpClient. A value of 0 means <em>never</em> timeout.
+	 * Set the connection timeout for the underlying HttpClient.
+	 * A timeout value of 0 specifies an infinite timeout.
+	 * @param timeout the timeout value in milliseconds
+	 * @see org.apache.commons.httpclient.params.HttpConnectionManagerParams#setConnectionTimeout(int)
+	 */
+	public void setConnectTimeout(int timeout) {
+		Assert.isTrue(timeout < 0, "Timeout must be a non-negative value");
+		this.httpClient.getHttpConnectionManager().getParams().setConnectionTimeout(timeout);
+	}
+
+	/**
+	 * Set the socket read timeout for the underlying HttpClient.
+	 * A timeout value of 0 specifies an infinite timeout.
 	 * @param timeout the timeout value in milliseconds
 	 * @see org.apache.commons.httpclient.params.HttpConnectionManagerParams#setSoTimeout(int)
 	 */
 	public void setReadTimeout(int timeout) {
-		if (timeout < 0) {
+		Assert.isTrue(timeout < 0, "Timeout must be a non-negative value");
-			throw new IllegalArgumentException("timeout must be a non-negative value");
-		}
 		getHttpClient().getHttpConnectionManager().getParams().setSoTimeout(timeout);
 	}
 

org.springframework.web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java

@@ -42,7 +42,8 @@ import org.springframework.util.Assert;
 
 /**
  * {@link org.springframework.http.client.ClientHttpRequestFactory} implementation that uses
- * <a href="http://hc.apache.org/httpcomponents-client-ga/httpclient/">Http Components HttpClient</a> to create requests.
+ * <a href="http://hc.apache.org/httpcomponents-client-ga/httpclient/">Apache HttpComponents HttpClient</a>
+ * to create requests.
  *
  * <p>Allows to use a pre-configured {@link HttpClient} instance -
  * potentially with authentication, HTTP connection pooling, etc.
@@ -61,9 +62,10 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 
 	private HttpClient httpClient;
 
+
 	/**
-	 * Create a new instance of the {@code HttpComponentsClientHttpRequestFactory} with a default {@link HttpClient} that
+	 * Create a new instance of the {@code HttpComponentsClientHttpRequestFactory} with a default
-	 * uses a default {@link org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager}
+	 * {@link HttpClient} that uses a default {@link org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager}.
 	 */
 	public HttpComponentsClientHttpRequestFactory() {
 		SchemeRegistry schemeRegistry = new SchemeRegistry();
@@ -74,21 +76,22 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 		connectionManager.setMaxTotal(DEFAULT_MAX_TOTAL_CONNECTIONS);
 		connectionManager.setDefaultMaxPerRoute(DEFAULT_MAX_CONNECTIONS_PER_ROUTE);
 
-		httpClient = new DefaultHttpClient(connectionManager);
+		this.httpClient = new DefaultHttpClient(connectionManager);
-		this.setReadTimeout(DEFAULT_READ_TIMEOUT_MILLISECONDS);
+		setReadTimeout(DEFAULT_READ_TIMEOUT_MILLISECONDS);
 	}
 
+
 	/**
-	 * Create a new instance of the {@code HttpComponentsClientHttpRequestFactory} with the given {@link HttpClient}
+	 * Create a new instance of the {@code HttpComponentsClientHttpRequestFactory}
-	 * instance.
+	 * with the given {@link HttpClient} instance.
-	 *
 	 * @param httpClient the HttpClient instance to use for this factory
 	 */
 	public HttpComponentsClientHttpRequestFactory(HttpClient httpClient) {
-		Assert.notNull(httpClient, "httpClient must not be null");
+		Assert.notNull(httpClient, "HttpClient must not be null");
 		this.httpClient = httpClient;
 	}
 
+
 	/**
 	 * Set the {@code HttpClient} used by this factory.
 	 */
@@ -96,19 +99,6 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 		this.httpClient = httpClient;
 	}
 
-	/**
-	 * Set the socket read timeout for the underlying HttpClient. A value of 0 means <em>never</em> timeout.
-	 *
-	 * @param timeout the timeout value in milliseconds
-	 * @see org.apache.commons.httpclient.params.HttpConnectionManagerParams#setSoTimeout(int)
-	 */
-	public void setReadTimeout(int timeout) {
-		if (timeout < 0) {
-			throw new IllegalArgumentException("timeout must be a non-negative value");
-		}
-		getHttpClient().getParams().setIntParameter(CoreConnectionPNames.SO_TIMEOUT, timeout);
-	}
-
 	/**
 	 * Return the {@code HttpClient} used by this factory.
 	 */
@@ -116,6 +106,29 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 		return this.httpClient;
 	}
 
+	/**
+	 * Set the connection timeout for the underlying HttpClient.
+	 * A timeout value of 0 specifies an infinite timeout.
+	 * @param timeout the timeout value in milliseconds
+	 * @see org.apache.commons.httpclient.params.HttpConnectionManagerParams#setConnectionTimeout(int)
+	 */
+	public void setConnectTimeout(int timeout) {
+		Assert.isTrue(timeout < 0, "Timeout must be a non-negative value");
+		getHttpClient().getParams().setIntParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, timeout);
+	}
+
+	/**
+	 * Set the socket read timeout for the underlying HttpClient.
+	 * A timeout value of 0 specifies an infinite timeout.
+	 * @param timeout the timeout value in milliseconds
+	 * @see org.apache.commons.httpclient.params.HttpConnectionManagerParams#setSoTimeout(int)
+	 */
+	public void setReadTimeout(int timeout) {
+		Assert.isTrue(timeout < 0, "Timeout must be a non-negative value");
+		getHttpClient().getParams().setIntParameter(CoreConnectionPNames.SO_TIMEOUT, timeout);
+	}
+
+
 	public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException {
 		HttpUriRequest httpRequest = createHttpUriRequest(httpMethod, uri);
 		postProcessHttpRequest(httpRequest);
@@ -124,7 +137,6 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 
 	/**
 	 * Create a Commons HttpMethodBase object for the given HTTP method and URI specification.
-	 *
 	 * @param httpMethod the HTTP method
 	 * @param uri the URI
 	 * @return the Commons HttpMethodBase object
@@ -151,20 +163,21 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 	}
 
 	/**
-	 * Template method that allows for manipulating the {@link HttpUriRequest} before it is returned as part of a {@link
+	 * Template method that allows for manipulating the {@link HttpUriRequest} before it is
-	 * HttpComponentsClientHttpRequest}.
+	 * returned as part of a {@link HttpComponentsClientHttpRequest}.
 	 * <p>The default implementation is empty.
-	 *
 	 * @param request the request to process
 	 */
 	protected void postProcessHttpRequest(HttpUriRequest request) {
 	}
 
 	/**
-	 * Shutdown hook that closes the underlying {@link org.apache.http.conn.ClientConnectionManager
+	 * Shutdown hook that closes the underlying
-	 * ClientConnectionManager}'s connection pool, if any.
+	 * {@link org.apache.http.conn.ClientConnectionManager ClientConnectionManager}'s
+	 * connection pool, if any.
 	 */
 	public void destroy() {
 		getHttpClient().getConnectionManager().shutdown();
 	}
+
 }

org.springframework.web/src/main/java/org/springframework/remoting/httpinvoker/CommonsHttpInvokerRequestExecutor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2011 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -31,6 +31,7 @@ import org.apache.commons.httpclient.methods.PostMethod;
 import org.springframework.context.i18n.LocaleContext;
 import org.springframework.context.i18n.LocaleContextHolder;
 import org.springframework.remoting.support.RemoteInvocationResult;
+import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
 /**
@@ -66,7 +67,7 @@ public class CommonsHttpInvokerRequestExecutor extends AbstractHttpInvokerReques
 	 */
 	public CommonsHttpInvokerRequestExecutor() {
 		this.httpClient = new HttpClient(new MultiThreadedHttpConnectionManager());
-		this.setReadTimeout(DEFAULT_READ_TIMEOUT_MILLISECONDS);
+		setReadTimeout(DEFAULT_READ_TIMEOUT_MILLISECONDS);
 	}
 
 	/**
@@ -95,16 +96,25 @@ public class CommonsHttpInvokerRequestExecutor extends AbstractHttpInvokerReques
 	}
 
 	/**
-	 * Set the socket read timeout for the underlying HttpClient. A value
+	 * Set the connection timeout for the underlying HttpClient.
-	 * of 0 means <emphasis>never</emphasis> timeout. 
+	 * A timeout value of 0 specifies an infinite timeout.
+	 * @param timeout the timeout value in milliseconds
+	 * @see org.apache.commons.httpclient.params.HttpConnectionManagerParams#setConnectionTimeout(int)
+	 */
+	public void setConnectTimeout(int timeout) {
+		Assert.isTrue(timeout < 0, "Timeout must be a non-negative value");
+		this.httpClient.getHttpConnectionManager().getParams().setConnectionTimeout(timeout);
+	}
+
+	/**
+	 * Set the socket read timeout for the underlying HttpClient.
+	 * A timeout value of 0 specifies an infinite timeout.
 	 * @param timeout the timeout value in milliseconds
 	 * @see org.apache.commons.httpclient.params.HttpConnectionManagerParams#setSoTimeout(int)
 	 * @see #DEFAULT_READ_TIMEOUT_MILLISECONDS
 	 */
 	public void setReadTimeout(int timeout) {
-		if (timeout < 0) {
+		Assert.isTrue(timeout < 0, "Timeout must be a non-negative value");
-			throw new IllegalArgumentException("timeout must be a non-negative value");
-		}
 		this.httpClient.getHttpConnectionManager().getParams().setSoTimeout(timeout);
 	}
 

org.springframework.web/src/main/java/org/springframework/remoting/httpinvoker/SimpleHttpInvokerRequestExecutor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2008 the original author or authors.
+ * Copyright 2002-2011 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -45,6 +45,32 @@ import org.springframework.util.StringUtils;
  */
 public class SimpleHttpInvokerRequestExecutor extends AbstractHttpInvokerRequestExecutor {
 
+	private int connectTimeout = -1;
+
+	private int readTimeout = -1;
+
+
+	/**
+	 * Set the underlying URLConnection's connect timeout (in milliseconds).
+	 * A timeout value of 0 specifies an infinite timeout.
+	 * <p>Default is the system's default timeout.
+	 * @see URLConnection#setConnectTimeout(int)
+	 */
+	public void setConnectTimeout(int connectTimeout) {
+		this.connectTimeout = connectTimeout;
+	}
+
+	/**
+	 * Set the underlying URLConnection's read timeout (in milliseconds).
+	 * A timeout value of 0 specifies an infinite timeout.
+	 * <p>Default is the system's default timeout.
+	 * @see URLConnection#setReadTimeout(int)
+	 */
+	public void setReadTimeout(int readTimeout) {
+		this.readTimeout = readTimeout;
+	}
+
+
 	/**
 	 * Execute the given request through a standard J2SE HttpURLConnection.
 	 * <p>This method implements the basic processing workflow:
@@ -90,23 +116,29 @@ public class SimpleHttpInvokerRequestExecutor extends AbstractHttpInvokerRequest
 	 * <p>The default implementation specifies POST as method,
 	 * "application/x-java-serialized-object" as "Content-Type" header,
 	 * and the given content length as "Content-Length" header.
-	 * @param con the HTTP connection to prepare
+	 * @param connection the HTTP connection to prepare
 	 * @param contentLength the length of the content to send
 	 * @throws IOException if thrown by HttpURLConnection methods
 	 * @see java.net.HttpURLConnection#setRequestMethod
 	 * @see java.net.HttpURLConnection#setRequestProperty
 	 */
-	protected void prepareConnection(HttpURLConnection con, int contentLength) throws IOException {
+	protected void prepareConnection(HttpURLConnection connection, int contentLength) throws IOException {
-		con.setDoOutput(true);
+		if (this.connectTimeout >= 0) {
-		con.setRequestMethod(HTTP_METHOD_POST);
+			connection.setConnectTimeout(this.connectTimeout);
-		con.setRequestProperty(HTTP_HEADER_CONTENT_TYPE, getContentType());
+		}
-		con.setRequestProperty(HTTP_HEADER_CONTENT_LENGTH, Integer.toString(contentLength));
+		if (this.readTimeout >= 0) {
+			connection.setReadTimeout(this.readTimeout);
+		}
+		connection.setDoOutput(true);
+		connection.setRequestMethod(HTTP_METHOD_POST);
+		connection.setRequestProperty(HTTP_HEADER_CONTENT_TYPE, getContentType());
+		connection.setRequestProperty(HTTP_HEADER_CONTENT_LENGTH, Integer.toString(contentLength));
 		LocaleContext locale = LocaleContextHolder.getLocaleContext();
 		if (locale != null) {
-			con.setRequestProperty(HTTP_HEADER_ACCEPT_LANGUAGE, StringUtils.toLanguageTag(locale.getLocale()));
+			connection.setRequestProperty(HTTP_HEADER_ACCEPT_LANGUAGE, StringUtils.toLanguageTag(locale.getLocale()));
 		}
 		if (isAcceptGzipEncoding()) {
-			con.setRequestProperty(HTTP_HEADER_ACCEPT_ENCODING, ENCODING_GZIP);
+			connection.setRequestProperty(HTTP_HEADER_ACCEPT_ENCODING, ENCODING_GZIP);
 		}
 	}
 
@@ -187,7 +219,7 @@ public class SimpleHttpInvokerRequestExecutor extends AbstractHttpInvokerRequest
 	 */
 	protected boolean isGzipResponse(HttpURLConnection con) {
 		String encodingHeader = con.getHeaderField(HTTP_HEADER_CONTENT_ENCODING);
-		return (encodingHeader != null && encodingHeader.toLowerCase().indexOf(ENCODING_GZIP) != -1);
+		return (encodingHeader != null && encodingHeader.toLowerCase().contains(ENCODING_GZIP));
 	}
 
 }