Fixed s3 decoding response for ListObjectsVersion and ListMultipartUploads

.changes/next-release/bugfix-AmazonS3-9bf1df7.json

@@ -0,0 +1,5 @@
+{
+    "category": "Amazon S3", 
+    "type": "bugfix", 
+    "description": "Fixed an issue where fields in `ListObjectVersionsResponse` and `ListMultipartUploadsResponse` are not decoded correctly when encodingType is specified as url. See [#1601](https://github.com/aws/aws-sdk-java-v2/issues/1601)"
+}

services/s3/src/it/java/software/amazon/awssdk/services/s3/UrlEncodingIntegrationTest.java

@@ -0,0 +1,134 @@
+/*
+ * Copyright 2010-2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License").
+ * You may not use this file except in compliance with the License.
+ * A copy of the License is located at
+ *
+ *  http://aws.amazon.com/apache2.0
+ *
+ * or in the "license" file accompanying this file. This file is distributed
+ * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and limitations under the License.
+ */
+
+package software.amazon.awssdk.services.s3;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
+
+import org.apache.commons.lang3.RandomStringUtils;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
+import software.amazon.awssdk.services.s3.model.EncodingType;
+import software.amazon.awssdk.services.s3.model.ListMultipartUploadsResponse;
+import software.amazon.awssdk.services.s3.model.ListObjectVersionsResponse;
+import software.amazon.awssdk.services.s3.model.ListObjectsResponse;
+import software.amazon.awssdk.services.s3.model.ListObjectsV2Response;
+import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+import software.amazon.awssdk.services.s3.model.UploadPartResponse;
+
+/**
+ * Integration tests for the operations that support encoding type
+ */
+public class UrlEncodingIntegrationTest extends S3IntegrationTestBase {
+    /**
+     * The name of the bucket created, used, and deleted by these tests.
+     */
+    private static final String BUCKET_NAME = temporaryBucketName(UrlEncodingIntegrationTest.class);
+    private static final String KEY_NAME_WITH_SPECIAL_CHARS = "filename_@_=_&_?_+_)_.temp";
+
+    @BeforeClass
+    public static void createResources() {
+        createBucket(BUCKET_NAME);
+        s3.putObject(PutObjectRequest.builder()
+                                     .bucket(BUCKET_NAME)
+                                     .key(KEY_NAME_WITH_SPECIAL_CHARS)
+                                     .build(), RequestBody.fromString(RandomStringUtils.random(1000)));
+    }
+
+    /**
+     * Releases all resources created in this test.
+     */
+    @AfterClass
+    public static void tearDown() {
+        deleteBucketAndAllContents(BUCKET_NAME);
+    }
+
+    @Test
+    public void listObjectVersionsWithUrlEncodingType_shouldDecode() {
+        ListObjectVersionsResponse listObjectVersionsResponse =
+            s3.listObjectVersions(b -> b.bucket(BUCKET_NAME).encodingType(EncodingType.URL));
+        listObjectVersionsResponse.versions().forEach(v -> assertKeyIsDecoded(v.key()));
+
+        ListObjectVersionsResponse asyncResponse =
+            s3Async.listObjectVersions(b -> b.bucket(BUCKET_NAME).encodingType(EncodingType.URL)).join();
+
+        asyncResponse.versions().forEach(v -> assertKeyIsDecoded(v.key()));
+    }
+
+    @Test
+    public void listObjectV2WithUrlEncodingType_shouldDecode() {
+        ListObjectsV2Response listObjectsV2Response =
+            s3.listObjectsV2(b -> b.bucket(BUCKET_NAME).encodingType(EncodingType.URL));
+
+        listObjectsV2Response.contents().forEach(c -> assertKeyIsDecoded(c.key()));
+        ListObjectVersionsResponse asyncResponse =
+            s3Async.listObjectVersions(b -> b.bucket(BUCKET_NAME).encodingType(EncodingType.URL)).join();
+
+        asyncResponse.versions().forEach(v -> assertKeyIsDecoded(v.key()));
+    }
+
+    @Test
+    public void listObjectWithUrlEncodingType_shouldDecode() {
+        ListObjectsResponse listObjectsV2Response =
+            s3.listObjects(b -> b.bucket(BUCKET_NAME).encodingType(EncodingType.URL));
+
+        listObjectsV2Response.contents().forEach(c -> assertKeyIsDecoded(c.key()));
+        ListObjectVersionsResponse asyncResponse =
+            s3Async.listObjectVersions(b -> b.bucket(BUCKET_NAME).encodingType(EncodingType.URL)).join();
+
+        asyncResponse.versions().forEach(v -> assertKeyIsDecoded(v.key()));
+    }
+
+    @Test
+    public void listMultipartUploadsWithUrlEncodingType_shouldDecode() {
+        String uploaddId = null;
+        try {
+            CreateMultipartUploadResponse multipartUploadResponse =
+                s3.createMultipartUpload(b -> b.bucket(BUCKET_NAME).key(KEY_NAME_WITH_SPECIAL_CHARS));
+            uploaddId = multipartUploadResponse.uploadId();
+
+            String finalUploadId = uploaddId;
+            UploadPartResponse uploadPartResponse = s3.uploadPart(b -> b.bucket(BUCKET_NAME)
+                                                                        .key(KEY_NAME_WITH_SPECIAL_CHARS)
+                                                                        .partNumber(1)
+                                                                        .uploadId(finalUploadId),
+                                                                  RequestBody.fromString(RandomStringUtils.random(1000)));
+
+
+            ListMultipartUploadsResponse listMultipartUploadsResponse =
+                s3.listMultipartUploads(b -> b.encodingType(EncodingType.URL).bucket(BUCKET_NAME));
+
+            listMultipartUploadsResponse.uploads().forEach(upload -> assertThat(upload.key()).isEqualTo(KEY_NAME_WITH_SPECIAL_CHARS));
+
+            ListMultipartUploadsResponse asyncListMultipartUploadsResponse =
+                s3Async.listMultipartUploads(b -> b.encodingType(EncodingType.URL).bucket(BUCKET_NAME)).join();
+
+            asyncListMultipartUploadsResponse.uploads().forEach(upload -> assertKeyIsDecoded(upload.key()));
+        } finally {
+            if (uploaddId != null) {
+                String finalUploadId = uploaddId;
+                s3.abortMultipartUpload(b -> b.bucket(BUCKET_NAME).key(KEY_NAME_WITH_SPECIAL_CHARS).uploadId(finalUploadId));
+            }
+        }
+    }
+
+    private void assertKeyIsDecoded(String key) {
+        assertThat(key).isEqualTo(KEY_NAME_WITH_SPECIAL_CHARS);
+    }
+}

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

@@ -17,16 +17,22 @@
 
 import static software.amazon.awssdk.utils.http.SdkHttpUtils.urlDecode;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.core.SdkResponse;
 import software.amazon.awssdk.core.interceptor.Context;
 import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
 import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
+import software.amazon.awssdk.services.s3.model.CommonPrefix;
 import software.amazon.awssdk.services.s3.model.EncodingType;
+import software.amazon.awssdk.services.s3.model.ListMultipartUploadsResponse;
+import software.amazon.awssdk.services.s3.model.ListObjectVersionsResponse;
 import software.amazon.awssdk.services.s3.model.ListObjectsResponse;
 import software.amazon.awssdk.services.s3.model.ListObjectsV2Response;
+import software.amazon.awssdk.services.s3.model.MultipartUpload;
+import software.amazon.awssdk.services.s3.model.ObjectVersion;
 import software.amazon.awssdk.services.s3.model.S3Object;
 
 /**
@@ -50,9 +56,19 @@ public SdkResponse modifyResponse(Context.ModifyResponse context,
         SdkResponse response = context.response();
         if (shouldHandle(response)) {
             if (response instanceof ListObjectsResponse) {
-                response = modifyListObjectsResponse((ListObjectsResponse) response);
-            } else if (response instanceof ListObjectsV2Response) {
-                response = modifyListObjectsV2Response((ListObjectsV2Response) response);
+                return modifyListObjectsResponse((ListObjectsResponse) response);
+            }
+
+            if (response instanceof ListObjectsV2Response) {
+                return modifyListObjectsV2Response((ListObjectsV2Response) response);
+            }
+
+            if (response instanceof ListObjectVersionsResponse) {
+                return modifyListObjectVersionsResponse((ListObjectVersionsResponse) response);
+            }
+
+            if (response instanceof ListMultipartUploadsResponse) {
+                return modifyListMultipartUploadsResponse((ListMultipartUploadsResponse) response);
             }
         }
         return response;
@@ -67,30 +83,90 @@ private static boolean shouldHandle(SdkResponse sdkResponse) {
     // Elements to decode: Delimiter, Marker, Prefix, NextMarker, Key
     private static SdkResponse modifyListObjectsResponse(ListObjectsResponse response) {
         return response.toBuilder()
-                .delimiter(urlDecode(response.delimiter()))
-                .marker(urlDecode(response.marker()))
-                .prefix(urlDecode(response.prefix()))
-                .nextMarker(urlDecode(response.nextMarker()))
-                .contents(decodeContents(response.contents()))
-                .build();
+                       .delimiter(urlDecode(response.delimiter()))
+                       .marker(urlDecode(response.marker()))
+                       .prefix(urlDecode(response.prefix()))
+                       .nextMarker(urlDecode(response.nextMarker()))
+                       .contents(decodeContents(response.contents()))
+                       .commonPrefixes(decodeCommonPrefixes(response.commonPrefixes()))
+                       .build();
     }
 
     // Elements to decode: Delimiter, Prefix, Key, and StartAfter
     private static SdkResponse modifyListObjectsV2Response(ListObjectsV2Response response) {
         return response.toBuilder()
-                .delimiter(urlDecode(response.delimiter()))
-                .prefix(urlDecode(response.prefix()))
-                .startAfter(urlDecode(response.startAfter()))
-                .contents(decodeContents(response.contents()))
-                .build();
+                       .delimiter(urlDecode(response.delimiter()))
+                       .prefix(urlDecode(response.prefix()))
+                       .startAfter(urlDecode(response.startAfter()))
+                       .contents(decodeContents(response.contents()))
+                       .commonPrefixes(decodeCommonPrefixes(response.commonPrefixes()))
+                       .build();
+    }
+
+    // https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListObjectVersions.html
+    // Elements to decode: Delimiter, KeyMarker, NextKeyMarker, Prefix
+    private SdkResponse modifyListObjectVersionsResponse(ListObjectVersionsResponse response) {
+
+        return response.toBuilder()
+                       .prefix(urlDecode(response.prefix()))
+                       .keyMarker(urlDecode(response.keyMarker()))
+                       .delimiter(urlDecode(response.delimiter()))
+                       .nextKeyMarker(urlDecode(response.nextKeyMarker()))
+                       .commonPrefixes(decodeCommonPrefixes(response.commonPrefixes()))
+                       .versions(decodeObjectVersions(response.versions()))
+                       .build();
+    }
+
+    // https://docs.aws.amazon.com/AmazonS3/latest/API/API_ListMultipartUploads.html
+    // Elements to decode: Delimiter, KeyMarker, NextKeyMarker, Prefix, Key
+    private SdkResponse modifyListMultipartUploadsResponse(ListMultipartUploadsResponse response) {
+        return response.toBuilder()
+                       .delimiter(urlDecode(response.delimiter()))
+                       .keyMarker(urlDecode(response.keyMarker()))
+                       .nextKeyMarker(urlDecode(response.nextKeyMarker()))
+                       .prefix(urlDecode(response.prefix()))
+                       .commonPrefixes(decodeCommonPrefixes(response.commonPrefixes()))
+                       .uploads(decodeMultipartUpload(response.uploads()))
+                       .build();
+
     }
 
     private static List<S3Object> decodeContents(List<S3Object> contents) {
         if (contents == null) {
             return null;
         }
-        return contents.stream()
-                       .map(o -> o.toBuilder().key(urlDecode(o.key())).build())
-                       .collect(Collectors.toList());
+        return Collections.unmodifiableList(contents.stream()
+                                                     .map(o -> o.toBuilder().key(urlDecode(o.key())).build())
+                                                     .collect(Collectors.toList()));
+    }
+
+    private static List<ObjectVersion> decodeObjectVersions(List<ObjectVersion> objectVersions) {
+        if (objectVersions == null) {
+            return null;
+        }
+
+        return Collections.unmodifiableList(objectVersions.stream()
+                                                          .map(o -> o.toBuilder().key(urlDecode(o.key())).build())
+                                                          .collect(Collectors.toList()));
+    }
+
+    private static List<CommonPrefix> decodeCommonPrefixes(List<CommonPrefix> commonPrefixes) {
+        if (commonPrefixes == null) {
+            return null;
+        }
+
+        return Collections.unmodifiableList(commonPrefixes.stream()
+                                                          .map(p -> p.toBuilder().prefix(urlDecode(p.prefix())).build())
+                                                          .collect(Collectors.toList()));
+    }
+
+    private static List<MultipartUpload> decodeMultipartUpload(List<MultipartUpload> multipartUploads) {
+        if (multipartUploads == null) {
+            return null;
+        }
+
+        return Collections.unmodifiableList(multipartUploads.stream()
+                                                            .map(u -> u.toBuilder().key(urlDecode(u.key())).build())
+                                                            .collect(Collectors.toList()));
     }
 }