From 81ee88335ef64ae4871449b4775841f1873c03f8 Mon Sep 17 00:00:00 2001 From: "adrian.f.cole" Date: Fri, 26 Jun 2009 17:41:39 +0000 Subject: [PATCH] Issue 69: ensured redirect handlers copy the content stream before closing it, as this data is needed for error parsing git-svn-id: http://jclouds.googlecode.com/svn/trunk@1465 3d8758e0-26b5-11de-8745-db77d3ebf521 --- .../org/jclouds/aws/AWSResponseException.java | 4 +- .../java/org/jclouds/aws/domain/AWSError.java | 165 +++++------ .../jclouds/aws/s3/commands/DeleteBucket.java | 3 +- .../aws/s3/config/LiveS3ConnectionModule.java | 3 + .../handlers/ParseAWSErrorFromXmlContent.java | 81 +++--- .../FutureCommandConnectionPoolClient.java | 16 +- .../handlers/RedirectionRetryHandler.java | 43 ++- .../src/main/java/org/jclouds/util/Utils.java | 262 ++++++++++-------- .../jclouds/gae/URLFetchServiceClient.java | 3 +- 9 files changed, 332 insertions(+), 248 deletions(-) diff --git a/aws/core/src/main/java/org/jclouds/aws/AWSResponseException.java b/aws/core/src/main/java/org/jclouds/aws/AWSResponseException.java index 4b0d811f9f..1dae15caf2 100644 --- a/aws/core/src/main/java/org/jclouds/aws/AWSResponseException.java +++ b/aws/core/src/main/java/org/jclouds/aws/AWSResponseException.java @@ -44,8 +44,8 @@ public class AWSResponseException extends HttpResponseException { private AWSError error = new AWSError(); public AWSResponseException(HttpFutureCommand command, HttpResponse response, AWSError error) { - super(String.format("command %1$s failed with error: %2$s", command.toString(), error - .toString()), command, response); + super(String.format("command %s failed with code %s, error: %s", command.toString(), response + .getStatusCode(), error.toString()), command, response); this.setError(error); } diff --git a/aws/core/src/main/java/org/jclouds/aws/domain/AWSError.java b/aws/core/src/main/java/org/jclouds/aws/domain/AWSError.java index 0caf09afaa..720bdbbe73 100644 --- a/aws/core/src/main/java/org/jclouds/aws/domain/AWSError.java +++ b/aws/core/src/main/java/org/jclouds/aws/domain/AWSError.java @@ -29,103 +29,104 @@ import java.util.Map; /** * When an Amazon S3 request is in error, the client receives an error response. * - * @see + * @see * @author Adrian Cole * */ public class AWSError { - private String code; - private String message; - private String requestId; - private String requestToken; - private Map details = new HashMap(); - private String stringSigned; + private String code; + private String message; + private String requestId; + private String requestToken; + private Map details = new HashMap(); + private String stringSigned; - @Override - public String toString() { - final StringBuilder sb = new StringBuilder(); - sb.append("AWSError"); - sb.append("{code='").append(code).append('\''); - sb.append(", message='").append(message).append('\''); - sb.append(", requestId='").append(requestId).append('\''); - sb.append(", requestToken='").append(requestToken).append('\''); - if (stringSigned != null) - sb.append(", stringSigned='").append(stringSigned).append('\''); - sb.append(", context='").append(details.toString()).append('\'') - .append('}'); - return sb.toString(); - } + @Override + public String toString() { + final StringBuilder sb = new StringBuilder(); + sb.append("AWSError"); + sb.append("{requestId='").append(requestId).append('\''); + sb.append(", requestToken='").append(requestToken).append('\''); + if (code != null) + sb.append(", code='").append(code).append('\''); + if (message != null) + sb.append(", message='").append(message).append('\''); + if (stringSigned != null) + sb.append(", stringSigned='").append(stringSigned).append('\''); + if (details.size() != 0) + sb.append(", context='").append(details.toString()).append('\'').append('}'); + return sb.toString(); + } - public void setCode(String code) { - this.code = code; - } + public void setCode(String code) { + this.code = code; + } - /** - * The error code is a string that uniquely identifies an error condition. - * It is meant to be read and understood by programs that detect and handle - * errors by type - * - * @see - */ - public String getCode() { - return code; - } + /** + * The error code is a string that uniquely identifies an error condition. It is meant to be read + * and understood by programs that detect and handle errors by type + * + * @see + */ + public String getCode() { + return code; + } - public void setMessage(String message) { - this.message = message; - } + public void setMessage(String message) { + this.message = message; + } - /** - * The error message contains a generic description of the error condition - * in English. - * - * @see - */ - public String getMessage() { - return message; - } + /** + * The error message contains a generic description of the error condition in English. + * + * @see + */ + public String getMessage() { + return message; + } - public void setRequestId(String requestId) { - this.requestId = requestId; - } + public void setRequestId(String requestId) { + this.requestId = requestId; + } - /** - * * A unique ID assigned to each request by the system. In the unlikely - * event that you have problems with Amazon S3, Amazon can use this to help - * troubleshoot the problem. - * - */ - public String getRequestId() { - return requestId; - } + /** + * * A unique ID assigned to each request by the system. In the unlikely event that you have + * problems with Amazon S3, Amazon can use this to help troubleshoot the problem. + * + */ + public String getRequestId() { + return requestId; + } - public void setStringSigned(String stringSigned) { - this.stringSigned = stringSigned; - } + public void setStringSigned(String stringSigned) { + this.stringSigned = stringSigned; + } - /** - * @return what jclouds signed before sending the request. - */ - public String getStringSigned() { - return stringSigned; - } + /** + * @return what jclouds signed before sending the request. + */ + public String getStringSigned() { + return stringSigned; + } - public void setDetails(Map context) { - this.details = context; - } + public void setDetails(Map context) { + this.details = context; + } - /** - * @return additional details surrounding the error. - */ - public Map getDetails() { - return details; - } + /** + * @return additional details surrounding the error. + */ + public Map getDetails() { + return details; + } - public void setRequestToken(String requestToken) { - this.requestToken = requestToken; - } + public void setRequestToken(String requestToken) { + this.requestToken = requestToken; + } - public String getRequestToken() { - return requestToken; - } + public String getRequestToken() { + return requestToken; + } } diff --git a/aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/DeleteBucket.java b/aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/DeleteBucket.java index 3dad9262ba..bcd9bdd358 100644 --- a/aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/DeleteBucket.java +++ b/aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/DeleteBucket.java @@ -69,7 +69,8 @@ public class DeleteBucket extends S3FutureCommand { AWSResponseException responseException = (AWSResponseException) e.getCause(); if (responseException.getResponse().getStatusCode() == 404) { return true; - } else if ("BucketNotEmpty".equals(responseException.getError().getCode())) { + } else if ("BucketNotEmpty".equals(responseException.getError().getCode()) + || responseException.getResponse().getStatusCode() == 409) { return false; } } diff --git a/aws/s3/core/src/main/java/org/jclouds/aws/s3/config/LiveS3ConnectionModule.java b/aws/s3/core/src/main/java/org/jclouds/aws/s3/config/LiveS3ConnectionModule.java index 0d5eae6169..e5f7260299 100644 --- a/aws/s3/core/src/main/java/org/jclouds/aws/s3/config/LiveS3ConnectionModule.java +++ b/aws/s3/core/src/main/java/org/jclouds/aws/s3/config/LiveS3ConnectionModule.java @@ -38,6 +38,7 @@ import org.jclouds.http.HttpConstants; import org.jclouds.http.HttpErrorHandler; import org.jclouds.http.HttpRequestFilter; import org.jclouds.http.annotation.ClientError; +import org.jclouds.http.annotation.Redirection; import org.jclouds.http.annotation.ServerError; import org.jclouds.logging.Logger; @@ -78,6 +79,8 @@ public class LiveS3ConnectionModule extends AbstractModule { } protected void bindErrorHandlers() { + bind(HttpErrorHandler.class).annotatedWith(Redirection.class).to( + ParseAWSErrorFromXmlContent.class).in(Scopes.SINGLETON); bind(HttpErrorHandler.class).annotatedWith(ServerError.class).to( ParseAWSErrorFromXmlContent.class).in(Scopes.SINGLETON); bind(HttpErrorHandler.class).annotatedWith(ClientError.class).to( diff --git a/aws/s3/core/src/main/java/org/jclouds/aws/s3/handlers/ParseAWSErrorFromXmlContent.java b/aws/s3/core/src/main/java/org/jclouds/aws/s3/handlers/ParseAWSErrorFromXmlContent.java index 25431000b5..592680317d 100644 --- a/aws/s3/core/src/main/java/org/jclouds/aws/s3/handlers/ParseAWSErrorFromXmlContent.java +++ b/aws/s3/core/src/main/java/org/jclouds/aws/s3/handlers/ParseAWSErrorFromXmlContent.java @@ -23,20 +23,22 @@ */ package org.jclouds.aws.s3.handlers; -import java.io.InputStream; +import java.io.ByteArrayInputStream; import javax.annotation.Resource; -import org.apache.commons.io.IOUtils; import org.jclouds.aws.AWSResponseException; import org.jclouds.aws.domain.AWSError; import org.jclouds.aws.s3.filters.RequestAuthorizeSignature; import org.jclouds.aws.s3.reference.S3Headers; import org.jclouds.aws.s3.xml.S3ParserFactory; +import org.jclouds.http.HttpErrorHandler; +import org.jclouds.http.HttpException; import org.jclouds.http.HttpFutureCommand; import org.jclouds.http.HttpResponse; -import org.jclouds.http.HttpErrorHandler; +import org.jclouds.http.HttpResponseException; import org.jclouds.logging.Logger; +import org.jclouds.util.Utils; import com.google.inject.Inject; @@ -48,38 +50,51 @@ import com.google.inject.Inject; * */ public class ParseAWSErrorFromXmlContent implements HttpErrorHandler { - @Resource - protected Logger logger = Logger.NULL; + @Resource + protected Logger logger = Logger.NULL; - private final S3ParserFactory parserFactory; + private final S3ParserFactory parserFactory; - @Inject - public ParseAWSErrorFromXmlContent(S3ParserFactory parserFactory) { - this.parserFactory = parserFactory; - } + @Inject + public ParseAWSErrorFromXmlContent(S3ParserFactory parserFactory) { + this.parserFactory = parserFactory; + } - public void handleError(HttpFutureCommand command, HttpResponse response) { - AWSError error = new AWSError(); - error.setRequestId(response.getFirstHeaderOrNull(S3Headers.REQUEST_ID)); - error.setRequestToken(response - .getFirstHeaderOrNull(S3Headers.REQUEST_TOKEN)); - InputStream errorStream = response.getContent(); - try { - if (errorStream != null) { - error = parserFactory.createErrorParser().parse(errorStream); - if ("SignatureDoesNotMatch".equals(error.getCode())) - error.setStringSigned(RequestAuthorizeSignature - .createStringToSign(command.getRequest())); - error.setRequestToken(response - .getFirstHeaderOrNull(S3Headers.REQUEST_TOKEN)); - } - } catch (Exception e) { - logger.warn(e, "error parsing XML reponse: %1$s", response); - } finally { - command.setException(new AWSResponseException(command, response, - error)); - IOUtils.closeQuietly(errorStream); - } - } + public void handleError(HttpFutureCommand command, HttpResponse response) { + String content; + try { + content = response.getContent() != null ? Utils.toStringAndClose(response.getContent()) + : null; + if (content != null) { + try { + if (content.indexOf('<') >= 0) { + AWSError error = parseAWSErrorFromContent(command, response, content); + command.setException(new AWSResponseException(command, response, error)); + } else { + command.setException(new HttpResponseException(command, response, content)); + } + } catch (Exception he) { + command.setException(new HttpResponseException(command, response, content)); + Utils.rethrowIfRuntime(he); + } + } else { + command.setException(new HttpResponseException(command, response)); + } + } catch (Exception e) { + command.setException(new HttpResponseException(command, response)); + Utils.rethrowIfRuntime(e); + } + } + + private AWSError parseAWSErrorFromContent(HttpFutureCommand command, HttpResponse response, + String content) throws HttpException { + AWSError error = parserFactory.createErrorParser().parse( + new ByteArrayInputStream(content.getBytes())); + error.setRequestId(response.getFirstHeaderOrNull(S3Headers.REQUEST_ID)); + error.setRequestToken(response.getFirstHeaderOrNull(S3Headers.REQUEST_TOKEN)); + if ("SignatureDoesNotMatch".equals(error.getCode())) + error.setStringSigned(RequestAuthorizeSignature.createStringToSign(command.getRequest())); + return error; + } } \ No newline at end of file diff --git a/core/src/main/java/org/jclouds/command/pool/FutureCommandConnectionPoolClient.java b/core/src/main/java/org/jclouds/command/pool/FutureCommandConnectionPoolClient.java index 3dc658c9af..dbf6d43874 100644 --- a/core/src/main/java/org/jclouds/command/pool/FutureCommandConnectionPoolClient.java +++ b/core/src/main/java/org/jclouds/command/pool/FutureCommandConnectionPoolClient.java @@ -59,9 +59,14 @@ public class FutureCommandConnectionPoolClient>() { public FutureCommandConnectionPool apply(E endPoint) { - FutureCommandConnectionPool pool = poolFactory.create(endPoint); - addDependency(pool); - return pool; + try { + FutureCommandConnectionPool pool = poolFactory.create(endPoint); + addDependency(pool); + return pool; + } catch (RuntimeException e) { + logger.error(e, "error creating entry for %s", endPoint); + throw e; + } } }); this.commandQueue = commandQueue; @@ -124,7 +129,7 @@ public class FutureCommandConnectionPoolClient pool = poolMap.get(command.getRequest().getEndPoint()); if (pool == null) { - //TODO limit; + // TODO limit; logger.warn("pool not available for command %s; retrying", command); commandQueue.add(command); return; @@ -138,7 +143,8 @@ public class FutureCommandConnectionPoolClient command, HttpResponse response) { - IOUtils.closeQuietly(response.getContent()); + closeConnectionButKeepContentStream(response); + command.incrementRedirectCount(); String hostHeader = response.getFirstHeaderOrNull(HttpHeaders.LOCATION); if (hostHeader != null && command.getRedirectCount() < retryCountLimit) { - URI redirectURI = URI.create(hostHeader); - command.getRequest().setEndPoint(redirectURI); + URI endPoint = parseEndPoint(hostHeader); + command.getRequest().setEndPoint(endPoint); return true; } else { return false; } } + /** + * Content stream may need to be read. However, we should always close the http stream. + */ + @VisibleForTesting + void closeConnectionButKeepContentStream(HttpResponse response) { + if (response.getContent() != null) { + try { + byte[] data = IOUtils.toByteArray(response.getContent()); + response.setContent(new ByteArrayInputStream(data)); + } catch (IOException e) { + logger.error(e, "Error consuming input"); + } finally { + IOUtils.closeQuietly(response.getContent()); + } + } + } + + private URI parseEndPoint(String hostHeader) { + URI redirectURI = URI.create(hostHeader); + String scheme = redirectURI.getScheme(); + + checkState(redirectURI.getScheme().startsWith("http"), String.format( + "header %s didn't parse an http scheme: [%s]", hostHeader, scheme)); + int port = redirectURI.getPort() > 0 ? redirectURI.getPort() : redirectURI.getScheme() + .equals("https") ? 443 : 80; + String host = redirectURI.getHost(); + checkState(!host.matches("[/]"), String.format( + "header %s didn't parse an http host correctly: [%s]", hostHeader, host)); + URI endPoint = URI.create(String.format("%s://%s:%d", scheme, host, port)); + return endPoint; + } } diff --git a/core/src/main/java/org/jclouds/util/Utils.java b/core/src/main/java/org/jclouds/util/Utils.java index 583052bdbb..b92bc6fe56 100644 --- a/core/src/main/java/org/jclouds/util/Utils.java +++ b/core/src/main/java/org/jclouds/util/Utils.java @@ -36,7 +36,7 @@ import org.jclouds.logging.Logger; /** * // TODO: Adrian: Document this! - * + * * @author Adrian Cole */ public class Utils { @@ -45,135 +45,155 @@ public class Utils { @Resource protected static Logger logger = Logger.NULL; - /** - * - * @param Exception type you'd like rethrown - * @param e Exception you are inspecting - * @throws E - */ - @SuppressWarnings("unchecked") - public static void rethrowIfRuntimeOrSameType(Exception e) throws E { - if (e instanceof ExecutionException) { - Throwable nested = e.getCause(); - if (nested instanceof Error) - throw (Error) nested; - e = (Exception) nested; - } + /** + * + * @param + * Exception type you'd like rethrown + * @param e + * Exception you are inspecting + * @throws E + */ + public static void rethrowIfRuntime(Exception e) { + if (e instanceof ExecutionException) { + Throwable nested = e.getCause(); + if (nested instanceof Error) + throw (Error) nested; + e = (Exception) nested; + } - if (e instanceof RuntimeException) { - throw (RuntimeException) e; - } else { - try { - throw (E) e; - } catch (ClassCastException throwAway) { - // using cce as there's no way to do instanceof E in current java - } - } - } + if (e instanceof RuntimeException) { + throw (RuntimeException) e; + } + } - public static String toStringAndClose(InputStream input) throws IOException { - try { - return IOUtils.toString(input); - } finally { - IOUtils.closeQuietly(input); - } - } + /** + * + * @param + * Exception type you'd like rethrown + * @param e + * Exception you are inspecting + * @throws E + */ + @SuppressWarnings("unchecked") + public static void rethrowIfRuntimeOrSameType(Exception e) throws E { + if (e instanceof ExecutionException) { + Throwable nested = e.getCause(); + if (nested instanceof Error) + throw (Error) nested; + e = (Exception) nested; + } - /** - * Encode the given string with the given encoding, if possible. - * If the encoding fails with {@link UnsupportedEncodingException}, - * log a warning and fall back to the system's default encoding. - * - * @see {@link String#getBytes(String)} - * @see {@link String#getBytes()} - used as fall-back. - * - * @param str - * @param encoding - * @return - */ - public static byte[] encodeString(String str, String encoding) { - try { - return str.getBytes(encoding); - } catch (UnsupportedEncodingException e) { - logger.warn(e, "Failed to encode string to bytes with encoding " + encoding - + ". Falling back to system's default encoding"); - return str.getBytes(); - } - } - - /** - * Encode the given string with the UTF-8 encoding, the sane default. - * In the very unlikely event the encoding fails with - * {@link UnsupportedEncodingException}, log a warning and fall back - * to the system's default encoding. - * - * @param str - * @return - */ - public static byte[] encodeString(String str) { - return encodeString(str, UTF8_ENCODING); - } - - /** - * Decode the given string with the given encoding, if possible. - * If the decoding fails with {@link UnsupportedEncodingException}, - * log a warning and fall back to the system's default encoding. - * - * @param bytes - * @param encoding - * @return - */ - public static String decodeString(byte[] bytes, String encoding) { - try { + if (e instanceof RuntimeException) { + throw (RuntimeException) e; + } else { + try { + throw (E) e; + } catch (ClassCastException throwAway) { + // using cce as there's no way to do instanceof E in current java + } + } + } + + public static String toStringAndClose(InputStream input) throws IOException { + try { + return IOUtils.toString(input); + } finally { + IOUtils.closeQuietly(input); + } + } + + /** + * Encode the given string with the given encoding, if possible. If the encoding fails with + * {@link UnsupportedEncodingException}, log a warning and fall back to the system's default + * encoding. + * + * @see {@link String#getBytes(String)} + * @see {@link String#getBytes()} - used as fall-back. + * + * @param str + * @param encoding + * @return + */ + public static byte[] encodeString(String str, String encoding) { + try { + return str.getBytes(encoding); + } catch (UnsupportedEncodingException e) { + logger.warn(e, "Failed to encode string to bytes with encoding " + encoding + + ". Falling back to system's default encoding"); + return str.getBytes(); + } + } + + /** + * Encode the given string with the UTF-8 encoding, the sane default. In the very unlikely event + * the encoding fails with {@link UnsupportedEncodingException}, log a warning and fall back to + * the system's default encoding. + * + * @param str + * @return + */ + public static byte[] encodeString(String str) { + return encodeString(str, UTF8_ENCODING); + } + + /** + * Decode the given string with the given encoding, if possible. If the decoding fails with + * {@link UnsupportedEncodingException}, log a warning and fall back to the system's default + * encoding. + * + * @param bytes + * @param encoding + * @return + */ + public static String decodeString(byte[] bytes, String encoding) { + try { return new String(bytes, encoding); } catch (UnsupportedEncodingException e) { logger.warn(e, "Failed to decode bytes to string with encoding " + encoding - + ". Falling back to system's default encoding"); + + ". Falling back to system's default encoding"); return new String(bytes); } - } + } - /** - * Decode the given string with the UTF-8 encoding, the sane default. - * In the very unlikely event the encoding fails with - * {@link UnsupportedEncodingException}, log a warning and fall back - * to the system's default encoding. - * - * @param str - * @return - */ - public static String decodeString(byte[] bytes) { - return decodeString(bytes, UTF8_ENCODING); - } + /** + * Decode the given string with the UTF-8 encoding, the sane default. In the very unlikely event + * the encoding fails with {@link UnsupportedEncodingException}, log a warning and fall back to + * the system's default encoding. + * + * @param str + * @return + */ + public static String decodeString(byte[] bytes) { + return decodeString(bytes, UTF8_ENCODING); + } - /** - * Encode a path portion of a URI using the UTF-8 encoding, but leave slash '/' - * characters and any parameters (anything after the first '?') un-encoded. - * If encoding with UTF-8 fails, the method falls back to using the - * system's default encoding. - * - * @param uri - * @return - */ - @SuppressWarnings("deprecation") - public static String encodeUriPath(String uri) { - String path, params = ""; - - int offset; - if ((offset = uri.indexOf('?')) >= 0) { - path = uri.substring(0, offset); - params = uri.substring(offset); - } else { - path = uri; - } - - String encodedUri; - try { - encodedUri = URLEncoder.encode(path, "UTF-8"); - } catch (UnsupportedEncodingException e) { - encodedUri = URLEncoder.encode(path); - } - return encodedUri.replace("%2F", "/") + params; - } + /** + * Encode a path portion of a URI using the UTF-8 encoding, but leave slash '/' characters and + * any parameters (anything after the first '?') un-encoded. If encoding with UTF-8 fails, the + * method falls back to using the system's default encoding. + * + * @param uri + * @return + */ + @SuppressWarnings("deprecation") + public static String encodeUriPath(String uri) { + String path, params = ""; + + int offset; + if ((offset = uri.indexOf('?')) >= 0) { + path = uri.substring(0, offset); + params = uri.substring(offset); + } else { + path = uri; + } + + String encodedUri; + try { + encodedUri = URLEncoder.encode(path, "UTF-8"); + } catch (UnsupportedEncodingException e) { + encodedUri = URLEncoder.encode(path); + } + return encodedUri.replace("%2F", "/") + params; + } } diff --git a/extensions/gae/src/main/java/org/jclouds/gae/URLFetchServiceClient.java b/extensions/gae/src/main/java/org/jclouds/gae/URLFetchServiceClient.java index f03c5e42fa..250c79d99a 100644 --- a/extensions/gae/src/main/java/org/jclouds/gae/URLFetchServiceClient.java +++ b/extensions/gae/src/main/java/org/jclouds/gae/URLFetchServiceClient.java @@ -155,7 +155,8 @@ public class URLFetchServiceClient extends BaseHttpFutureCommandClient