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

Prior to this commit, jclouds wouldn't allow you to interact with any buckets
in S3 that were named with uppercase characters.  Per AWS S3 docs, this
non-standard naming is valid in US regions only.  This update fixes jclouds so
that it can interact with, and even attempt to create, buckets with uppercase
characters for AWS S3 without actually impacting other S3 implementations.  This
fix also will not have any impact in non-US regions other than instead of a
bucket name validation error you'll get an InvalidBucketName error back from
AWS S3 when you attempt to create a bucket with an uppercase character in a
non-US region.  To summarize, nothing changes other than US regions now allow
creation of bucket names with upper case characters and jclouds now can
interact with these non-standard named buckets without failure.
This commit is contained in:
Jeremy Whitlock 2012-06-21 17:05:20 -06:00
parent a12e10d7f3
commit 2702e942d2
9 changed files with 199 additions and 37 deletions

View File

@ -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();

View File

@ -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;
}
}

View File

@ -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("/"))

View File

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

View File

@ -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)

View File

@ -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();
}

View File

@ -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());
}
}

View File

@ -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");

View File

@ -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());
}
}
}