From 675c649cb6c0d81cec7b6115b80fe807366244e8 Mon Sep 17 00:00:00 2001 From: Diwaker Gupta Date: Tue, 30 Jul 2013 17:01:05 -0700 Subject: [PATCH] URL encode string to sign. As recommended at http://s3.amazonaws.com/doc/s3-developer-guide/RESTAuthentication.html: "You can also send a signature as a URL-encoded query-string parameter in the URL for the request." Also deals with some of the craziness of URL encoding/decoding in jclouds. References JCLOUDS-200 --- .../s3/blobstore/AWSS3BlobRequestSigner.java | 22 ++++++++++++++++--- .../blobstore/AWSS3BlobSignerExpectTest.java | 12 +++++----- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/AWSS3BlobRequestSigner.java b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/AWSS3BlobRequestSigner.java index 775cdf5784..dc33ec2481 100644 --- a/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/AWSS3BlobRequestSigner.java +++ b/providers/aws-s3/src/main/java/org/jclouds/aws/s3/blobstore/AWSS3BlobRequestSigner.java @@ -19,6 +19,8 @@ package org.jclouds.aws.s3.blobstore; import static com.google.common.base.Preconditions.checkNotNull; import static org.jclouds.blobstore.util.BlobStoreUtils.cleanRequest; +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; import java.util.Date; import java.util.concurrent.TimeUnit; @@ -35,6 +37,7 @@ import org.jclouds.s3.blobstore.S3BlobRequestSigner; import org.jclouds.s3.blobstore.functions.BlobToObject; import org.jclouds.s3.filters.RequestAuthorizeSignature; +import com.google.common.base.Charsets; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.net.HttpHeaders; @@ -91,10 +94,23 @@ public class AWSS3BlobRequestSigner extends S3BlobRequestSigner builder = request.toBuilder().replaceHeader(HttpHeaders.DATE, expiration); - final String signature = authSigner.sign(authSigner.createStringToSign(builder.build())); - return builder.addQueryParam(TEMPORARY_SIGNATURE_PARAM, signature) - .addQueryParam(HttpHeaders.EXPIRES, expiration) + String stringToSign = authSigner.createStringToSign(builder.build()); + // We MUST encode the signature because addQueryParam internally _always_ decodes values + // and if we don't encode the signature here, the decoding may change the signature. For e.g. + // any '+' characters in the signature will be converted to space ' ' on decoding. + String signature = authSigner.sign(stringToSign); + try { + signature = URLEncoder.encode(signature, Charsets.UTF_8.name()); + } catch (UnsupportedEncodingException e) { + throw new IllegalStateException("Bad encoding on input: " + signature, e); + } + HttpRequest ret = builder.addQueryParam(HttpHeaders.EXPIRES, expiration) .addQueryParam("AWSAccessKeyId", identity) + // Signature MUST be the last parameter because if it isn't, even encoded '+' values in the + // signature will be converted to a space by a subsequent addQueryParameter. + // See HttpRequestTest.testAddBase64AndUrlEncodedQueryParams for more details. + .addQueryParam(TEMPORARY_SIGNATURE_PARAM, signature) .build(); + return ret; } } diff --git a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java index d041abd912..0b38b0b420 100644 --- a/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java +++ b/providers/aws-s3/src/test/java/org/jclouds/aws/s3/blobstore/AWSS3BlobSignerExpectTest.java @@ -35,6 +35,8 @@ import com.google.inject.Module; */ @Test(groups = "unit", testName = "AWSS3BlobSignerExpectTest") public class AWSS3BlobSignerExpectTest extends S3BlobSignerExpectTest { + private static final String DATE = "Thu, 05 Jun 2008 16:38:19 GMT"; + public AWSS3BlobSignerExpectTest() { provider = "aws-s3"; } @@ -43,9 +45,9 @@ public class AWSS3BlobSignerExpectTest extends S3BlobSignerExpectTest { protected HttpRequest getBlobWithTime() { return HttpRequest.builder().method("GET") .endpoint("https://container.s3.amazonaws.com/name" + - "?Signature=Y4Ac4sZfBemGZmgfG78F7IX%20IFg%3D&Expires=1212683902&AWSAccessKeyId=identity") + "?Expires=1212683902&AWSAccessKeyId=identity&Signature=Y4Ac4sZfBemGZmgfG78F7IX%2BIFg%3D") .addHeader("Host", "container.s3.amazonaws.com") - .addHeader("Date", "Thu, 05 Jun 2008 16:38:19 GMT").build(); + .addHeader("Date", DATE).build(); } @Test @@ -60,10 +62,10 @@ public class AWSS3BlobSignerExpectTest extends S3BlobSignerExpectTest { protected HttpRequest putBlobWithTime() { return HttpRequest.builder().method("PUT") .endpoint("https://container.s3.amazonaws.com/name" + - "?Signature=genkB2vLxe3AWV/bPvRTMqQts7E%3D&Expires=1212683902&AWSAccessKeyId=identity") + "?Expires=1212683902&AWSAccessKeyId=identity&Signature=genkB2vLxe3AWV/bPvRTMqQts7E%3D") .addHeader("Expect", "100-continue") .addHeader("Host", "container.s3.amazonaws.com") - .addHeader("Date", "Thu, 05 Jun 2008 16:38:19 GMT").build(); + .addHeader("Date", DATE).build(); } @Test @@ -86,7 +88,7 @@ public class AWSS3BlobSignerExpectTest extends S3BlobSignerExpectTest { @Override @TimeStamp protected String provideTimeStamp(@TimeStamp Supplier cache) { - return "Thu, 05 Jun 2008 16:38:19 GMT"; + return DATE; } } }