You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jclouds.apache.org by ga...@apache.org on 2018/09/12 15:03:44 UTC

[2/2] jclouds git commit: JCLOUDS-1447: URL encode x-amz-copy-source

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 <dc...@cloudbees.com>
Authored: Tue Sep 11 11:47:50 2018 +0100
Committer: Andrew Gaul <ga...@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}");