Merge pull request #684 from whitlockjc/master

Issue 992: Fix AWS S3 to work with non-DNS, but still valid, named buckets.

apis/s3/src/main/java/org/jclouds/s3/binders/BindAsHostPrefixIfConfigured.java

@@ -63,7 +63,14 @@ public class BindAsHostPrefixIfConfigured implements Binder {
    @SuppressWarnings("unchecked")
    @Override
    public <R extends HttpRequest> R bindToRequest(R request, Object payload) {
-      if (isVhostStyle) {
+      // If we have a payload/bucket/container that is not all lowercase, vhost-style URLs are not an option and must be
+      // automatically converted to their path-based equivalent.  This should only be possible for AWS-S3 since it is
+      // the only S3 implementation configured to allow uppercase payload/bucket/container names.
+      //
+      // http://code.google.com/p/jclouds/issues/detail?id=992
+      String payloadAsString = payload.toString();
+
+      if (isVhostStyle && payloadAsString.equals(payloadAsString.toLowerCase())) {
          request = bindAsHostPrefix.bindToRequest(request, payload);
          String host = request.getEndpoint().getHost();
          if (request.getEndpoint().getPort() != -1) {
@@ -80,7 +87,7 @@ public class BindAsHostPrefixIfConfigured implements Binder {
             indexToInsert = indexToInsert == -1 ? 0 : indexToInsert;
             indexToInsert += servicePath.length();
          }
-         path.insert(indexToInsert, "/" + payload.toString());
+         path.insert(indexToInsert, "/" + payloadAsString);
          builder.replacePath(path.toString());
          return (R) request.toBuilder().endpoint(builder.buildFromEncodedMap(ImmutableMap.<String, Object> of()))
                   .build();

apis/s3/src/main/java/org/jclouds/s3/filters/RequestAuthorizeSignature.java

@@ -33,8 +33,8 @@ import java.lang.annotation.Annotation;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Locale;
-import java.util.Set;
 import java.util.Map.Entry;
+import java.util.Set;
 
 import javax.annotation.Resource;
 import javax.inject.Inject;
@@ -161,7 +161,6 @@ public class RequestAuthorizeSignature implements HttpRequestFilter, RequestSign
       }
 
       appendAmzHeaders(canonicalizedHeaders, buffer);
-      if (isVhostStyle)
       appendBucketName(request, buffer);
       appendUriPath(request, buffer);
       if (signatureWire.enabled())
@@ -232,19 +231,14 @@ public class RequestAuthorizeSignature implements HttpRequestFilter, RequestSign
 
    @VisibleForTesting
    void appendBucketName(HttpRequest req, StringBuilder toSign) {
-      checkArgument(req instanceof GeneratedHttpRequest<?>, "this should be a generated http request");
+      String bucketName = getBucketName(req);
-      GeneratedHttpRequest<?> request = GeneratedHttpRequest.class.cast(req);
 
-      String bucketName = null;
+      // If we have a payload/bucket/container that is not all lowercase, vhost-style URLs are not an option and must be
-
+      // automatically converted to their path-based equivalent.  This should only be possible for AWS-S3 since it is
-      for (int i = 0; i < request.getJavaMethod().getParameterAnnotations().length; i++) {
+      // the only S3 implementation configured to allow uppercase payload/bucket/container names.
-         if (any(Arrays.asList(request.getJavaMethod().getParameterAnnotations()[i]), ANNOTATIONTYPE_BUCKET)) {
+      //
-            bucketName = (String) request.getArgs().get(i);
+      // http://code.google.com/p/jclouds/issues/detail?id=992
-            break;
+      if (isVhostStyle && bucketName!= null && bucketName.equals(bucketName.toLowerCase()))
-         }
-      }
-
-      if (bucketName != null)
          toSign.append(servicePath).append(bucketName);
    }
 
@@ -271,4 +265,21 @@ public class RequestAuthorizeSignature implements HttpRequestFilter, RequestSign
          }
       }
    }
+
+   private String getBucketName(HttpRequest req) {
+      checkArgument(req instanceof GeneratedHttpRequest<?>, "this should be a generated http request");
+      GeneratedHttpRequest<?> request = GeneratedHttpRequest.class.cast(req);
+
+      String bucketName = null;
+
+      for (int i = 0; i < request.getJavaMethod().getParameterAnnotations().length; i++) {
+         if (any(Arrays.asList(request.getJavaMethod().getParameterAnnotations()[i]), ANNOTATIONTYPE_BUCKET)) {
+            bucketName = (String) request.getArgs().get(i);
+            break;
+         }
+      }
+
+      return bucketName;
+   }
+
 }

apis/s3/src/main/java/org/jclouds/s3/handlers/ParseS3ErrorFromXmlContent.java

@@ -25,12 +25,14 @@ import static com.google.common.collect.Lists.newArrayList;
 import static org.jclouds.s3.reference.S3Constants.PROPERTY_S3_SERVICE_PATH;
 import static org.jclouds.s3.reference.S3Constants.PROPERTY_S3_VIRTUAL_HOST_BUCKETS;
 
+import java.net.URI;
 import java.util.List;
-
 import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import com.google.common.base.Joiner;
+import com.google.common.base.Splitter;
 import org.jclouds.aws.domain.AWSError;
 import org.jclouds.aws.handlers.ParseAWSErrorFromXmlContent;
 import org.jclouds.aws.util.AWSUtils;
@@ -39,9 +41,7 @@ import org.jclouds.blobstore.KeyNotFoundException;
 import org.jclouds.http.HttpCommand;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.rest.ResourceNotFoundException;
-
+import org.jclouds.s3.S3ApiMetadata;
-import com.google.common.base.Joiner;
-import com.google.common.base.Splitter;
 
 /**
  * @author Adrian Cole
@@ -54,7 +54,7 @@ public class ParseS3ErrorFromXmlContent extends ParseAWSErrorFromXmlContent {
    private final boolean isVhostStyle;
 
    @Inject
-   ParseS3ErrorFromXmlContent(AWSUtils utils, @Named(PROPERTY_S3_VIRTUAL_HOST_BUCKETS) boolean isVhostStyle,
+   public ParseS3ErrorFromXmlContent(AWSUtils utils, @Named(PROPERTY_S3_VIRTUAL_HOST_BUCKETS) boolean isVhostStyle,
             @Named(PROPERTY_S3_SERVICE_PATH) String servicePath) {
       super(utils);
       this.servicePath = servicePath;
@@ -66,8 +66,19 @@ public class ParseS3ErrorFromXmlContent extends ParseAWSErrorFromXmlContent {
       switch (response.getStatusCode()) {
          case 404:
             if (!command.getCurrentRequest().getMethod().equals("DELETE")) {
+               // If we have a payload/bucket/container that is not all lowercase, vhost-style URLs are not an option
+               // and must be automatically converted to their path-based equivalent.  This should only be possible for
+               // AWS-S3 since it is the only S3 implementation configured to allow uppercase payload/bucket/container
+               // names.
+               //
+               // http://code.google.com/p/jclouds/issues/detail?id=992
+               URI defaultS3Endpoint = URI.create(new S3ApiMetadata().getDefaultEndpoint().get());
+               URI requestEndpoint = command.getCurrentRequest().getEndpoint();
+               boolean wasPathBasedRequest = requestEndpoint.getHost().contains(defaultS3Endpoint.getHost()) &&
+                     requestEndpoint.getHost().equals(defaultS3Endpoint.getHost());
+
                exception = new ResourceNotFoundException(message, exception);
-               if (isVhostStyle) {
+               if (isVhostStyle && !wasPathBasedRequest) {
                   String container = command.getCurrentRequest().getEndpoint().getHost();
                   String key = command.getCurrentRequest().getEndpoint().getPath();
                   if (key == null || key.equals("/"))

apis/s3/src/main/java/org/jclouds/s3/predicates/validators/BucketNameValidator.java

@@ -40,7 +40,7 @@ import com.google.inject.Singleton;
 public class BucketNameValidator extends DnsNameValidator {
 
    @Inject
-   BucketNameValidator() {
+   public BucketNameValidator() {
       super(3, 63);
    }
 

providers/aws-s3/src/main/java/org/jclouds/aws/s3/AWSS3AsyncClient.java

@@ -21,7 +21,6 @@ package org.jclouds.aws.s3;
 import static org.jclouds.blobstore.attr.BlobScopes.CONTAINER;
 
 import java.util.Map;
-
 import javax.ws.rs.DELETE;
 import javax.ws.rs.GET;
 import javax.ws.rs.POST;
@@ -68,7 +67,7 @@ import com.google.common.util.concurrent.ListenableFuture;
 /**
  * Provides access to amazon-specific S3 features
  * 
- * @author Adrian Cole
+ * @author Adrian Cole, Jeremy Whitlock
  */
 @SkipEncoding('/')
 @RequestFilters(RequestAuthorizeSignature.class)

providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java

@@ -23,13 +23,13 @@ import static org.jclouds.location.reference.LocationConstants.ENDPOINT;
 import static org.jclouds.location.reference.LocationConstants.PROPERTY_REGION;
 
 import java.net.URI;
-
 import javax.inject.Named;
 import javax.inject.Singleton;
 
 import org.jclouds.aws.s3.AWSS3AsyncClient;
 import org.jclouds.aws.s3.AWSS3Client;
 import org.jclouds.aws.s3.binders.AssignCorrectHostnameAndBindAsHostPrefixIfConfigured;
+import org.jclouds.aws.s3.predicates.validators.AWSS3BucketNameValidator;
 import org.jclouds.location.Region;
 import org.jclouds.rest.ConfiguresRestClient;
 import org.jclouds.rest.RestContext;
@@ -38,6 +38,7 @@ import org.jclouds.s3.S3AsyncClient;
 import org.jclouds.s3.S3Client;
 import org.jclouds.s3.binders.BindAsHostPrefixIfConfigured;
 import org.jclouds.s3.config.S3RestClientModule;
+import org.jclouds.s3.predicates.validators.BucketNameValidator;
 
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
@@ -64,6 +65,7 @@ public class AWSS3RestClientModule extends S3RestClientModule<AWSS3Client, AWSS3
    @Override
    protected void configure() {
       bind(BindAsHostPrefixIfConfigured.class).to(AssignCorrectHostnameAndBindAsHostPrefixIfConfigured.class);
+      bind(BucketNameValidator.class).to(AWSS3BucketNameValidator.class);
       super.configure();
    }
    

providers/aws-s3/src/main/java/org/jclouds/aws/s3/predicates/validators/AWSS3BucketNameValidator.java

@@ -0,0 +1,57 @@
+/**
+ * Licensed to jclouds, Inc. (jclouds) under one or more
+ * contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  jclouds licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.jclouds.aws.s3.predicates.validators;
+
+import javax.inject.Inject;
+
+import org.jclouds.s3.predicates.validators.BucketNameValidator;
+
+import com.google.inject.Singleton;
+
+/**
+ * Validates name for AWS S3 buckets. The complete requirements are listed at:
+ * http://docs.amazonwebservices.com/AmazonS3/latest/index.html?BucketRestrictions.html
+ * 
+ * @see org.jclouds.rest.InputParamValidator
+ * @see org.jclouds.predicates.Validator
+ * 
+ * @author Adrian Cole, Jeremy Whitlock
+ */
+@Singleton
+public class AWSS3BucketNameValidator extends BucketNameValidator {
+
+   @Inject
+   AWSS3BucketNameValidator() {
+      super();
+   }
+
+   public void validate(String containerName) {
+      // AWS S3 allows for upper case characters in bucket names (US Standard region only) and behind the scenes will
+      // use the lower-cased version of the bucket name for its DNS name.  So for AWS S3, we will lowercase the bucket
+      // name prior to validation.  For all other regions than US Standard region, we will let AWS throw handle the
+      // error.
+      //
+      // http://code.google.com/p/jclouds/issues/detail?id=992
+      //
+      // It would be nice to scope this more lax validator to only the us regions, since based on AWS S3 documentation,
+      // this is only necessary for the us regions.
+      super.validate(containerName.toLowerCase());
+   }
+
+}

providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3AsyncClientTest.java

@@ -19,10 +19,16 @@
 package org.jclouds.aws.s3;
 
 import java.io.IOException;
+import java.lang.reflect.Array;
 import java.lang.reflect.Method;
 import java.util.Map;
 import java.util.concurrent.ConcurrentMap;
 
+import com.google.common.base.Supplier;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+import com.google.inject.Module;
+import com.google.inject.TypeLiteral;
 import org.jclouds.aws.s3.config.AWSS3RestClientModule;
 import org.jclouds.aws.s3.functions.ETagFromHttpResponseViaRegex;
 import org.jclouds.aws.s3.functions.UploadIdFromHttpResponseViaRegex;
@@ -39,22 +45,18 @@ import org.jclouds.rest.ConfiguresRestClient;
 import org.jclouds.rest.functions.MapHttp4xxCodesToExceptions;
 import org.jclouds.rest.functions.ReturnVoidOnNotFoundOr404;
 import org.jclouds.rest.internal.RestAnnotationProcessor;
+import org.jclouds.s3.S3AsyncClient;
 import org.jclouds.s3.S3AsyncClientTest;
 import org.jclouds.s3.domain.ObjectMetadata;
 import org.jclouds.s3.domain.ObjectMetadataBuilder;
 import org.jclouds.s3.domain.S3Object;
 import org.jclouds.s3.functions.ReturnFalseIfBucketAlreadyOwnedByYouOrIllegalState;
+import org.jclouds.s3.options.CopyObjectOptions;
 import org.jclouds.s3.options.PutBucketOptions;
 import org.jclouds.s3.options.PutObjectOptions;
 import org.jclouds.s3.xml.LocationConstraintHandler;
 import org.testng.annotations.Test;
 
-import com.google.common.base.Supplier;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Maps;
-import com.google.inject.Module;
-import com.google.inject.TypeLiteral;
-
 /**
  * @author Adrian Cole
  */
@@ -63,6 +65,16 @@ import com.google.inject.TypeLiteral;
 @Test(groups = "unit", testName = "AWSS3AsyncClientTest")
 public class AWSS3AsyncClientTest extends S3AsyncClientTest<AWSS3AsyncClient> {
 
+   @Override
+   public void testCopyObjectInvalidName() throws ArrayIndexOutOfBoundsException, SecurityException,
+                                                  IllegalArgumentException, NoSuchMethodException, IOException {
+      // For AWS S3, S3AsyncClientTest#testCopyObjectInvalidName() will not throw an exception
+      Method method = S3AsyncClient.class.getMethod("copyObject", String.class, String.class, String.class,
+                                                    String.class,
+                                                    Array.newInstance(CopyObjectOptions.class, 0).getClass());
+      processor.createRequest(method, "sourceBucket", "sourceObject", "destinationBucket", "destinationObject");
+   }
+
    public void testGetBucketLocationEU() throws SecurityException, NoSuchMethodException, IOException {
       Method method = AWSS3AsyncClient.class.getMethod("getBucketLocation", String.class);
       HttpRequest request = processor.createRequest(method, "eubucket");

providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java

@@ -26,6 +26,8 @@ import static org.jclouds.crypto.CryptoStreams.md5;
 import static org.jclouds.io.Payloads.newByteArrayPayload;
 import static org.jclouds.s3.options.ListBucketOptions.Builder.withPrefix;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.fail;
 
 import java.io.ByteArrayInputStream;
 import java.io.File;
@@ -34,11 +36,15 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.zip.GZIPInputStream;
 
+import org.jclouds.aws.AWSResponseException;
+import org.jclouds.aws.domain.Region;
 import org.jclouds.blobstore.AsyncBlobStore;
 import org.jclouds.blobstore.BlobStore;
 import org.jclouds.blobstore.KeyNotFoundException;
 import org.jclouds.blobstore.domain.Blob;
+import org.jclouds.blobstore.domain.StorageMetadata;
 import org.jclouds.blobstore.options.PutOptions;
+import org.jclouds.domain.Location;
 import org.jclouds.http.BaseJettyTest;
 import org.jclouds.io.Payload;
 import org.jclouds.s3.S3Client;
@@ -48,14 +54,16 @@ import org.jclouds.s3.domain.ObjectMetadata;
 import org.jclouds.s3.domain.ObjectMetadata.StorageClass;
 import org.jclouds.s3.domain.ObjectMetadataBuilder;
 import org.jclouds.s3.domain.S3Object;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.io.ByteStreams;
+import com.google.common.io.InputSupplier;
+
 import org.testng.ITestContext;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
-import com.google.common.collect.ImmutableMap;
-import com.google.common.io.ByteStreams;
-import com.google.common.io.InputSupplier;
-
 /**
  * Tests behavior of {@code S3Client}
  * 
@@ -197,4 +205,59 @@ public class AWSS3ClientLiveTest extends S3ClientLiveTest {
          returnContainer(containerName);
       }
    }
+
+   /**
+    * http://code.google.com/p/jclouds/issues/detail?id=992
+    */
+   public void testUseBucketWithUpperCaseName() throws Exception {
+      String bucketName = CONTAINER_PREFIX + "-TestBucket";
+      String blobName = "TestBlob.txt";
+      StorageMetadata container = null;
+      BlobStore store = view.getBlobStore();
+
+      // Create and use a valid bucket name with uppercase characters in the bucket name (US regions only)
+      try {
+         store.createContainerInLocation(null, bucketName);
+
+         for (StorageMetadata metadata : store.list()) {
+            if (metadata.getName().equals(bucketName)) {
+               container = metadata;
+               break;
+            }
+         }
+
+         assertNotNull(container);
+
+         store.putBlob(bucketName, store.blobBuilder(blobName)
+                                          .payload("This is a test!")
+                                          .contentType("text/plain")
+                                          .build());
+
+         assertNotNull(store.getBlob(bucketName, blobName));
+      } finally {
+         if (container != null) {
+            store.deleteContainer(bucketName);
+         }
+      }
+
+      // Try to create the same bucket successfully created above in one of the non-US regions to ensure an error is
+      // encountered as expected.
+      Location location = null;
+
+      for (Location pLocation : store.listAssignableLocations()) {
+         if (!ImmutableSet.of(Region.US_STANDARD, Region.US_EAST_1, Region.US_WEST_1, Region.US_WEST_2)
+            .contains(pLocation.getId())) {
+            location = pLocation;
+            break;
+         }
+      }
+
+      try {
+         store.createContainerInLocation(location, bucketName);
+         fail("Should had failed because in non-US regions, mixed-case bucket names are invalid.");
+      } catch (AWSResponseException e) {
+         assertEquals("InvalidBucketName", e.getError().getCode());
+      }
+   }
+
 }