From 61f45b5c48564ec416d64e02fb8f39ab2c26e24d Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Tue, 8 Mar 2011 20:31:34 +0000 Subject: [PATCH 1/4] HttpClient 4.1.x branch git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/branches/4.1.x@1079518 13f79535-47bb-0310-9956-ffa450edef68 From 73e6ca8bb80119eea448019535bba3c33a6d2e66 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Tue, 8 Mar 2011 20:54:13 +0000 Subject: [PATCH 2/4] Removed 4.2 methods git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/branches/4.1.x@1079534 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/http/auth/AuthScope.java | 15 ---- .../client/protocol/RequestAuthCache.java | 3 +- .../DefaultHttpRequestRetryHandler.java | 5 +- .../StandardHttpRequestRetryHandler.java | 81 ------------------- .../client/protocol/TestRequestAuthCache.java | 4 +- .../protocol/TestResponseAuthCache.java | 4 +- 6 files changed, 7 insertions(+), 105 deletions(-) delete mode 100644 httpclient/src/main/java/org/apache/http/impl/client/StandardHttpRequestRetryHandler.java diff --git a/httpclient/src/main/java/org/apache/http/auth/AuthScope.java b/httpclient/src/main/java/org/apache/http/auth/AuthScope.java index 26e6f7040..f14627cff 100644 --- a/httpclient/src/main/java/org/apache/http/auth/AuthScope.java +++ b/httpclient/src/main/java/org/apache/http/auth/AuthScope.java @@ -28,7 +28,6 @@ import java.util.Locale; -import org.apache.http.HttpHost; import org.apache.http.annotation.Immutable; import org.apache.http.util.LangUtils; @@ -110,20 +109,6 @@ public AuthScope(final String host, int port, this.scheme = (scheme == null) ? ANY_SCHEME: scheme.toUpperCase(Locale.ENGLISH); } - /** - * @since 4.2 - */ - public AuthScope(final HttpHost host, final String realm, final String schemeName) { - this(host.getHostName(), host.getPort(), realm, schemeName); - } - - /** - * @since 4.2 - */ - public AuthScope(final HttpHost host) { - this(host, ANY_REALM, ANY_SCHEME); - } - /** Creates a new credentials scope for the given * host, port, realm, and any * authentication scheme. diff --git a/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java b/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java index 0602824f6..4693418f7 100644 --- a/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java +++ b/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java @@ -112,7 +112,8 @@ private void doPreemptiveAuth( this.log.debug("Re-using cached '" + schemeName + "' auth scheme for " + host); } - AuthScope authScope = new AuthScope(host, AuthScope.ANY_REALM, schemeName); + AuthScope authScope = new AuthScope(host.getHostName(), host.getPort(), + AuthScope.ANY_REALM, schemeName); Credentials creds = credsProvider.getCredentials(authScope); if (creds != null) { diff --git a/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java b/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java index e586ba3c1..9cfecc7ed 100644 --- a/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java +++ b/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java @@ -142,10 +142,7 @@ public int getRetryCount() { return retryCount; } - /** - * @since 4.2 - */ - protected boolean handleAsIdempotent(final HttpRequest request) { + private boolean handleAsIdempotent(final HttpRequest request) { return !(request instanceof HttpEntityEnclosingRequest); } diff --git a/httpclient/src/main/java/org/apache/http/impl/client/StandardHttpRequestRetryHandler.java b/httpclient/src/main/java/org/apache/http/impl/client/StandardHttpRequestRetryHandler.java deleted file mode 100644 index 0ebac0d23..000000000 --- a/httpclient/src/main/java/org/apache/http/impl/client/StandardHttpRequestRetryHandler.java +++ /dev/null @@ -1,81 +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.impl.client; - -import java.util.Locale; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -import org.apache.http.HttpRequest; -import org.apache.http.annotation.Immutable; -import org.apache.http.client.HttpRequestRetryHandler; - -/** - * A {@link HttpRequestRetryHandler} which assumes that all requested - * HTTP methods which should be idempotent according to RFC-2616 are - * in fact idempotent and can be retried. - * - * According to RFC-2616 section 9.1.2 the idempotent HTTP methods are: - * GET, HEAD, PUT, DELETE, OPTIONS, and TRACE - * - * @since 4.2 - */ -@Immutable -public class StandardHttpRequestRetryHandler extends DefaultHttpRequestRetryHandler { - - private final Map idempotentMethods; - - /** - * Default constructor - */ - public StandardHttpRequestRetryHandler(int retryCount, boolean requestSentRetryEnabled) { - super(retryCount, requestSentRetryEnabled); - this.idempotentMethods = new ConcurrentHashMap(); - this.idempotentMethods.put("GET", Boolean.TRUE); - this.idempotentMethods.put("HEAD", Boolean.TRUE); - this.idempotentMethods.put("PUT", Boolean.TRUE); - this.idempotentMethods.put("DELETE", Boolean.TRUE); - this.idempotentMethods.put("OPTIONS", Boolean.TRUE); - this.idempotentMethods.put("TRACE", Boolean.TRUE); - } - - /** - * Default constructor - */ - public StandardHttpRequestRetryHandler() { - this(3, false); - } - - @Override - protected boolean handleAsIdempotent(final HttpRequest request) { - String method = request.getRequestLine().getMethod().toUpperCase(Locale.US); - Boolean b = this.idempotentMethods.get(method); - return b != null && b.booleanValue(); - } - -} diff --git a/httpclient/src/test/java/org/apache/http/client/protocol/TestRequestAuthCache.java b/httpclient/src/test/java/org/apache/http/client/protocol/TestRequestAuthCache.java index 6f93890a1..6f6742f0c 100644 --- a/httpclient/src/test/java/org/apache/http/client/protocol/TestRequestAuthCache.java +++ b/httpclient/src/test/java/org/apache/http/client/protocol/TestRequestAuthCache.java @@ -68,8 +68,8 @@ public void setUp() { this.credProvider = new BasicCredentialsProvider(); this.creds1 = new UsernamePasswordCredentials("user1", "secret1"); this.creds2 = new UsernamePasswordCredentials("user2", "secret2"); - this.authscope1 = new AuthScope(this.target); - this.authscope2 = new AuthScope(this.proxy); + this.authscope1 = new AuthScope(this.target.getHostName(), this.target.getPort()); + this.authscope2 = new AuthScope(this.proxy.getHostName(), this.proxy.getPort()); this.authscheme1 = new BasicScheme(); this.authscheme2 = new BasicScheme(); diff --git a/httpclient/src/test/java/org/apache/http/client/protocol/TestResponseAuthCache.java b/httpclient/src/test/java/org/apache/http/client/protocol/TestResponseAuthCache.java index 6ed4e301e..d8edf3518 100644 --- a/httpclient/src/test/java/org/apache/http/client/protocol/TestResponseAuthCache.java +++ b/httpclient/src/test/java/org/apache/http/client/protocol/TestResponseAuthCache.java @@ -68,8 +68,8 @@ public void setUp() throws Exception { this.creds1 = new UsernamePasswordCredentials("user1", "secret1"); this.creds2 = new UsernamePasswordCredentials("user2", "secret2"); - this.authscope1 = new AuthScope(this.target); - this.authscope2 = new AuthScope(this.proxy); + this.authscope1 = new AuthScope(this.target.getHostName(), this.target.getPort()); + this.authscope2 = new AuthScope(this.proxy.getHostName(), this.proxy.getPort()); this.authscheme1 = new BasicScheme(); this.authscheme2 = new BasicScheme(); From 126f1da34957b6150c2c68947617dac36b39e0c4 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 10 Mar 2011 09:24:57 +0000 Subject: [PATCH 3/4] Use SSL socket factory to create unconnected sockets git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/branches/4.1.x@1080163 13f79535-47bb-0310-9956-ffa450edef68 --- .../main/java/org/apache/http/conn/ssl/SSLSocketFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 index 47dc0c427..38b6141da 100644 --- a/httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java +++ b/httpclient/src/main/java/org/apache/http/conn/ssl/SSLSocketFactory.java @@ -339,12 +339,12 @@ private SSLSocketFactory() { * @since 4.1 */ public Socket createSocket(final HttpParams params) throws IOException { - return new Socket(); + return this.socketfactory.createSocket(); } @Deprecated public Socket createSocket() throws IOException { - return new Socket(); + return this.socketfactory.createSocket(); } /** From 490778e271dcb7508928060c44fcfbcee6077d9b Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Thu, 10 Mar 2011 09:35:17 +0000 Subject: [PATCH 4/4] HTTPCLIENT-1069: HttpHostConnectException not correctly retried for direct and non-tunnelled proxy connections git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/branches/4.1.x@1080165 13f79535-47bb-0310-9956-ffa450edef68 --- RELEASE_NOTES.txt | 4 +++ .../impl/client/DefaultRequestDirector.java | 35 +++++++++---------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 23cea491e..64b022cb6 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,5 +1,9 @@ Changes since 4.1 +* [HTTPCLIENT-1069] HttpHostConnectException not correctly retried for direct and non-tunnelled + proxy connections. + Contributed by Oleg Kalnichevski + * [HTTPCLIENT-1066] Changed the way URIUtils#rewriteURI handles multiple consecutive slashes in the URI path component: multiple leading slashes will be replaced by one slash in order to avoid confusion with the authority component. The remaining content of the path will not be modified. diff --git a/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java b/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java index 92f982e5f..d9818f12b 100644 --- a/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java +++ b/httpclient/src/main/java/org/apache/http/impl/client/DefaultRequestDirector.java @@ -552,9 +552,8 @@ private void tryConnect( final RoutedRequest req, final HttpContext context) throws HttpException, IOException { HttpRoute route = req.getRoute(); - boolean retrying = true; int connectCount = 0; - while (retrying) { + for (;;) { // Increment connect count connectCount++; try { @@ -564,7 +563,7 @@ private void tryConnect( managedConn.setSocketTimeout(HttpConnectionParams.getSoTimeout(params)); } establishRoute(route, context); - retrying = false; + break; } catch (IOException ex) { try { managedConn.close(); @@ -596,9 +595,8 @@ private HttpResponse tryExecute( HttpRoute route = req.getRoute(); HttpResponse response = null; - boolean retrying = true; Exception retryReason = null; - while (retrying) { + for (;;) { // Increment total exec count (with redirects) execCount++; // Increment exec count for this particular request @@ -616,11 +614,24 @@ private HttpResponse tryExecute( } try { + if (!managedConn.isOpen()) { + // If we have a direct route to the target host + // just re-open connection and re-try the request + if (!route.isTunnelled()) { + this.log.debug("Reopening the direct connection."); + managedConn.open(route, context, params); + } else { + // otherwise give up + this.log.debug("Proxied connection. Need to start over."); + break; + } + } + if (this.log.isDebugEnabled()) { this.log.debug("Attempt " + execCount + " to execute request"); } response = requestExec.execute(wrapper, managedConn, context); - retrying = false; + break; } catch (IOException ex) { this.log.debug("Closing the connection."); @@ -642,18 +653,6 @@ private HttpResponse tryExecute( } else { throw ex; } - - // If we have a direct route to the target host - // just re-open connection and re-try the request - if (!route.isTunnelled()) { - this.log.debug("Reopening the direct connection."); - managedConn.open(route, context, params); - } else { - // otherwise give up - this.log.debug("Proxied connection. Need to start over."); - retrying = false; - } - } } return response;