JCLOUDS-1447: URL encode x-amz-copy-source The x-amz-copy-source header on S3 CopyObject should be URL encoded (as a path). This is not universally true of all headers though (for example the = in x-amz-copy-source-range) therefore introducing a new parameter on @Headers to indicate whether URL encoding should take place.
Project: http://git-wip-us.apache.org/repos/asf/jclouds/repo Commit: http://git-wip-us.apache.org/repos/asf/jclouds/commit/f74d1c09 Tree: http://git-wip-us.apache.org/repos/asf/jclouds/tree/f74d1c09 Diff: http://git-wip-us.apache.org/repos/asf/jclouds/diff/f74d1c09 Branch: refs/heads/2.1.x Commit: f74d1c0976538f343cd192c18b2fccf91eb22b57 Parents: 8693687 Author: David Currie <dcur...@cloudbees.com> Authored: Tue Sep 11 11:47:50 2018 +0100 Committer: Andrew Gaul <g...@apache.org> Committed: Wed Sep 12 08:03:27 2018 -0700 ---------------------------------------------------------------------- .../src/main/java/org/jclouds/s3/S3Client.java | 4 +- .../java/org/jclouds/s3/S3ClientLiveTest.java | 16 +++++++- .../java/org/jclouds/s3/S3ClientMockTest.java | 15 ++++++++ .../org/jclouds/rest/annotations/Headers.java | 6 +++ .../rest/internal/RestAnnotationProcessor.java | 3 ++ .../internal/RestAnnotationProcessorTest.java | 40 ++++++++++++++++++++ 6 files changed, 81 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/jclouds/blob/f74d1c09/apis/s3/src/main/java/org/jclouds/s3/S3Client.java ---------------------------------------------------------------------- diff --git a/apis/s3/src/main/java/org/jclouds/s3/S3Client.java b/apis/s3/src/main/java/org/jclouds/s3/S3Client.java index 35fce21..5f1b58d 100644 --- a/apis/s3/src/main/java/org/jclouds/s3/S3Client.java +++ b/apis/s3/src/main/java/org/jclouds/s3/S3Client.java @@ -382,7 +382,7 @@ public interface S3Client extends Closeable { @Named("PutObject") @PUT @Path("/{destinationObject}") - @Headers(keys = "x-amz-copy-source", values = "/{sourceBucket}/{sourceObject}") + @Headers(keys = "x-amz-copy-source", values = "/{sourceBucket}/{sourceObject}", urlEncode = true) @XMLResponseParser(CopyObjectHandler.class) ObjectMetadata copyObject(@PathParam("sourceBucket") String sourceBucket, @PathParam("sourceObject") String sourceObject, @@ -733,7 +733,7 @@ public interface S3Client extends Closeable { @Named("UploadPartCopy") @PUT @Path("/{key}") - @Headers(keys = {"x-amz-copy-source", "x-amz-copy-source-range"}, values = {"/{sourceBucket}/{sourceObject}", "bytes={startOffset}-{endOffset}"}) + @Headers(keys = {"x-amz-copy-source", "x-amz-copy-source-range"}, values = {"/{sourceBucket}/{sourceObject}", "bytes={startOffset}-{endOffset}"}, urlEncode = {true, false}) @ResponseParser(ETagFromHttpResponseViaRegex.class) String uploadPartCopy(@Bucket @EndpointParam(parser = AssignCorrectHostnameForBucket.class) @BinderParam( BindAsHostPrefixIfConfigured.class) @ParamValidators(BucketNameValidator.class) String bucketName, http://git-wip-us.apache.org/repos/asf/jclouds/blob/f74d1c09/apis/s3/src/test/java/org/jclouds/s3/S3ClientLiveTest.java ---------------------------------------------------------------------- diff --git a/apis/s3/src/test/java/org/jclouds/s3/S3ClientLiveTest.java b/apis/s3/src/test/java/org/jclouds/s3/S3ClientLiveTest.java index 425f37a..9c994b2 100644 --- a/apis/s3/src/test/java/org/jclouds/s3/S3ClientLiveTest.java +++ b/apis/s3/src/test/java/org/jclouds/s3/S3ClientLiveTest.java @@ -93,7 +93,7 @@ public class S3ClientLiveTest extends BaseBlobStoreIntegrationTest { public S3ClientLiveTest() { this.provider = "s3"; } - + @Override protected Properties setupProperties() { Properties overrides = super.setupProperties(); @@ -408,6 +408,20 @@ public class S3ClientLiveTest extends BaseBlobStoreIntegrationTest { } } + public void testCopyObjectWithSourceKeyRequiringEncoding() throws Exception { + String containerName = getContainerName(); + String sourceKeyRequiringEncoding = "apples#?:$&'\"<>Äà¥"; + String destinationContainer = getContainerName(); + try { + addToContainerAndValidate(containerName, sourceKeyRequiringEncoding); + getApi().copyObject(containerName, sourceKeyRequiringEncoding, destinationContainer, destinationKey); + validateContent(destinationContainer, destinationKey); + } finally { + returnContainer(containerName); + returnContainer(destinationContainer); + } + } + protected String addToContainerAndValidate(String containerName, String sourceKey) throws InterruptedException, ExecutionException, TimeoutException, IOException { String etag = addBlobToContainer(containerName, sourceKey); http://git-wip-us.apache.org/repos/asf/jclouds/blob/f74d1c09/apis/s3/src/test/java/org/jclouds/s3/S3ClientMockTest.java ---------------------------------------------------------------------- diff --git a/apis/s3/src/test/java/org/jclouds/s3/S3ClientMockTest.java b/apis/s3/src/test/java/org/jclouds/s3/S3ClientMockTest.java index 8374253..e8b49a7 100644 --- a/apis/s3/src/test/java/org/jclouds/s3/S3ClientMockTest.java +++ b/apis/s3/src/test/java/org/jclouds/s3/S3ClientMockTest.java @@ -33,6 +33,7 @@ import org.jclouds.ContextBuilder; import org.jclouds.concurrent.config.ExecutorServiceModule; import org.jclouds.http.okhttp.config.OkHttpCommandExecutorServiceModule; import org.jclouds.s3.domain.S3Object; +import org.jclouds.s3.options.CopyObjectOptions; import org.testng.annotations.Test; import com.google.common.collect.ImmutableList; @@ -96,4 +97,18 @@ public class S3ClientMockTest { server.shutdown(); } + + public void testSourceEncodedOnCopy() throws IOException, InterruptedException { + MockWebServer server = new MockWebServer(); + server.enqueue(new MockResponse().setBody("<CopyObjectResult>\n" + + " <LastModified>2009-10-28T22:32:00</LastModified>\n" + + " <ETag>\"9b2cf535f27731c974343645a3985328\"</ETag>\n" + + " </CopyObjectResult>")); + server.play(); + S3Client client = getS3Client(server.getUrl("/")); + client.copyObject("sourceBucket", "apples#?:$&'\"<>Äà¥", "destinationBucket", "destinationObject", CopyObjectOptions.NONE); + RecordedRequest request = server.takeRequest(); + assertEquals(request.getHeaders("x-amz-copy-source"), ImmutableList.of("/sourceBucket/apples%23%3F%3A%24%26%27%22%3C%3E%C4%8D%E0%A5%90")); + server.shutdown(); + } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/f74d1c09/core/src/main/java/org/jclouds/rest/annotations/Headers.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/rest/annotations/Headers.java b/core/src/main/java/org/jclouds/rest/annotations/Headers.java index 44541b4..0d73192 100644 --- a/core/src/main/java/org/jclouds/rest/annotations/Headers.java +++ b/core/src/main/java/org/jclouds/rest/annotations/Headers.java @@ -48,4 +48,10 @@ public @interface Headers { * */ String[] values(); + + /** + * Indicates whether a header should be URL encoded. Optional for backwards compatibility. + * The default behavior is that the header is not URL encoded. + */ + boolean[] urlEncode() default {}; } http://git-wip-us.apache.org/repos/asf/jclouds/blob/f74d1c09/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java index 352e032..1af047a 100644 --- a/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java +++ b/core/src/main/java/org/jclouds/rest/internal/RestAnnotationProcessor.java @@ -763,6 +763,9 @@ public class RestAnnotationProcessor implements Function<Invocation, HttpRequest for (int i = 0; i < header.keys().length; i++) { String value = header.values()[i]; value = replaceTokens(value, tokenValues); + // urlEncode may have less entries than keys e.g. default value of {} + if (i < header.urlEncode().length && header.urlEncode()[i]) + value = urlEncode(value, '/'); headers.put(header.keys()[i], value); } } http://git-wip-us.apache.org/repos/asf/jclouds/blob/f74d1c09/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java index e2d7eb2..c81c652 100644 --- a/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java +++ b/core/src/test/java/org/jclouds/rest/internal/RestAnnotationProcessorTest.java @@ -1796,6 +1796,12 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { @GET @Path("/") + @Headers(keys = "x-amz-copy-source", values = "/{bucket}", urlEncode = true) + public void oneHeaderEncoded(@PathParam("bucket") String path) { + } + + @GET + @Path("/") @Headers(keys = { "slash", "hyphen" }, values = { "/{bucket}", "-{bucket}" }) public void twoHeader(@PathParam("bucket") String path) { } @@ -1811,6 +1817,12 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { @Headers(keys = "x-amz-copy-source", values = "/{bucket}/{key}") public void twoHeadersOutOfOrder(@PathParam("key") String path, @PathParam("bucket") String path2) { } + + @GET + @Path("/") + @Headers(keys = {"unencoded", "x-amz-copy-source"}, values = {"/{bucket}", "/{bucket}"}, urlEncode = {false, true}) + public void twoHeadersMixedEncoding(@PathParam("bucket") String path) { + } } @Test @@ -1850,6 +1862,24 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { } @Test + public void testBuildOneHeaderUnencoded() throws SecurityException, NoSuchMethodException { + Invokable<?, ?> method = method(TestHeader.class, "oneHeader", String.class); + Multimap<String, String> headers = processor.apply(Invocation.create(method, + ImmutableList.<Object> of("apples#?:$&'\"<>Äà¥"))).getHeaders(); + assertEquals(headers.size(), 1); + assertEquals(headers.get("x-amz-copy-source"), ImmutableList.of("/apples#?:$&'\"<>Äà¥")); + } + + @Test + public void testBuildOneHeaderEncoded() throws SecurityException, NoSuchMethodException { + Invokable<?, ?> method = method(TestHeader.class, "oneHeaderEncoded", String.class); + Multimap<String, String> headers = processor.apply(Invocation.create(method, + ImmutableList.<Object> of("apples#?:$&'\"<>Äà¥"))).getHeaders(); + assertEquals(headers.size(), 1); + assertEquals(headers.get("x-amz-copy-source"), ImmutableList.of("/apples%23%3F%3A%24%26%27%22%3C%3E%C4%8D%E0%A5%90")); + } + + @Test public void testBuildTwoHeaders() throws SecurityException, NoSuchMethodException { Invokable<?, ?> method = method(TestHeader.class, "twoHeaders", String.class, String.class); Multimap<String, String> headers = processor.apply(Invocation.create(method, @@ -1868,6 +1898,16 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest { assertEquals(headers.get("x-amz-copy-source"), ImmutableList.of("/eggs/robot")); } + @Test + public void testBuildTwoHeadersMixedEncoding() throws SecurityException, NoSuchMethodException { + Invokable<?, ?> method = method(TestHeader.class, "twoHeadersMixedEncoding", String.class); + Multimap<String, String> headers = processor.apply(Invocation.create(method, + ImmutableList.<Object> of("apples#?:$&'\"<>Äà¥"))).getHeaders(); + assertEquals(headers.size(), 2); + assertEquals(headers.get("unencoded"), ImmutableList.of("/apples#?:$&'\"<>Äà¥")); + assertEquals(headers.get("x-amz-copy-source"), ImmutableList.of("/apples%23%3F%3A%24%26%27%22%3C%3E%C4%8D%E0%A5%90")); + } + public static class TestReplaceQueryOptions extends BaseHttpRequestOptions { public TestReplaceQueryOptions() { this.queryParameters.put("x-amz-copy-source", "/{bucket}");