From 9c511ccdd94a19a3b230e54361fb1e339bd100ee Mon Sep 17 00:00:00 2001 From: adriancole Date: Tue, 19 Feb 2013 00:56:17 -0800 Subject: [PATCH] RedirectionRetryHandler ignored PROPERTY_MAX_REDIRECTS on backoff --- ...DeltacloudRedirectionRetryHandlerTest.java | 1 + .../handlers/RedirectionRetryHandler.java | 62 +++++++++---------- .../handlers/RedirectionRetryHandlerTest.java | 5 +- 3 files changed, 36 insertions(+), 32 deletions(-) diff --git a/apis/deltacloud/src/test/java/org/jclouds/deltacloud/handlers/DeltacloudRedirectionRetryHandlerTest.java b/apis/deltacloud/src/test/java/org/jclouds/deltacloud/handlers/DeltacloudRedirectionRetryHandlerTest.java index 5b7814fc28..823ad71ce7 100644 --- a/apis/deltacloud/src/test/java/org/jclouds/deltacloud/handlers/DeltacloudRedirectionRetryHandlerTest.java +++ b/apis/deltacloud/src/test/java/org/jclouds/deltacloud/handlers/DeltacloudRedirectionRetryHandlerTest.java @@ -67,6 +67,7 @@ public class DeltacloudRedirectionRetryHandlerTest { HttpRequest request = HttpRequest.builder().method("GET").endpoint("http://localhost").build(); HttpResponse response = HttpResponse.builder().statusCode(302).message("HTTP/1.1 302 Found").build(); + expect(command.isReplayable()).andReturn(true); expect(command.getCurrentRequest()).andReturn(request).atLeastOnce(); expect(command.incrementRedirectCount()).andReturn(1); diff --git a/core/src/main/java/org/jclouds/http/handlers/RedirectionRetryHandler.java b/core/src/main/java/org/jclouds/http/handlers/RedirectionRetryHandler.java index 610611e81c..ac80108faa 100644 --- a/core/src/main/java/org/jclouds/http/handlers/RedirectionRetryHandler.java +++ b/core/src/main/java/org/jclouds/http/handlers/RedirectionRetryHandler.java @@ -35,13 +35,12 @@ import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; import org.jclouds.http.HttpRetryHandler; import org.jclouds.logging.Logger; -import org.jclouds.util.Multimaps2; -import com.google.common.collect.Multimap; import com.google.inject.Inject; /** - * Handles Retryable responses with error codes in the 3xx range + * Handles Retryable responses with error codes in the 3xx range, backing off + * when redirecting to itself. * * @author Adrian Cole */ @@ -63,35 +62,36 @@ public class RedirectionRetryHandler implements HttpRetryHandler { public boolean shouldRetryRequest(HttpCommand command, HttpResponse response) { closeClientButKeepContentStream(response); - String hostHeader = response.getFirstHeaderOrNull(LOCATION); - if (command.incrementRedirectCount() < retryCountLimit && hostHeader != null) { - URI redirectionUrl = URI.create(hostHeader); - - // if you are sent the same uri, assume there's a transient problem and retry. - HttpRequest currentRequest = command.getCurrentRequest(); - if (redirectionUrl.equals(currentRequest.getEndpoint())) - return backoffHandler.shouldRetryRequest(command, response); - - assert redirectionUrl.getPath() != null : "no path in redirect header from: " + response; - if (!redirectionUrl.isAbsolute()) { - redirectionUrl = uriBuilder(currentRequest.getEndpoint()).path(redirectionUrl.getPath()) - .query(redirectionUrl.getQuery()).build(); - } - - if (currentRequest.getFirstHeaderOrNull(HOST) != null && redirectionUrl.getHost() != null) { - String host = redirectionUrl.getHost(); - if (redirectionUrl.getPort() != -1) { - host += ":" + redirectionUrl.getPort(); - } - Multimap newHeaders = Multimaps2.replaceValue(currentRequest.getHeaders(), HOST, host); - command.setCurrentRequest(currentRequest.toBuilder().headers(newHeaders).endpoint(redirectionUrl).build()); - } else { - command.setCurrentRequest(currentRequest.toBuilder().endpoint(redirectionUrl).build()); - } - return true; - } else { + if (!command.isReplayable()) { + logger.error("Cannot retry after redirect, command is not replayable: %s", command); return false; } + if (command.incrementRedirectCount() > retryCountLimit) { + logger.error("Cannot retry after redirect, command exceeded retry limit %d: %s", retryCountLimit, command); + return false; + } + String location = response.getFirstHeaderOrNull(LOCATION); + if (location == null) { + logger.error("Cannot retry after redirect, no host header: %s", command); + return false; + } + HttpRequest current = command.getCurrentRequest(); + URI redirect = URI.create(location); + if (!redirect.isAbsolute()) { + if (redirect.getPath() == null) { + logger.error("Cannot retry after redirect, no path in location header %s", command); + return false; + } + redirect = uriBuilder(current.getEndpoint()).path(redirect.getPath()).query(redirect.getQuery()).build(); + } + if (redirect.equals(current.getEndpoint())) { + backoffHandler.imposeBackoffExponentialDelay(command.getRedirectCount(), "redirect: " + command.toString()); + } else if (current.getFirstHeaderOrNull(HOST) != null && redirect.getHost() != null) { + String host = redirect.getPort() > 0 ? redirect.getHost() + ":" + redirect.getPort() : redirect.getHost(); + command.setCurrentRequest(current.toBuilder().replaceHeader(HOST, host).endpoint(redirect).build()); + } else { + command.setCurrentRequest(current.toBuilder().endpoint(redirect).build()); + } + return true; } - } diff --git a/core/src/test/java/org/jclouds/http/handlers/RedirectionRetryHandlerTest.java b/core/src/test/java/org/jclouds/http/handlers/RedirectionRetryHandlerTest.java index 22d67b3d34..efb1516e59 100644 --- a/core/src/test/java/org/jclouds/http/handlers/RedirectionRetryHandlerTest.java +++ b/core/src/test/java/org/jclouds/http/handlers/RedirectionRetryHandlerTest.java @@ -59,6 +59,7 @@ public class RedirectionRetryHandlerTest { .statusCode(302) .message("HTTP/1.1 302 Found").build(); + expect(command.isReplayable()).andReturn(true); expect(command.incrementRedirectCount()).andReturn(0); replay(command); @@ -80,7 +81,8 @@ public class RedirectionRetryHandlerTest { .message("HTTP/1.1 302 Found") .addHeader(LOCATION, "/api/v0.8b-ext2.5/Error.aspx?aspxerrorpath=/api/v0.8b-ext2.5/org.svc/1906645").build(); - expect(command.incrementRedirectCount()).andReturn(5); + expect(command.isReplayable()).andReturn(true); + expect(command.incrementRedirectCount()).andReturn(6); replay(command); @@ -174,6 +176,7 @@ public class RedirectionRetryHandlerTest { protected void verifyRedirectRoutes(HttpRequest request, HttpResponse response, HttpRequest expected) { HttpCommand command = createMock(HttpCommand.class); + expect(command.isReplayable()).andReturn(true); expect(command.incrementRedirectCount()).andReturn(0); expect(command.getCurrentRequest()).andReturn(request); command.setCurrentRequest(expected);