diff --git a/apis/s3/src/main/java/org/jclouds/s3/binders/BindAsHostPrefixIfConfigured.java b/apis/s3/src/main/java/org/jclouds/s3/binders/BindAsHostPrefixIfConfigured.java index 321910f564..465b8c245a 100644 --- a/apis/s3/src/main/java/org/jclouds/s3/binders/BindAsHostPrefixIfConfigured.java +++ b/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 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. of())) .build(); diff --git a/apis/s3/src/main/java/org/jclouds/s3/filters/RequestAuthorizeSignature.java b/apis/s3/src/main/java/org/jclouds/s3/filters/RequestAuthorizeSignature.java index 74876d10e6..f6a2d20a0d 100644 --- a/apis/s3/src/main/java/org/jclouds/s3/filters/RequestAuthorizeSignature.java +++ b/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,8 +161,7 @@ public class RequestAuthorizeSignature implements HttpRequestFilter, RequestSign } appendAmzHeaders(canonicalizedHeaders, buffer); - if (isVhostStyle) - appendBucketName(request, buffer); + appendBucketName(request, buffer); appendUriPath(request, buffer); if (signatureWire.enabled()) signatureWire.output(buffer.toString()); @@ -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"); - GeneratedHttpRequest request = GeneratedHttpRequest.class.cast(req); + String bucketName = getBucketName(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; - } - } - - if (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 + // the only S3 implementation configured to allow uppercase payload/bucket/container names. + // + // http://code.google.com/p/jclouds/issues/detail?id=992 + if (isVhostStyle && bucketName!= null && bucketName.equals(bucketName.toLowerCase())) 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; + } + } diff --git a/apis/s3/src/main/java/org/jclouds/s3/handlers/ParseS3ErrorFromXmlContent.java b/apis/s3/src/main/java/org/jclouds/s3/handlers/ParseS3ErrorFromXmlContent.java index ecf1c3fb72..62c9fc426a 100644 --- a/apis/s3/src/main/java/org/jclouds/s3/handlers/ParseS3ErrorFromXmlContent.java +++ b/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 com.google.common.base.Joiner; -import com.google.common.base.Splitter; +import org.jclouds.s3.S3ApiMetadata; /** * @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("/")) diff --git a/apis/s3/src/main/java/org/jclouds/s3/predicates/validators/BucketNameValidator.java b/apis/s3/src/main/java/org/jclouds/s3/predicates/validators/BucketNameValidator.java index acf473fe45..ed441fe62c 100644 --- a/apis/s3/src/main/java/org/jclouds/s3/predicates/validators/BucketNameValidator.java +++ b/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); } diff --git a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/AWSS3AsyncClient.java b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/AWSS3AsyncClient.java index 9f6e5d9ce3..ac0ec71c0b 100644 --- a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/AWSS3AsyncClient.java +++ b/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) diff --git a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java index 93e4538f3d..aab19e953d 100644 --- a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/config/AWSS3RestClientModule.java +++ b/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 { + @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"); diff --git a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java index 61339bcd6a..568a7f8f71 100644 --- a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/AWSS3ClientLiveTest.java +++ b/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()); + } + } + }