jglick commented on this pull request. @nacx please take another look. I added both a unit test and a live test. A mock test seems redundant given those.
> @@ -507,6 +538,23 @@ public void testCopyWithMetadata() throws > InterruptedException, ExecutionExcepti } } + // JCLOUDS-1401 + public void testUnusualKeyCharacters() throws InterruptedException, ExecutionException, TimeoutException, IOException { + String containerName = getContainerName(); + try { + String dirName = "a%2Fb&xxx#?:$'\\\"<>čॐ"; + String fileName = "foo%3Abar.xml"; + addToContainerAndValidate(containerName, dirName + '/' + fileName); + PageSet<? extends StorageMetadata> list = view.getBlobStore().list(containerName, + ListContainerOptions.Builder.prefix(dirName + "/")); + assertEquals(list.size(), 1); + StorageMetadata md = list.iterator().next(); This fails with the reported AWS error if you s/rawQuery/query/. > @@ -83,6 +84,11 @@ + "SignedHeaders=content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-storage-class, " + "Signature=090f1bb1db984221ae1a20c5d12a82820a0d74b4be85f20daa1431604f41df08"; + private static final String LIST_BUCKET_RESULT = "AWS4-HMAC-SHA256 " + + "Credential=AKIAPAEBI3QI4EXAMPLE/20150203/cn-north-1/s3/aws4_request, " + + "SignedHeaders=host;x-amz-content-sha256;x-amz-date, " + + "Signature=6cc5d0758e2599be7cb172fd57cefab2828201a2b4d372972a83dc304de93958"; Merely based on running the test input against rawQuery. I did not attempt to consult the AWS documentation and manually construct the signature base as per [these instructions](https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html). The key to the fix is that `Queries.parseKeyValueFromStringToDecodedMap` is used to URL-decode the parameters of the query string after splitting on `&`, and we then reëncode in `getCanonicalizedQueryString`, so using `URI.getQuery()` (which decodes the value) not only double-decodes, it actually loses information. For example, ``` URI u = new URI("https://x.com/?p1=a&p2=b%26c%3Dd&p3=e"); System.out.println(u.getRawQuery()); System.out.println(u.getQuery()); ``` will print ``` p1=a&p2=b%26c%3Dd&p3=e p1=a&p2=b&c=d&p3=e ``` > public S3ClientLiveTest() { this.provider = "s3"; } + @Override + protected Properties setupProperties() { + Properties overrides = super.setupProperties(); + sessionToken = setIfTestSystemPropertyPresent(overrides, provider + ".sessionToken"); + return overrides; + } + + @Override + protected ContextBuilder newBuilder() { + ContextBuilder builder = super.newBuilder(); + if (sessionToken != null) { + builder.credentialsSupplier(new Supplier<Credentials>() { + @Override + public Credentials get() { + return SessionCredentials.builder().identity(identity).credential(credential).sessionToken(sessionToken).build(); Without this I was unable to run live tests at all. (Maybe something about IAM?) The new parameter is optional. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/1226#pullrequestreview-136273302