Fixed an issue where presigned URLs for access point objects could bypass encoding, causing an IllegalArgumentException

Author: millems

.changes/next-release/bugfix-AmazonS3-8e119e7.json

@@ -0,0 +1,6 @@
+{
+    "category": "Amazon S3", 
+    "contributor": "", 
+    "type": "bugfix", 
+    "description": "Fixed an issue where presigned URLs for access point objects could bypass encoding, causing an IllegalArgumentException."
+}

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

@@ -17,7 +17,6 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.HttpURLConnection;
@@ -30,9 +29,7 @@
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
-
 import software.amazon.awssdk.awscore.presigner.PresignedRequest;
-import software.amazon.awssdk.core.ResponseInputStream;
 import software.amazon.awssdk.core.SdkBytes;
 import software.amazon.awssdk.core.sync.RequestBody;
 import software.amazon.awssdk.http.AbortableInputStream;
@@ -45,8 +42,6 @@
 import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest;
 import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
 import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
-import software.amazon.awssdk.services.s3.model.GetObjectResponse;
-import software.amazon.awssdk.services.s3.model.ListMultipartUploadsResponse;
 import software.amazon.awssdk.services.s3.model.MultipartUpload;
 import software.amazon.awssdk.services.s3.model.RequestPayer;
 import software.amazon.awssdk.services.s3.model.UploadPartRequest;
@@ -116,7 +111,7 @@ public void bucketsWithScaryCharactersWorks() throws IOException {
 
     @Test
     public void keysWithScaryCharactersWorks() throws IOException {
-        String scaryObjectKey = testGetObjectKey + " !'/()~`";
+        String scaryObjectKey = "a0A!-_.*'()&@:,$=+?; \n\\^`<>{}[]#%\"~|山/" + testGetObjectKey;
         S3TestUtils.putObject(S3PresignerIntegrationTest.class, client, testBucket, scaryObjectKey, testObjectContent);
 
         assertThatPresigningWorks(testBucket, scaryObjectKey);

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/endpoints/S3AccessPointEndpointResolver.java

@@ -39,6 +39,7 @@
 import software.amazon.awssdk.services.s3.internal.resource.S3Resource;
 import software.amazon.awssdk.services.s3.internal.resource.S3ResourceType;
 import software.amazon.awssdk.utils.Validate;
+import software.amazon.awssdk.utils.http.SdkHttpUtils;
 
 /**
  * Returns a new configured HTTP request with a resolved access point endpoint and signing overrides.
@@ -107,7 +108,7 @@ private String buildPath(URI accessPointUri, S3EndpointResolverContext context)
             if (pathBuilder.length() > 0) {
                 pathBuilder.append('/');
             }
-            pathBuilder.append(key);
+            pathBuilder.append(SdkHttpUtils.urlEncodeIgnoreSlashes(key));
         }
         return pathBuilder.length() > 0 ? pathBuilder.toString() : null;
     }
@@ -117,7 +118,6 @@ private String validateConfiguration(S3EndpointResolverContext context, S3Resour
         String arnRegion = s3Resource.region().orElseThrow(() -> new IllegalArgumentException(
             "An S3 access point ARN must have a region"));
 
-
         S3Configuration serviceConfiguration = context.serviceConfiguration();
         if (isAccelerateEnabled(serviceConfiguration)) {
             throw new IllegalArgumentException("An access point ARN cannot be passed as a bucket parameter to an S3 "

services/s3control/src/it/java/software.amazon.awssdk.services.s3control/S3AccessPointsIntegrationTest.java

@@ -17,17 +17,28 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertNotNull;
 import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
+import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
 
+import java.io.IOException;
+import java.time.Duration;
 import java.util.StringJoiner;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import software.amazon.awssdk.core.sync.RequestBody;
+import software.amazon.awssdk.http.HttpExecuteRequest;
+import software.amazon.awssdk.http.HttpExecuteResponse;
+import software.amazon.awssdk.http.SdkHttpClient;
+import software.amazon.awssdk.http.SdkHttpRequest;
+import software.amazon.awssdk.http.apache.ApacheHttpClient;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.model.GetObjectRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
+import software.amazon.awssdk.services.s3.presigner.S3Presigner;
 import software.amazon.awssdk.services.sts.StsClient;
+import software.amazon.awssdk.utils.IoUtils;
+import software.amazon.awssdk.utils.StringInputStream;
 
 public class S3AccessPointsIntegrationTest extends S3ControlIntegrationTestBase {
 
@@ -108,6 +119,51 @@ public void transfer_Succeeds_UsingAccessPoint_CrossRegion() {
         assertThat(objectContent).isEqualTo("helloworld");
     }
 
+    @Test
+    public void uploadAndDownloadWithPresignedUrlWorks() throws IOException {
+        String accessPointArn = new StringJoiner(":").add("arn").add("aws").add("s3").add("us-west-2").add(accountId)
+                                                     .add("accesspoint").add(AP_NAME).toString();
+        String key = "foo/a0A!-_.*'()&@:,$=+?; \n\\^`<>{}[]#%\"~|山";
+
+        testAccessPointPresigning(accessPointArn, key);
+    }
+
+    private void testAccessPointPresigning(String accessPointArn, String key) throws IOException {
+        String data = "Hello";
+
+        S3Presigner presigner = S3Presigner.builder().region(Region.US_WEST_2).build();
+
+        SdkHttpRequest presignedPut = presigner.presignPutObject(r -> r.signatureDuration(Duration.ofDays(7))
+                                                                       .putObjectRequest(por -> por.bucket(accessPointArn)
+                                                                                                   .key(key)))
+                                               .httpRequest();
+
+
+        SdkHttpRequest presignedGet = presigner.presignGetObject(r -> r.signatureDuration(Duration.ofDays(7))
+                                                                       .getObjectRequest(gor -> gor.bucket(accessPointArn)
+                                                                                                   .key(key)))
+                                               .httpRequest();
+
+        try (SdkHttpClient client = ApacheHttpClient.create()) {
+            client.prepareRequest(HttpExecuteRequest.builder()
+                                                    .request(presignedPut)
+                                                    .contentStreamProvider(() -> new StringInputStream(data))
+                                                    .build())
+                  .call();
+
+            HttpExecuteResponse getResult = client.prepareRequest(HttpExecuteRequest.builder()
+                                                                                    .request(presignedGet)
+                                                                                    .build())
+                                             .call();
+
+            String result = getResult.responseBody()
+                                     .map(stream -> invokeSafely(() -> IoUtils.toUtf8String(stream)))
+                                     .orElseThrow(AssertionError::new);
+
+            assertThat(result).isEqualTo(data);
+        }
+    }
+
     @Test
     public void accessPointOperation_nonArns() {
         assertNotNull(s3control.listAccessPoints(b -> b.bucket(BUCKET).accountId(accountId).maxResults(1)));