Skip to content

Commit babaa5d

Browse files
millemsmillems
authored
Do not prefix copy-source with / when using the sourceBucket and sourceKey APIs. (#3630)
The main goal of this change is to fix an issue where access point ARNs were prefixed with /, causing them to fail because / is not supported as a prefix. This also stops prefixing buckets with /, but that seems to work fine in the included integration test.
1 parent 22b6403 commit babaa5d

File tree

4 files changed

+59
-35
lines changed

4 files changed

+59
-35
lines changed

‎.changes/next-release/bugfix-AmazonS3-cd8c17d.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "Amazon S3",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"description": "Do not prefix copy-source with / when using the sourceBucket and sourceKey APIs."
6+
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/CopyObjectIntegrationTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,10 @@ public static void initializeTestData() throws Exception {
8787
@Test
8888
public void copyObject_CopiesObjectToNewKey() throws Exception {
8989
s3.copyObject(CopyObjectRequest.builder()
90-
.copySource(BUCKET_NAME + "/" + SOURCE_KEY)
91-
.bucket(BUCKET_NAME)
92-
.key(DESTINATION_KEY)
90+
.sourceBucket(BUCKET_NAME)
91+
.sourceKey(SOURCE_KEY)
92+
.destinationBucket(BUCKET_NAME)
93+
.destinationKey(DESTINATION_KEY)
9394
.build());
9495

9596
s3.headObject(HeadObjectRequest.builder()

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/CopySourceInterceptor.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import static software.amazon.awssdk.utils.http.SdkHttpUtils.urlEncodeIgnoreSlashes;
1919

20-
import java.util.Optional;
2120
import software.amazon.awssdk.annotations.SdkInternalApi;
2221
import software.amazon.awssdk.core.SdkRequest;
2322
import software.amazon.awssdk.core.interceptor.Context.ModifyRequest;
@@ -98,12 +97,8 @@ private static SdkRequest modifyUploadPartCopyRequest(UploadPartCopyRequest requ
9897

9998
private static String constructCopySource(String sourceBucket, String sourceKey, String sourceVersionId) {
10099
StringBuilder copySource = new StringBuilder();
101-
Optional<S3ResourceType> arnTypeOptional = S3ArnUtils.getArnType(sourceBucket);
102-
if (!arnTypeOptional.isPresent()) {
103-
copySource.append("/");
104-
}
105100
copySource.append(urlEncodeIgnoreSlashes(sourceBucket));
106-
arnTypeOptional.ifPresent(arnType -> {
101+
S3ArnUtils.getArnType(sourceBucket).ifPresent(arnType -> {
107102
if (arnType == S3ResourceType.ACCESS_POINT || arnType == S3ResourceType.OUTPOST) {
108103
copySource.append("/object");
109104
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/handlers/CopySourceInterceptorTest.java

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,40 @@ public class CopySourceInterceptorTest {
3636
public static Collection<String[]> parameters() throws Exception {
3737
return Arrays.asList(new String[][] {
3838
{"bucket", "simpleKey", null,
39-
"/bucket/simpleKey"},
39+
"bucket/simpleKey"},
4040

4141
{"bucket", "key/with/slashes", null,
42-
"/bucket/key/with/slashes"},
42+
"bucket/key/with/slashes"},
4343

4444
{"bucket", "\uD83E\uDEA3", null,
45-
"/bucket/%F0%9F%AA%A3"},
45+
"bucket/%F0%9F%AA%A3"},
4646

4747
{"bucket", "specialChars._ +!#$&'()*,:;=?@\"", null,
48-
"/bucket/specialChars._%20%2B%21%23%24%26%27%28%29%2A%2C%3A%3B%3D%3F%40%22"},
48+
"bucket/specialChars._%20%2B%21%23%24%26%27%28%29%2A%2C%3A%3B%3D%3F%40%22"},
4949

5050
{"bucket", "%20", null,
51-
"/bucket/%2520"},
51+
"bucket/%2520"},
5252

5353
{"bucket", "key/with/version", "ZJlqdTGGfnWjRWjm.WtQc5XRTNJn3sz_",
54-
"/bucket/key/with/version?versionId=ZJlqdTGGfnWjRWjm.WtQc5XRTNJn3sz_"}
54+
"bucket/key/with/version?versionId=ZJlqdTGGfnWjRWjm.WtQc5XRTNJn3sz_"},
55+
56+
{"source-bucke-e00000144705073101keauk155va6smod88ynqbeta0--op-s3", "CT-debug-Info-16", null,
57+
"source-bucke-e00000144705073101keauk155va6smod88ynqbeta0--op-s3/CT-debug-Info-16"},
58+
59+
{"source-bucke-e00000144705073101keauk155va6smod88ynqbeta0--op-s3", "CT-debug-Info-16", "123",
60+
"source-bucke-e00000144705073101keauk155va6smod88ynqbeta0--op-s3/CT-debug-Info-16?versionId=123"},
61+
62+
{"arn:aws:s3-outposts:us-west-2:123456789012:outpost/my-outpost/bucket/my-bucket", "my-key", null,
63+
"arn%3Aaws%3As3-outposts%3Aus-west-2%3A123456789012%3Aoutpost/my-outpost/bucket/my-bucket/object/my-key"},
64+
65+
{"arn:aws:s3-outposts:us-west-2:123456789012:outpost/my-outpost/bucket/my-bucket", "my-key", "123",
66+
"arn%3Aaws%3As3-outposts%3Aus-west-2%3A123456789012%3Aoutpost/my-outpost/bucket/my-bucket/object/my-key?versionId=123"},
67+
68+
{"arn:aws:s3:us-west-2:123456789012:accesspoint/my-access-point", "my-key", null,
69+
"arn%3Aaws%3As3%3Aus-west-2%3A123456789012%3Aaccesspoint/my-access-point/object/my-key"},
70+
71+
{"arn:aws:s3:us-west-2:123456789012:accesspoint/my-access-point", "my-key", "123",
72+
"arn%3Aaws%3As3%3Aus-west-2%3A123456789012%3Aaccesspoint/my-access-point/object/my-key?versionId=123"}
5573
});
5674
}
5775

@@ -125,7 +143,10 @@ public void modifyRequest_Throws_whenCopySourceUsedWithSourceBucket_withUploadPa
125143

126144
@Test
127145
public void modifyRequest_Throws_whenSourceBucketNotSpecified_withCopyObjectRequest() {
128-
CopyObjectRequest originalRequest = CopyObjectRequest.builder().build();
146+
CopyObjectRequest originalRequest = CopyObjectRequest.builder()
147+
.sourceKey(sourceKey)
148+
.sourceVersionId(sourceVersionId)
149+
.build();
129150

130151
assertThatThrownBy(() -> {
131152
interceptor.modifyRequest(() -> originalRequest, new ExecutionAttributes());
@@ -135,7 +156,10 @@ public void modifyRequest_Throws_whenSourceBucketNotSpecified_withCopyObjectRequ
135156

136157
@Test
137158
public void modifyRequest_Throws_whenSourceBucketNotSpecified_withUploadPartCopyRequest() {
138-
UploadPartCopyRequest originalRequest = UploadPartCopyRequest.builder().build();
159+
UploadPartCopyRequest originalRequest = UploadPartCopyRequest.builder()
160+
.sourceKey(sourceKey)
161+
.sourceVersionId(sourceVersionId)
162+
.build();
139163

140164
assertThatThrownBy(() -> {
141165
interceptor.modifyRequest(() -> originalRequest, new ExecutionAttributes());
@@ -144,30 +168,28 @@ public void modifyRequest_Throws_whenSourceBucketNotSpecified_withUploadPartCopy
144168
}
145169

146170
@Test
147-
public void modifyRequest_insertsSlashObject_whenAccessPointArn() {
148-
String accessPointArn = "arn:aws:s3:us-west-2:123456789012:accesspoint/my-access-point";
171+
public void modifyRequest_Throws_whenSourceKeyNotSpecified_withCopyObjectRequest() {
149172
CopyObjectRequest originalRequest = CopyObjectRequest.builder()
150-
.sourceBucket(accessPointArn)
151-
.sourceKey("my-key")
173+
.sourceBucket(sourceBucket)
174+
.sourceVersionId(sourceVersionId)
152175
.build();
153-
CopyObjectRequest modifiedRequest = (CopyObjectRequest) interceptor
154-
.modifyRequest(() -> originalRequest, new ExecutionAttributes());
155176

156-
assertThat(modifiedRequest.copySource()).isEqualTo(
157-
"arn%3Aaws%3As3%3Aus-west-2%3A123456789012%3Aaccesspoint/my-access-point/object/my-key");
177+
assertThatThrownBy(() -> {
178+
interceptor.modifyRequest(() -> originalRequest, new ExecutionAttributes());
179+
}).isInstanceOf(IllegalArgumentException.class)
180+
.hasMessage("Parameter 'sourceKey' must not be null");
158181
}
159182

160183
@Test
161-
public void modifyRequest_insertsSlashObject_whenOutpostArn() {
162-
String outpostBucketArn = "arn:aws:s3-outposts:us-west-2:123456789012:outpost/my-outpost/bucket/my-bucket";
163-
CopyObjectRequest originalRequest = CopyObjectRequest.builder()
164-
.sourceBucket(outpostBucketArn)
165-
.sourceKey("my-key")
166-
.build();
167-
CopyObjectRequest modifiedRequest = (CopyObjectRequest) interceptor
168-
.modifyRequest(() -> originalRequest, new ExecutionAttributes());
184+
public void modifyRequest_Throws_whenSourceKeyNotSpecified_withUploadPartCopyRequest() {
185+
UploadPartCopyRequest originalRequest = UploadPartCopyRequest.builder()
186+
.sourceBucket(sourceBucket)
187+
.sourceVersionId(sourceVersionId)
188+
.build();
169189

170-
assertThat(modifiedRequest.copySource()).isEqualTo(
171-
"arn%3Aaws%3As3-outposts%3Aus-west-2%3A123456789012%3Aoutpost/my-outpost/bucket/my-bucket/object/my-key");
190+
assertThatThrownBy(() -> {
191+
interceptor.modifyRequest(() -> originalRequest, new ExecutionAttributes());
192+
}).isInstanceOf(IllegalArgumentException.class)
193+
.hasMessage("Parameter 'sourceKey' must not be null");
172194
}
173195
}

0 commit comments

Comments
 (0)