mirror of https://github.com/apache/jclouds.git
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.
This commit is contained in:
parent
5803de0f8e
commit
7ebf12bf38
|
@ -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,
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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 {};
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -1794,6 +1794,12 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
|
|||
public void oneHeader(@PathParam("bucket") String path) {
|
||||
}
|
||||
|
||||
@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}" })
|
||||
|
@ -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
|
||||
|
@ -1849,6 +1861,24 @@ public class RestAnnotationProcessorTest extends BaseRestApiTest {
|
|||
assertEquals(headers.get("x-amz-copy-source"), ImmutableList.of("/robot"));
|
||||
}
|
||||
|
||||
@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);
|
||||
|
@ -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}");
|
||||
|
|
Loading…
Reference in New Issue