From f74d1c0976538f343cd192c18b2fccf91eb22b57 Mon Sep 17 00:00:00 2001 From: David Currie Date: Tue, 11 Sep 2018 11:47:50 +0100 Subject: [PATCH] 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. --- .../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 +++ .../internal/RestAnnotationProcessor.java | 3 ++ .../internal/RestAnnotationProcessorTest.java | 40 +++++++++++++++++++ 6 files changed, 81 insertions(+), 3 deletions(-) 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 35fce21df4..5f1b58d936 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, 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 425f37a5db..9c994b2fe3 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); 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 8374253398..e8b49a7c27 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("\n" + + " 2009-10-28T22:32:00\n" + + " \"9b2cf535f27731c974343645a3985328\"\n" + + " ")); + 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(); + } } 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 44541b4739..0d73192c3e 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 {}; } 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 352e032ca2..1af047abb2 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 method = method(TestHeader.class, "oneHeader", String.class); + Multimap headers = processor.apply(Invocation.create(method, + ImmutableList. 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 headers = processor.apply(Invocation.create(method, + ImmutableList. 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); @@ -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 headers = processor.apply(Invocation.create(method, + ImmutableList. 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}");