Issue 70 sorting out url encoding

git-svn-id: http://jclouds.googlecode.com/svn/trunk@1483 3d8758e0-26b5-11de-8745-db77d3ebf521

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/CopyObject.java

@@ -24,6 +24,7 @@
 package org.jclouds.aws.s3.commands;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
 
 import java.net.URI;
 
@@ -32,7 +33,6 @@ import org.jclouds.aws.s3.domain.S3Object;
 import org.jclouds.aws.s3.xml.CopyObjectHandler;
 import org.jclouds.http.HttpMethod;
 import org.jclouds.http.commands.callables.xml.ParseSax;
-import org.jclouds.util.Utils;
 
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
@@ -64,14 +64,15 @@ public class CopyObject extends S3FutureCommand<S3Object.Metadata> {
             @Assisted("destinationBucket") String destinationBucket,
             @Assisted("destinationObject") String destinationObject,
             @Assisted CopyObjectOptions options) {
-      super(endPoint, HttpMethod.PUT, "/" + checkNotNull(destinationObject, "destinationObject"),
+      super(endPoint, HttpMethod.PUT, "/"
-               callable, destinationBucket);
+               + urlEncode(checkNotNull(destinationObject, "destinationObject")), callable,
+               destinationBucket);
       CopyObjectHandler handler = (CopyObjectHandler) callable.getHandler();
       handler.setKey(destinationObject);
       getRequest().getHeaders().put(
                "x-amz-copy-source",
-               String.format("/%1$s/%2$s", checkNotNull(sourceBucket, "sourceBucket"), Utils
+               String.format("/%1$s/%2$s", checkNotNull(sourceBucket, "sourceBucket"),
-                        .encodeUriPath(checkNotNull(sourceObject, "sourceObject"))));
+                        urlEncode(checkNotNull(sourceObject, "sourceObject"))));
       getRequest().getHeaders().putAll(options.buildRequestHeaders());
    }
 }

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/DeleteObject.java

@@ -24,6 +24,7 @@
 package org.jclouds.aws.s3.commands;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
 
 import java.net.URI;
 
@@ -46,6 +47,6 @@ public class DeleteObject extends S3FutureCommand<Boolean> {
    @Inject
    public DeleteObject(URI endPoint, ReturnTrueIf2xx callable,
             @Assisted("bucketName") String bucket, @Assisted("key") String key) {
-      super(endPoint, HttpMethod.DELETE, "/" + checkNotNull(key), callable, bucket);
+      super(endPoint, HttpMethod.DELETE, "/" + urlEncode(checkNotNull(key)), callable, bucket);
    }
 }

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/GetAccessControlList.java

@@ -23,6 +23,9 @@
  */
 package org.jclouds.aws.s3.commands;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
+
 import java.net.URI;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
@@ -57,7 +60,8 @@ public class GetAccessControlList extends S3FutureCommand<AccessControlList> {
    @Inject
    public GetAccessControlList(URI endPoint, ParseSax<AccessControlList> accessControlListParser,
             @Assisted("bucketName") String bucket, @Assisted("objectKey") String objectKey) {
-      super(endPoint, HttpMethod.GET, "/" + objectKey + "?acl", accessControlListParser, bucket);
+      super(endPoint, HttpMethod.GET, "/" + urlEncode(checkNotNull(objectKey)) + "?acl",
+               accessControlListParser, bucket);
    }
 
    @Override

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/GetObject.java

@@ -24,6 +24,7 @@
 package org.jclouds.aws.s3.commands;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
 
 import java.net.URI;
 import java.util.concurrent.ExecutionException;
@@ -69,7 +70,7 @@ public class GetObject extends S3FutureCommand<S3Object> {
    public GetObject(URI endPoint, ParseObjectFromHeadersAndHttpContent callable,
             @Assisted("bucketName") String s3Bucket, @Assisted("key") String key,
             @Assisted GetObjectOptions options) {
-      super(endPoint, HttpMethod.GET, "/" + checkNotNull(key), callable, s3Bucket);
+      super(endPoint, HttpMethod.GET, "/" + urlEncode(checkNotNull(key)), callable, s3Bucket);
       this.getRequest().getHeaders().putAll(options.buildRequestHeaders());
       callable.setKey(key);
    }

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/HeadObject.java

@@ -24,6 +24,7 @@
 package org.jclouds.aws.s3.commands;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
 
 import java.net.URI;
 import java.util.concurrent.ExecutionException;
@@ -60,7 +61,7 @@ public class HeadObject extends S3FutureCommand<S3Object.Metadata> {
    @Inject
    public HeadObject(URI endPoint, ParseMetadataFromHeaders callable,
             @Assisted("bucketName") String bucket, @Assisted("key") String key) {
-      super(endPoint, HttpMethod.HEAD, "/" + checkNotNull(key), callable, bucket);
+      super(endPoint, HttpMethod.HEAD, "/" + urlEncode(checkNotNull(key)), callable, bucket);
       callable.setKey(key);
    }
 

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/PutObject.java

@@ -25,6 +25,7 @@ package org.jclouds.aws.s3.commands;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
 
 import java.net.URI;
 
@@ -59,7 +60,8 @@ public class PutObject extends S3FutureCommand<byte[]> {
    @Inject
    public PutObject(URI endPoint, ParseMd5FromETagHeader callable, @Assisted String s3Bucket,
             @Assisted S3Object object, @Assisted PutObjectOptions options) {
-      super(endPoint, HttpMethod.PUT, "/" + checkNotNull(object.getKey()), callable, s3Bucket);
+      super(endPoint, HttpMethod.PUT, "/" + urlEncode(checkNotNull(object.getKey())), callable,
+               s3Bucket);
       checkArgument(object.getMetadata().getSize() >= 0, "size must be set");
 
       getRequest().setPayload(checkNotNull(object.getData(), "object.getContent()"));

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/PutObjectAccessControlList.java

@@ -23,6 +23,9 @@
  */
 package org.jclouds.aws.s3.commands;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
+
 import java.net.URI;
 
 import javax.annotation.Resource;
@@ -54,7 +57,7 @@ public class PutObjectAccessControlList extends S3FutureCommand<Boolean> {
    public PutObjectAccessControlList(URI endPoint, ReturnTrueIf2xx callable,
             @Assisted("bucketName") String bucket, @Assisted("key") String objectKey,
             @Assisted AccessControlList acl) {
-      super(endPoint, HttpMethod.PUT, "/" + objectKey + "?acl", callable, bucket);
+      super(endPoint, HttpMethod.PUT, "/" + urlEncode(checkNotNull(objectKey)) + "?acl", callable, bucket);
 
       String aclPayload = "";
       try {

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/options/CopyObjectOptions.java

@@ -211,8 +211,6 @@ public class CopyObjectOptions extends BaseHttpRequestOptions {
     * 
     * @param md5
     *           hash representing the entity
-    * @throws UnsupportedEncodingException
-    *            if there was a problem converting this into an S3 eTag string
     */
    public CopyObjectOptions ifSourceMd5Matches(byte[] md5) throws UnsupportedEncodingException {
       checkState(getIfNoneMatch() == null,

aws/s3/core/src/main/java/org/jclouds/aws/s3/commands/options/ListBucketOptions.java

@@ -25,16 +25,14 @@ package org.jclouds.aws.s3.commands.options;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
-import org.jclouds.http.options.BaseHttpRequestOptions;
 
-import java.io.UnsupportedEncodingException;
+import org.jclouds.http.options.BaseHttpRequestOptions;
-import java.net.URLEncoder;
 
 /**
  * Contains options supported in the REST API for the GET bucket operation. <h2>
- * Usage</h2> The recommended way to instantiate a GetBucketOptions object is to
+ * Usage</h2> The recommended way to instantiate a GetBucketOptions object is to statically import
- * statically import GetBucketOptions.Builder.* and invoke a static creation
+ * GetBucketOptions.Builder.* and invoke a static creation method followed by an instance mutator
- * method followed by an instance mutator (if needed):
+ * (if needed):
  * <p/>
  * <code>
  * import static org.jclouds.aws.s3.commands.options.GetBucketOptions.Builder.*
@@ -44,23 +42,21 @@ import java.net.URLEncoder;
  * <code>
  * 
  * @author Adrian Cole
- * @see <a href="http://docs.amazonwebservices.com/AmazonS3/2006-03-01/index.html?RESTBucketGET.html?"
+ * @see <a
+ *      href="http://docs.amazonwebservices.com/AmazonS3/2006-03-01/index.html?RESTBucketGET.html?"
  *      />
  */
 public class ListBucketOptions extends BaseHttpRequestOptions {
    public static final ListBucketOptions NONE = new ListBucketOptions();
 
    /**
-     * Limits the response to keys which begin with the indicated prefix. You
+    * Limits the response to keys which begin with the indicated prefix. You can use prefixes to
-     * can use prefixes to separate a bucket into different sets of keys in a
+    * separate a bucket into different sets of keys in a way similar to how a file system uses
-     * way similar to how a file system uses folders.
+    * folders.
     * 
-     * @throws UnsupportedEncodingException
     */
-    public ListBucketOptions withPrefix(String prefix)
+   public ListBucketOptions withPrefix(String prefix) {
-            throws UnsupportedEncodingException {
+      parameters.put("prefix", checkNotNull(prefix, "prefix"));
-        parameters.put("prefix", URLEncoder.encode(checkNotNull(prefix, "prefix"),
-                "UTF-8"));
       return this;
    }
 
@@ -72,17 +68,12 @@ public class ListBucketOptions extends BaseHttpRequestOptions {
    }
 
    /**
-     * Indicates where in the bucket to begin listing. The list will only
+    * Indicates where in the bucket to begin listing. The list will only include keys that occur
-     * include keys that occur lexicographically after marker. This is
+    * lexicographically after marker. This is convenient for pagination: To get the next page of
-     * convenient for pagination: To get the next page of results use the last
+    * results use the last key of the current page as the marker.
-     * key of the current page as the marker.
-     *
-     * @throws UnsupportedEncodingException
     */
-    public ListBucketOptions afterMarker(String marker)
+   public ListBucketOptions afterMarker(String marker) {
-            throws UnsupportedEncodingException {
+      parameters.put("marker", checkNotNull(marker, "marker"));
-        parameters.put("marker", URLEncoder.encode(checkNotNull(marker, "marker"),
-                "UTF-8"));
       return this;
    }
 
@@ -94,8 +85,8 @@ public class ListBucketOptions extends BaseHttpRequestOptions {
    }
 
    /**
-     * The maximum number of keys you'd like to see in the response body. The
+    * The maximum number of keys you'd like to see in the response body. The server might return
-     * server might return fewer than this many keys, but will not return more.
+    * fewer than this many keys, but will not return more.
     */
    public ListBucketOptions maxResults(long maxKeys) {
       checkState(maxKeys >= 0, "maxKeys must be >= 0");
@@ -111,17 +102,13 @@ public class ListBucketOptions extends BaseHttpRequestOptions {
    }
 
    /**
-     * Causes keys that contain the same string between the prefix and the first
+    * Causes keys that contain the same string between the prefix and the first occurrence of the
-     * occurrence of the delimiter to be rolled up into a single result element
+    * delimiter to be rolled up into a single result element in the CommonPrefixes collection. These
-     * in the CommonPrefixes collection. These rolled-up keys are not returned
+    * rolled-up keys are not returned elsewhere in the response.
-     * elsewhere in the response.
     * 
-     * @throws UnsupportedEncodingException
     */
-    public ListBucketOptions delimiter(String delimiter)
+   public ListBucketOptions delimiter(String delimiter) {
-            throws UnsupportedEncodingException {
+      parameters.put("delimiter", checkNotNull(delimiter, "delimiter"));
-        parameters.put("delimiter", URLEncoder.encode(checkNotNull(delimiter,
-                "delimiter"), "UTF-8"));
       return this;
    }
 
@@ -135,21 +122,17 @@ public class ListBucketOptions extends BaseHttpRequestOptions {
    public static class Builder {
 
       /**
-         * @throws UnsupportedEncodingException
        * @see ListBucketOptions#withPrefix(String)
        */
-        public static ListBucketOptions withPrefix(String prefix)
+      public static ListBucketOptions withPrefix(String prefix) {
-                throws UnsupportedEncodingException {
          ListBucketOptions options = new ListBucketOptions();
          return options.withPrefix(prefix);
       }
 
       /**
-         * @throws UnsupportedEncodingException
        * @see ListBucketOptions#afterMarker(String)
        */
-        public static ListBucketOptions afterMarker(String marker)
+      public static ListBucketOptions afterMarker(String marker) {
-                throws UnsupportedEncodingException {
          ListBucketOptions options = new ListBucketOptions();
          return options.afterMarker(marker);
       }
@@ -163,11 +146,9 @@ public class ListBucketOptions extends BaseHttpRequestOptions {
       }
 
       /**
-         * @throws UnsupportedEncodingException
        * @see ListBucketOptions#delimiter(String)
        */
-        public static ListBucketOptions delimiter(String delimiter)
+      public static ListBucketOptions delimiter(String delimiter) {
-                throws UnsupportedEncodingException {
          ListBucketOptions options = new ListBucketOptions();
          return options.delimiter(delimiter);
       }

aws/s3/core/src/main/java/org/jclouds/aws/s3/util/S3Utils.java

@@ -25,6 +25,12 @@ package org.jclouds.aws.s3.util;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.output.ByteArrayOutputStream;
 import org.bouncycastle.crypto.digests.MD5Digest;
@@ -38,8 +44,6 @@ import org.jclouds.http.HttpException;
 import org.jclouds.http.HttpFutureCommand;
 import org.jclouds.http.HttpResponse;
 
-import java.io.*;
-
 /**
  * Encryption, Hashing, and IO Utilities needed to sign and verify S3 requests and responses.
  * 

aws/s3/core/src/test/java/org/jclouds/aws/s3/S3IntegrationTest.java

@@ -232,7 +232,6 @@ public class S3IntegrationTest {
          deleteBucket(scratchBucket);
          String newScratchBucket = bucketPrefix + (++bucketIndex);
          createBucketAndEnsureEmpty(newScratchBucket);
-         Thread.sleep(INCONSISTENCY_WINDOW);
          returnBucket(newScratchBucket);
       }
    }
@@ -317,7 +316,6 @@ public class S3IntegrationTest {
       deleteBucket(bucketName);
       client.putBucketIfNotExists(bucketName, createIn(LocationConstraint.EU)).get(10,
                TimeUnit.SECONDS);
-      Thread.sleep(INCONSISTENCY_WINDOW);
       return bucketName;
    }
 

aws/s3/core/src/test/java/org/jclouds/aws/s3/commands/DeleteObjectIntegrationTest.java

@@ -54,6 +54,39 @@ public class DeleteObjectIntegrationTest extends S3IntegrationTest {
       }
    }
    
+   @Test
+   void deleteObjectWithSpaces() throws Exception {
+      String bucketName = getBucketName();
+      try {
+         addObjectToBucket(bucketName, "p blic-read-acl");
+         assert client.deleteObject(bucketName, "p blic-read-acl").get(10, TimeUnit.SECONDS);
+      } finally {
+         returnBucket(bucketName);
+      }
+   }
+   
+   @Test
+   void deleteObjectUnicade() throws Exception {
+      String bucketName = getBucketName();
+      try {
+         addObjectToBucket(bucketName, "p¿blic-read-acl");
+         assert client.deleteObject(bucketName, "p¿blic-read-acl").get(10, TimeUnit.SECONDS);
+      } finally {
+         returnBucket(bucketName);
+      }
+   }
+   
+   @Test
+   void deleteObjectQuestion() throws Exception {
+      String bucketName = getBucketName();
+      try {
+         addObjectToBucket(bucketName, "p???blic-read-acl");
+         assert client.deleteObject(bucketName, "p???blic-read-acl").get(10, TimeUnit.SECONDS);
+      } finally {
+         returnBucket(bucketName);
+      }
+   }
+   
    @Test
    void deleteObjectNoBucket() throws Exception {
       try {

core/src/main/java/org/jclouds/http/HttpRequest.java

@@ -32,7 +32,6 @@ import javax.annotation.Resource;
 
 import org.jclouds.command.Request;
 import org.jclouds.logging.Logger;
-import org.jclouds.util.Utils;
 
 /**
  * Represents a request that can be executed within {@link HttpFutureCommandClient}
@@ -62,7 +61,7 @@ public class HttpRequest extends HttpMessage implements Request<URI> {
    public HttpRequest(URI endPoint, HttpMethod method, String uri) {
       this.endPoint = checkNotNull(endPoint, "endPoint");
       this.method = checkNotNull(method, "method");
-      this.uri = Utils.encodeUriPath(checkNotNull(uri, "uri"));
+      this.uri = checkNotNull(uri, "uri");
    }
 
    @Override

core/src/main/java/org/jclouds/http/options/BaseHttpRequestOptions.java

@@ -24,6 +24,7 @@
 package org.jclouds.http.options;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.jclouds.util.Utils.urlEncode;
 
 import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
@@ -86,7 +87,8 @@ public class BaseHttpRequestOptions implements HttpRequestOptions {
          builder.append("?");
          for (Iterator<Entry<String, String>> i = parameters.entrySet().iterator(); i.hasNext();) {
             Entry<String, String> entry = i.next();
-            builder.append(entry.getKey()).append("=").append(entry.getValue());
+            builder.append(urlEncode(entry.getKey())).append("=").append(
+                     urlEncode(entry.getValue()));
             if (i.hasNext())
                builder.append("&");
          }

core/src/main/java/org/jclouds/util/Utils.java

@@ -50,6 +50,14 @@ public class Utils {
    @Resource
    protected static Logger logger = Logger.NULL;
 
+   public static String urlEncode(String in) {
+      try {
+         return URLEncoder.encode(in, "UTF-8");
+      } catch (UnsupportedEncodingException e) {
+         throw new IllegalStateException("Bad encoding on input: " + in, e);
+      }
+   }
+
    /**
     * Content stream may need to be read. However, we should always close the http stream.
     */
@@ -209,33 +217,4 @@ public class Utils {
       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;
-   }
-
 }