RedirectionRetryHandler ignored PROPERTY_MAX_REDIRECTS on backoff

This commit is contained in:
adriancole 2013-02-19 00:56:17 -08:00
parent 72ad7d8199
commit 9c511ccdd9
3 changed files with 36 additions and 32 deletions

View File

@ -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);

View File

@ -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<String, String> 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;
}
}

View File

@ -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);