From d04b7baa642e7192bddb8e0ad26c7504c6908c85 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Tue, 4 Jun 2013 13:27:02 +0000 Subject: [PATCH] HTTPCLIENT-1362: better error messages for connect timed out and connection refused exceptions (second cut) git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1489438 13f79535-47bb-0310-9956-ffa450edef68 --- .../http/conn/ConnectTimeoutException.java | 19 +--- .../http/conn/HttpHostConnectException.java | 24 ++-- .../AbstractConnectionSocketFactory.java | 104 ------------------ .../conn/socket/ConnectionSocketFactory.java | 0 .../http/conn/socket/PlainSocketFactory.java | 27 ++++- .../http/conn/ssl/SSLSocketFactory.java | 20 +++- .../conn/HttpClientConnectionOperator.java | 17 ++- .../org/apache/http/conn/TestExceptions.java | 25 ++--- .../TestHttpClientConnectionOperator.java | 29 ++++- 9 files changed, 108 insertions(+), 157 deletions(-) mode change 100755 => 100644 httpclient/src/main/java/org/apache/http/conn/ConnectTimeoutException.java mode change 100755 => 100644 httpclient/src/main/java/org/apache/http/conn/HttpHostConnectException.java delete mode 100755 httpclient/src/main/java/org/apache/http/conn/socket/AbstractConnectionSocketFactory.java mode change 100755 => 100644 httpclient/src/main/java/org/apache/http/conn/socket/ConnectionSocketFactory.java mode change 100755 => 100644 httpclient/src/main/java/org/apache/http/conn/socket/PlainSocketFactory.java mode change 100755 => 100644 httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java diff --git a/httpclient/src/main/java/org/apache/http/conn/ConnectTimeoutException.java b/httpclient/src/main/java/org/apache/http/conn/ConnectTimeoutException.java old mode 100755 new mode 100644 index d1d145ed4..6eba40273 --- a/httpclient/src/main/java/org/apache/http/conn/ConnectTimeoutException.java +++ b/httpclient/src/main/java/org/apache/http/conn/ConnectTimeoutException.java @@ -29,7 +29,9 @@ package org.apache.http.conn; import java.io.IOException; import java.io.InterruptedIOException; +import java.net.InetAddress; import java.net.InetSocketAddress; +import java.util.Arrays; import org.apache.http.HttpHost; import org.apache.http.annotation.Immutable; @@ -47,7 +49,6 @@ public class ConnectTimeoutException extends InterruptedIOException { private static final long serialVersionUID = -4816682903149535989L; private final HttpHost host; - private final InetSocketAddress remoteAddress; /** * Creates a ConnectTimeoutException with a null detail message. @@ -55,7 +56,6 @@ public class ConnectTimeoutException extends InterruptedIOException { public ConnectTimeoutException() { super(); this.host = null; - this.remoteAddress = null; } /** @@ -64,7 +64,6 @@ public class ConnectTimeoutException extends InterruptedIOException { public ConnectTimeoutException(final String message) { super(message); this.host = null; - this.remoteAddress = null; } /** @@ -73,15 +72,14 @@ public class ConnectTimeoutException extends InterruptedIOException { * @since 4.3 */ public ConnectTimeoutException( + final IOException cause, final HttpHost host, - final InetSocketAddress remoteAddress, - final IOException cause) { + final InetAddress... remoteAddresses) { super("Connect to " + (host != null ? host.toHostString() : "remote host") + - (remoteAddress != null ? " (" + remoteAddress.getAddress() + ")" : "") + (remoteAddresses != null ? " " + Arrays.asList(remoteAddresses) : "") + ((cause != null && cause.getMessage() != null) ? " failed: " + cause.getMessage() : " timed out")); this.host = host; - this.remoteAddress = remoteAddress; initCause(cause); } @@ -92,11 +90,4 @@ public class ConnectTimeoutException extends InterruptedIOException { return host; } - /** - * @since 4.3 - */ - public InetSocketAddress getRemoteAddress() { - return remoteAddress; - } - } diff --git a/httpclient/src/main/java/org/apache/http/conn/HttpHostConnectException.java b/httpclient/src/main/java/org/apache/http/conn/HttpHostConnectException.java old mode 100755 new mode 100644 index 96cd11c9c..7f536d090 --- a/httpclient/src/main/java/org/apache/http/conn/HttpHostConnectException.java +++ b/httpclient/src/main/java/org/apache/http/conn/HttpHostConnectException.java @@ -28,7 +28,9 @@ package org.apache.http.conn; import java.io.IOException; import java.net.ConnectException; +import java.net.InetAddress; import java.net.InetSocketAddress; +import java.util.Arrays; import org.apache.http.HttpHost; import org.apache.http.annotation.Immutable; @@ -45,10 +47,14 @@ public class HttpHostConnectException extends ConnectException { private static final long serialVersionUID = -3194482710275220224L; private final HttpHost host; - private final InetSocketAddress remoteAddress; + /** + * @deprecated (4.3) use {@link #HttpHostConnectException(java.io.IOException, org.apache.http.HttpHost, + * java.net.InetAddress...)} + */ + @Deprecated public HttpHostConnectException(final HttpHost host, final ConnectException cause) { - this(host, null, cause); + this(cause, host, null); } /** @@ -57,15 +63,14 @@ public class HttpHostConnectException extends ConnectException { * @since 4.3 */ public HttpHostConnectException( + final IOException cause, final HttpHost host, - final InetSocketAddress remoteAddress, - final IOException cause) { + final InetAddress... remoteAddresses) { super("Connect to " + (host != null ? host.toHostString() : "remote host") + - (remoteAddress != null ? " (" + remoteAddress.getAddress() + ")" : "") + (remoteAddresses != null ? " " + Arrays.asList(remoteAddresses) : "") + ((cause != null && cause.getMessage() != null) ? " failed: " + cause.getMessage() : " refused")); this.host = host; - this.remoteAddress = remoteAddress; initCause(cause); } @@ -73,11 +78,4 @@ public class HttpHostConnectException extends ConnectException { return this.host; } - /** - * @since 4.3 - */ - public InetSocketAddress getRemoteAddress() { - return remoteAddress; - } - } diff --git a/httpclient/src/main/java/org/apache/http/conn/socket/AbstractConnectionSocketFactory.java b/httpclient/src/main/java/org/apache/http/conn/socket/AbstractConnectionSocketFactory.java deleted file mode 100755 index fbc9a840e..000000000 --- a/httpclient/src/main/java/org/apache/http/conn/socket/AbstractConnectionSocketFactory.java +++ /dev/null @@ -1,104 +0,0 @@ -/* - * ==================================================================== - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * ==================================================================== - * - * This software consists of voluntary contributions made by many - * individuals on behalf of the Apache Software Foundation. For more - * information on the Apache Software Foundation, please see - * . - * - */ - -package org.apache.http.conn.socket; - -import org.apache.http.HttpHost; -import org.apache.http.annotation.Immutable; -import org.apache.http.conn.ConnectTimeoutException; -import org.apache.http.conn.HttpHostConnectException; -import org.apache.http.protocol.HttpContext; - -import java.io.IOException; -import java.net.ConnectException; -import java.net.InetSocketAddress; -import java.net.Socket; -import java.net.SocketTimeoutException; - -/** - * This is a base class for {@link ConnectionSocketFactory} implementations. - * This class provides a common exception handling logic for connect operations. - * - * @since 4.3 - */ -public abstract class AbstractConnectionSocketFactory implements ConnectionSocketFactory { - - /** - * Connects a socket to the target host with the given resolved remote address. - * - * @param connectTimeout connect timeout. - * @param socket the socket to connect, as obtained from {@link #createSocket(HttpContext)}. - * null indicates that a new socket should be created and connected. - * @param host target host as specified by the caller (end user). - * @param remoteAddress the resolved remote address to connect to. - * @param localAddress the local address to bind the socket to, or null for any. - * @param context the actual HTTP context. - * - * @return the connected socket. The returned object may be different - * from the sock argument if this factory supports - * a layered protocol. - * @throws org.apache.http.conn.ConnectTimeoutException if the socket cannot be connected - * within the time limit defined by connectTimeout parameter. - * @throws org.apache.http.conn.HttpHostConnectException if the connection is refused - * by the opposite endpoint. - */ - public Socket connectSocket( - final int connectTimeout, - final Socket socket, - final HttpHost host, - final InetSocketAddress remoteAddress, - final InetSocketAddress localAddress, - final HttpContext context) throws IOException { - final Socket sock = socket != null ? socket : createSocket(context); - if (localAddress != null) { - sock.bind(localAddress); - } - try { - sock.connect(remoteAddress, connectTimeout); - } catch (final SocketTimeoutException ex) { - closeSocket(socket); - throw new ConnectTimeoutException(host, remoteAddress, ex); - } catch (final ConnectException ex) { - closeSocket(socket); - String msg = ex.getMessage(); - if ("Connection timed out".equals(msg)) { - throw new ConnectTimeoutException(host, remoteAddress, ex); - } else { - throw new HttpHostConnectException(host, remoteAddress, ex); - } - } - return sock; - } - - private void closeSocket(final Socket sock) { - try { - sock.close(); - } catch (final IOException ignore) { - } - } - -} diff --git a/httpclient/src/main/java/org/apache/http/conn/socket/ConnectionSocketFactory.java b/httpclient/src/main/java/org/apache/http/conn/socket/ConnectionSocketFactory.java old mode 100755 new mode 100644 diff --git a/httpclient/src/main/java/org/apache/http/conn/socket/PlainSocketFactory.java b/httpclient/src/main/java/org/apache/http/conn/socket/PlainSocketFactory.java old mode 100755 new mode 100644 index c73d0dd6a..ee17d03a5 --- a/httpclient/src/main/java/org/apache/http/conn/socket/PlainSocketFactory.java +++ b/httpclient/src/main/java/org/apache/http/conn/socket/PlainSocketFactory.java @@ -28,8 +28,10 @@ package org.apache.http.conn.socket; import java.io.IOException; +import java.net.InetSocketAddress; import java.net.Socket; +import org.apache.http.HttpHost; import org.apache.http.annotation.Immutable; import org.apache.http.protocol.HttpContext; @@ -39,7 +41,7 @@ import org.apache.http.protocol.HttpContext; * @since 4.3 */ @Immutable -public class PlainSocketFactory extends AbstractConnectionSocketFactory { +public class PlainSocketFactory implements ConnectionSocketFactory { public static final PlainSocketFactory INSTANCE = new PlainSocketFactory(); @@ -55,4 +57,27 @@ public class PlainSocketFactory extends AbstractConnectionSocketFactory { return new Socket(); } + public Socket connectSocket( + final int connectTimeout, + final Socket socket, + final HttpHost host, + final InetSocketAddress remoteAddress, + final InetSocketAddress localAddress, + final HttpContext context) throws IOException { + final Socket sock = socket != null ? socket : createSocket(context); + if (localAddress != null) { + sock.bind(localAddress); + } + try { + sock.connect(remoteAddress, connectTimeout); + } catch (final IOException ex) { + try { + sock.close(); + } catch (final IOException ignore) { + } + throw ex; + } + return sock; + } + } diff --git a/httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java b/httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java old mode 100755 new mode 100644 index 5c9659ef1..b7575242d --- a/httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java +++ b/httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java @@ -28,6 +28,7 @@ package org.apache.http.conn.ssl; import java.io.IOException; +import java.net.ConnectException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Socket; @@ -51,7 +52,6 @@ import org.apache.http.conn.scheme.HostNameResolver; import org.apache.http.conn.scheme.LayeredSchemeSocketFactory; import org.apache.http.conn.scheme.LayeredSocketFactory; import org.apache.http.conn.scheme.SchemeLayeredSocketFactory; -import org.apache.http.conn.socket.AbstractConnectionSocketFactory; import org.apache.http.conn.socket.LayeredConnectionSocketFactory; import org.apache.http.params.HttpConnectionParams; import org.apache.http.params.HttpParams; @@ -92,8 +92,7 @@ import org.apache.http.util.TextUtils; */ @SuppressWarnings("deprecation") @ThreadSafe -public class SSLSocketFactory extends AbstractConnectionSocketFactory - implements LayeredConnectionSocketFactory, SchemeLayeredSocketFactory, +public class SSLSocketFactory implements LayeredConnectionSocketFactory, SchemeLayeredSocketFactory, LayeredSchemeSocketFactory, LayeredSocketFactory { public static final String TLS = "TLS"; @@ -553,8 +552,19 @@ public class SSLSocketFactory extends AbstractConnectionSocketFactory final HttpContext context) throws IOException { Args.notNull(host, "HTTP host"); Args.notNull(remoteAddress, "Remote address"); - final Socket sock = super.connectSocket( - connectTimeout, socket, host, remoteAddress, localAddress, context); + final Socket sock = socket != null ? socket : createSocket(context); + if (localAddress != null) { + sock.bind(localAddress); + } + try { + sock.connect(remoteAddress, connectTimeout); + } catch (final IOException ex) { + try { + sock.close(); + } catch (final IOException ignore) { + } + throw ex; + } // Setup SSL layering if necessary if (sock instanceof SSLSocket) { final SSLSocket sslsock = (SSLSocket) sock; diff --git a/httpclient/src/main/java/org/apache/http/impl/conn/HttpClientConnectionOperator.java b/httpclient/src/main/java/org/apache/http/impl/conn/HttpClientConnectionOperator.java index 4b8567961..3a5b33d7d 100644 --- a/httpclient/src/main/java/org/apache/http/impl/conn/HttpClientConnectionOperator.java +++ b/httpclient/src/main/java/org/apache/http/impl/conn/HttpClientConnectionOperator.java @@ -31,6 +31,7 @@ import java.net.ConnectException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Socket; +import java.net.SocketTimeoutException; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -42,6 +43,7 @@ import org.apache.http.config.SocketConfig; import org.apache.http.conn.ConnectTimeoutException; import org.apache.http.conn.DnsResolver; import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.conn.HttpHostConnectException; import org.apache.http.conn.ManagedHttpClientConnection; import org.apache.http.conn.SchemePortResolver; import org.apache.http.conn.UnsupportedSchemeException; @@ -123,13 +125,18 @@ class HttpClientConnectionOperator { } conn.bind(sock); return; + } catch (final SocketTimeoutException ex) { + if (last) { + throw new ConnectTimeoutException(ex, host, addresses); + } } catch (final ConnectException ex) { if (last) { - throw ex; - } - } catch (final ConnectTimeoutException ex) { - if (last) { - throw ex; + final String msg = ex.getMessage(); + if ("Connection timed out".equals(msg)) { + throw new ConnectTimeoutException(ex, host, addresses); + } else { + throw new HttpHostConnectException(ex, host, addresses); + } } } if (this.log.isDebugEnabled()) { diff --git a/httpclient/src/test/java/org/apache/http/conn/TestExceptions.java b/httpclient/src/test/java/org/apache/http/conn/TestExceptions.java index 27297db9c..79b113206 100644 --- a/httpclient/src/test/java/org/apache/http/conn/TestExceptions.java +++ b/httpclient/src/test/java/org/apache/http/conn/TestExceptions.java @@ -33,7 +33,6 @@ import org.junit.Test; import java.io.IOException; import java.net.InetAddress; -import java.net.InetSocketAddress; /** * Unit tests for exceptions. @@ -62,7 +61,7 @@ public class TestExceptions { @Test public void testConnectTimeoutExceptionFromCause() { final IOException cause = new IOException("something awful"); - final ConnectTimeoutException ctx = new ConnectTimeoutException(null, null, cause); + final ConnectTimeoutException ctx = new ConnectTimeoutException(cause, null, null); Assert.assertEquals("Connect to remote host failed: something awful", ctx.getMessage()); } @@ -70,18 +69,17 @@ public class TestExceptions { public void testConnectTimeoutExceptionFromCauseAndHost() { final HttpHost target = new HttpHost("localhost"); final IOException cause = new IOException(); - final ConnectTimeoutException ctx = new ConnectTimeoutException(target, null, cause); + final ConnectTimeoutException ctx = new ConnectTimeoutException(cause, target, null); Assert.assertEquals("Connect to localhost timed out", ctx.getMessage()); } @Test public void testConnectTimeoutExceptionFromCauseHostAndRemoteAddress() throws Exception { final HttpHost target = new HttpHost("localhost"); - final InetSocketAddress remoteAddress = new InetSocketAddress( - InetAddress.getByAddress(new byte[] {1,2,3,4}), 1234); + final InetAddress remoteAddress = InetAddress.getByAddress(new byte[] {1,2,3,4}); final IOException cause = new IOException(); - final ConnectTimeoutException ctx = new ConnectTimeoutException(target, remoteAddress, cause); - Assert.assertEquals("Connect to localhost (/1.2.3.4) timed out", ctx.getMessage()); + final ConnectTimeoutException ctx = new ConnectTimeoutException(cause, target, remoteAddress); + Assert.assertEquals("Connect to localhost [/1.2.3.4] timed out", ctx.getMessage()); } @Test @@ -93,7 +91,7 @@ public class TestExceptions { @Test public void testHttpHostConnectExceptionFromCause() { final IOException cause = new IOException("something awful"); - final HttpHostConnectException ctx = new HttpHostConnectException(null, null, cause); + final HttpHostConnectException ctx = new HttpHostConnectException(cause, null, null); Assert.assertEquals("Connect to remote host failed: something awful", ctx.getMessage()); } @@ -101,18 +99,19 @@ public class TestExceptions { public void testHttpHostConnectExceptionFromCauseAndHost() { final HttpHost target = new HttpHost("localhost"); final IOException cause = new IOException(); - final HttpHostConnectException ctx = new HttpHostConnectException(target, null, cause); + final HttpHostConnectException ctx = new HttpHostConnectException(cause, target, null); Assert.assertEquals("Connect to localhost refused", ctx.getMessage()); } @Test public void testHttpHostConnectExceptionFromCauseHostAndRemoteAddress() throws Exception { final HttpHost target = new HttpHost("localhost"); - final InetSocketAddress remoteAddress = new InetSocketAddress( - InetAddress.getByAddress(new byte[] {1,2,3,4}), 1234); + final InetAddress remoteAddress1 = InetAddress.getByAddress(new byte[] {1,2,3,4}); + final InetAddress remoteAddress2 = InetAddress.getByAddress(new byte[] {5,6,7,8}); final IOException cause = new IOException(); - final HttpHostConnectException ctx = new HttpHostConnectException(target, remoteAddress, cause); - Assert.assertEquals("Connect to localhost (/1.2.3.4) refused", ctx.getMessage()); + final HttpHostConnectException ctx = new HttpHostConnectException(cause, target, + remoteAddress1, remoteAddress2); + Assert.assertEquals("Connect to localhost [/1.2.3.4, /5.6.7.8] refused", ctx.getMessage()); } @Test diff --git a/httpclient/src/test/java/org/apache/http/impl/conn/TestHttpClientConnectionOperator.java b/httpclient/src/test/java/org/apache/http/impl/conn/TestHttpClientConnectionOperator.java index 97f82b7f9..3e7054776 100644 --- a/httpclient/src/test/java/org/apache/http/impl/conn/TestHttpClientConnectionOperator.java +++ b/httpclient/src/test/java/org/apache/http/impl/conn/TestHttpClientConnectionOperator.java @@ -27,15 +27,18 @@ package org.apache.http.impl.conn; +import java.net.ConnectException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.Socket; +import java.net.SocketTimeoutException; import org.apache.http.HttpHost; import org.apache.http.config.Lookup; import org.apache.http.config.SocketConfig; import org.apache.http.conn.ConnectTimeoutException; import org.apache.http.conn.DnsResolver; +import org.apache.http.conn.HttpHostConnectException; import org.apache.http.conn.SchemePortResolver; import org.apache.http.conn.ManagedHttpClientConnection; import org.apache.http.conn.UnsupportedSchemeException; @@ -119,6 +122,28 @@ public class TestHttpClientConnectionOperator { } @Test(expected=ConnectTimeoutException.class) + public void testConnectTimeout() throws Exception { + final HttpContext context = new BasicHttpContext(); + final HttpHost host = new HttpHost("somehost"); + final InetAddress ip1 = InetAddress.getByAddress(new byte[] {10, 0, 0, 1}); + final InetAddress ip2 = InetAddress.getByAddress(new byte[] {10, 0, 0, 2}); + + Mockito.when(dnsResolver.resolve("somehost")).thenReturn(new InetAddress[] { ip1, ip2 }); + Mockito.when(socketFactoryRegistry.lookup("http")).thenReturn(plainSocketFactory); + Mockito.when(schemePortResolver.resolve(host)).thenReturn(80); + Mockito.when(plainSocketFactory.createSocket(Mockito.any())).thenReturn(socket); + Mockito.when(plainSocketFactory.connectSocket( + Mockito.anyInt(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any())).thenThrow(new SocketTimeoutException()); + + connectionOperator.connect(conn, host, null, 1000, SocketConfig.DEFAULT, context); + } + + @Test(expected=HttpHostConnectException.class) public void testConnectFailure() throws Exception { final HttpContext context = new BasicHttpContext(); final HttpHost host = new HttpHost("somehost"); @@ -135,7 +160,7 @@ public class TestHttpClientConnectionOperator { Mockito.any(), Mockito.any(), Mockito.any(), - Mockito.any())).thenThrow(new ConnectTimeoutException()); + Mockito.any())).thenThrow(new ConnectException()); connectionOperator.connect(conn, host, null, 1000, SocketConfig.DEFAULT, context); } @@ -158,7 +183,7 @@ public class TestHttpClientConnectionOperator { Mockito.any(), Mockito.eq(new InetSocketAddress(ip1, 80)), Mockito.any(), - Mockito.any())).thenThrow(new ConnectTimeoutException()); + Mockito.any())).thenThrow(new ConnectException()); Mockito.when(plainSocketFactory.connectSocket( Mockito.anyInt(), Mockito.any(),