From 2b3d2b0c4822e580bee7e70ce1c529306455c387 Mon Sep 17 00:00:00 2001 From: "adrian.f.cole" Date: Fri, 29 Jan 2010 23:39:47 +0000 Subject: [PATCH] fix bugs found by pvdyck blind usage of content on HEAD requests and throwing away data on generation of uri git-svn-id: http://jclouds.googlecode.com/svn/trunk@2738 3d8758e0-26b5-11de-8745-db77d3ebf521 --- .../AtmosStorageClientErrorRetryHandler.java | 21 +++++++++++------- .../handlers/AWSClientErrorRetryHandler.java | 20 ++++++++++------- .../AzureBlobClientErrorRetryHandler.java | 21 ++++++++++-------- .../java/org/jclouds/http/HttpRequest.java | 22 +++++++++---------- .../rest/internal/GeneratedHttpRequest.java | 4 ++-- .../internal/RestAnnotationProcessor.java | 7 +++--- 6 files changed, 53 insertions(+), 42 deletions(-) diff --git a/atmos/src/main/java/org/jclouds/atmosonline/saas/handlers/AtmosStorageClientErrorRetryHandler.java b/atmos/src/main/java/org/jclouds/atmosonline/saas/handlers/AtmosStorageClientErrorRetryHandler.java index a6317e8610..22e5c8a51e 100644 --- a/atmos/src/main/java/org/jclouds/atmosonline/saas/handlers/AtmosStorageClientErrorRetryHandler.java +++ b/atmos/src/main/java/org/jclouds/atmosonline/saas/handlers/AtmosStorageClientErrorRetryHandler.java @@ -64,16 +64,21 @@ public class AtmosStorageClientErrorRetryHandler implements HttpRetryHandler { return true; } else if (response.getStatusCode() == 409 || response.getStatusCode() == 400) { byte[] content = HttpUtils.closeClientButKeepContentStream(response); - try { - AtmosStorageError error = utils.parseAtmosStorageErrorFromContent(command, response, - new String(content)); - if (error.getCode() == 1016) { - return backoffHandler.shouldRetryRequest(command, response); + // Content can be null in the case of HEAD requests + if (content != null) { + try { + AtmosStorageError error = utils.parseAtmosStorageErrorFromContent(command, response, + new String(content)); + if (error.getCode() == 1016) { + return backoffHandler.shouldRetryRequest(command, response); + } + // don't increment count before here, since backoff handler does already + command.incrementFailureCount(); + } catch (HttpException e) { + logger.warn(e, "error parsing response: %s", new String(content)); } - // don't increment count before here, since backoff handler does already + } else { command.incrementFailureCount(); - } catch (HttpException e) { - logger.warn(e, "error parsing response: %s", new String(content)); } return true; } diff --git a/aws/core/src/main/java/org/jclouds/aws/handlers/AWSClientErrorRetryHandler.java b/aws/core/src/main/java/org/jclouds/aws/handlers/AWSClientErrorRetryHandler.java index 781717ff9d..b35f516c06 100755 --- a/aws/core/src/main/java/org/jclouds/aws/handlers/AWSClientErrorRetryHandler.java +++ b/aws/core/src/main/java/org/jclouds/aws/handlers/AWSClientErrorRetryHandler.java @@ -61,15 +61,19 @@ public class AWSClientErrorRetryHandler implements HttpRetryHandler { || response.getStatusCode() == 409) { byte[] content = HttpUtils.closeClientButKeepContentStream(response); command.incrementFailureCount(); - try { - AWSError error = utils.parseAWSErrorFromContent(command, response, new String(content)); - if ("RequestTimeout".equals(error.getCode()) - || "OperationAborted".equals(error.getCode()) - || "SignatureDoesNotMatch".equals(error.getCode())) { - return true; + // Content can be null in the case of HEAD requests + if (content != null) { + try { + AWSError error = utils.parseAWSErrorFromContent(command, response, new String( + content)); + if ("RequestTimeout".equals(error.getCode()) + || "OperationAborted".equals(error.getCode()) + || "SignatureDoesNotMatch".equals(error.getCode())) { + return true; + } + } catch (HttpException e) { + logger.warn(e, "error parsing response: %s", new String(content)); } - } catch (HttpException e) { - logger.warn(e, "error parsing response: %s", new String(content)); } } return false; diff --git a/azure/src/main/java/org/jclouds/azure/storage/blob/handlers/AzureBlobClientErrorRetryHandler.java b/azure/src/main/java/org/jclouds/azure/storage/blob/handlers/AzureBlobClientErrorRetryHandler.java index ca14560e44..080be34b57 100644 --- a/azure/src/main/java/org/jclouds/azure/storage/blob/handlers/AzureBlobClientErrorRetryHandler.java +++ b/azure/src/main/java/org/jclouds/azure/storage/blob/handlers/AzureBlobClientErrorRetryHandler.java @@ -72,16 +72,19 @@ public class AzureBlobClientErrorRetryHandler implements HttpRetryHandler { retryCountLimit, command); return false; } else if (response.getStatusCode() == 409) { - try { - AzureStorageError error = utils.parseAzureStorageErrorFromContent(command, response, - new ByteArrayInputStream(content)); - if ("ContainerBeingDeleted".equals(error.getCode())) { - backoffHandler.imposeBackoffExponentialDelay(100L, 3, command.getFailureCount(), - command.toString()); - return true; + // Content can be null in the case of HEAD requests + if (content != null) { + try { + AzureStorageError error = utils.parseAzureStorageErrorFromContent(command, response, + new ByteArrayInputStream(content)); + if ("ContainerBeingDeleted".equals(error.getCode())) { + backoffHandler.imposeBackoffExponentialDelay(100L, 3, command.getFailureCount(), + command.toString()); + return true; + } + } catch (HttpException e) { + logger.warn(e, "error parsing response: %s", new String(content)); } - } catch (HttpException e) { - logger.warn(e, "error parsing response: %s", new String(content)); } } return false; diff --git a/core/src/main/java/org/jclouds/http/HttpRequest.java b/core/src/main/java/org/jclouds/http/HttpRequest.java index cd569ecd68..a8ebd7fe88 100644 --- a/core/src/main/java/org/jclouds/http/HttpRequest.java +++ b/core/src/main/java/org/jclouds/http/HttpRequest.java @@ -46,40 +46,40 @@ public class HttpRequest extends HttpMessage { /** * - * @param endPoint + * @param endpoint * This may change over the life of the request due to redirects. * @param method * If the request is HEAD, this may change to GET due to redirects */ - public HttpRequest(String method, URI endPoint) { + public HttpRequest(String method, URI endpoint) { this.setMethod(checkNotNull(method, "method")); - this.setEndpoint(checkNotNull(endPoint, "endPoint")); - checkArgument(endPoint.getHost() != null, String.format("endPoint.getHost() is null for %s", - endPoint)); + this.setEndpoint(checkNotNull(endpoint, "endpoint")); + checkArgument(endpoint.getHost() != null, String.format("endpoint.getHost() is null for %s", + endpoint)); } /** * - * @param endPoint + * @param endpoint * This may change over the life of the request due to redirects. * @param method * If the request is HEAD, this may change to GET due to redirects */ - public HttpRequest(String method, URI endPoint, Multimap headers) { - this(method, endPoint); + public HttpRequest(String method, URI endpoint, Multimap headers) { + this(method, endpoint); getHeaders().putAll(checkNotNull(headers, "headers")); } /** * - * @param endPoint + * @param endpoint * This may change over the life of the request due to redirects. * @param method * If the request is HEAD, this may change to GET due to redirects */ - protected HttpRequest(String method, URI endPoint, Multimap headers, + protected HttpRequest(String method, URI endpoint, Multimap headers, @Nullable Payload payload) { - this(method, endPoint); + this(method, endpoint); getHeaders().putAll(checkNotNull(headers, "headers")); setPayload(payload); } diff --git a/core/src/main/java/org/jclouds/rest/internal/GeneratedHttpRequest.java b/core/src/main/java/org/jclouds/rest/internal/GeneratedHttpRequest.java index 122ac18f9d..5b8c25b505 100644 --- a/core/src/main/java/org/jclouds/rest/internal/GeneratedHttpRequest.java +++ b/core/src/main/java/org/jclouds/rest/internal/GeneratedHttpRequest.java @@ -40,9 +40,9 @@ public class GeneratedHttpRequest extends HttpRequest { private final Object[] args; private final RestAnnotationProcessor processor; - GeneratedHttpRequest(String method, URI endPoint, RestAnnotationProcessor processor, + GeneratedHttpRequest(String method, URI endpoint, RestAnnotationProcessor processor, Class declaring, Method javaMethod, Object... args) { - super(method, endPoint); + super(method, endpoint); this.processor = processor; this.declaring = declaring; this.javaMethod = javaMethod; diff --git a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java index a7287c6516..188c9014aa 100755 --- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java +++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java @@ -61,12 +61,12 @@ import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpRequestFilter; import org.jclouds.http.HttpResponse; import org.jclouds.http.HttpUtils; +import org.jclouds.http.functions.CloseContentAndReturn; import org.jclouds.http.functions.ParseSax; import org.jclouds.http.functions.ParseURIFromListOrLocationHeaderIf20x; import org.jclouds.http.functions.ReturnInputStream; import org.jclouds.http.functions.ReturnStringIf200; import org.jclouds.http.functions.ReturnTrueIf2xx; -import org.jclouds.http.functions.CloseContentAndReturn; import org.jclouds.http.functions.ParseSax.HandlerWithResult; import org.jclouds.http.options.HttpRequestOptions; import org.jclouds.logging.Logger; @@ -360,16 +360,15 @@ public class RestAnnotationProcessor { builder.replaceQuery(makeQueryLine(queryParams, null, skips)); } - URI endPoint; try { - endPoint = builder.buildFromEncodedMap(convertUnsafe(tokenValues)); + endpoint = builder.buildFromEncodedMap(convertUnsafe(tokenValues)); } catch (IllegalArgumentException e) { throw new IllegalStateException(e); } catch (UriBuilderException e) { throw new IllegalStateException(e); } - GeneratedHttpRequest request = new GeneratedHttpRequest(httpMethod, endPoint, this, + GeneratedHttpRequest request = new GeneratedHttpRequest(httpMethod, endpoint, this, declaring, method, args); addHostHeaderIfAnnotatedWithVirtualHost(headers, request.getEndpoint().getHost(), method); addFiltersIfAnnotated(method, request);