S3: Interacting with an access point in a different region to the one…

… the S3 client is configured for will no longer result in the request being signed for the wrong region and rejected by S3.
aws · Dec 4, 2019 · 1d2d644 · 1d2d644
1 parent d96b688
commit 1d2d644
Show file tree

Hide file tree

Showing 7 changed files with 198 additions and 26 deletions.
diff --git a/.changes/next-release/bugfix-AmazonS3-0f180ea.json b/.changes/next-release/bugfix-AmazonS3-0f180ea.json
@@ -0,0 +1,5 @@
+{
+    "type": "bugfix",
+    "category": "Amazon S3",
+    "description": "Interacting with an access point in a different region to the one the S3 client is configured for will no longer result in the request being signed for the wrong region and rejected by S3."
+}
diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/AccessPointsIntegrationTest.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/AccessPointsIntegrationTest.java
@@ -84,4 +84,25 @@ public void transfer_Succeeds_UsingAccessPoint() {
 
         assertThat(objectContent).isEqualTo("helloworld");
     }
+
+    @Test
+    public void transfer_Succeeds_UsingAccessPoint_CrossRegion() {
+        S3Client s3DifferentRegion =
+            s3ClientBuilder().region(Region.US_EAST_1).serviceConfiguration(c -> c.useArnRegionEnabled(true)).build();
+
+        StringJoiner apArn = new StringJoiner(":");
+        apArn.add("arn").add("aws").add("s3").add("us-west-2").add(accountId).add("accesspoint").add(AP_NAME);
+
+        s3DifferentRegion.putObject(PutObjectRequest.builder()
+                                                    .bucket(apArn.toString())
+                                                    .key(KEY)
+                                                    .build(), RequestBody.fromString("helloworld"));
+
+        String objectContent = s3DifferentRegion.getObjectAsBytes(GetObjectRequest.builder()
+                                                                                  .bucket(apArn.toString())
+                                                                                  .key(KEY)
+                                                                                  .build()).asUtf8String();
+
+        assertThat(objectContent).isEqualTo("helloworld");
+    }
 }
diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Utilities.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Utilities.java
@@ -19,6 +19,7 @@
 import java.net.URI;
 import java.net.URL;
 import java.util.function.Consumer;
+
 import software.amazon.awssdk.annotations.Immutable;
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.annotations.SdkPublicApi;
@@ -154,7 +155,8 @@ public URL getUrl(GetUrlRequest getUrlRequest) {
                                                                                 getObjectRequest,
                                                                                 resolvedRegion,
                                                                                 s3Configuration,
-                                                                                endpointOverridden);
+                                                                                endpointOverridden)
+                                                    .sdkHttpRequest();
 
         try {
             return httpRequest.getUri().toURL();

diff --git a/...src/main/java/software/amazon/awssdk/services/s3/internal/ConfiguredS3SdkHttpRequest.java b/...src/main/java/software/amazon/awssdk/services/s3/internal/ConfiguredS3SdkHttpRequest.java
@@ -0,0 +1,103 @@
+/*
+ * Copyright 2010-2019 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.internal;
+
+import java.util.Optional;
+
+import software.amazon.awssdk.annotations.SdkInternalApi;
+import software.amazon.awssdk.http.SdkHttpRequest;
+import software.amazon.awssdk.regions.Region;
+import software.amazon.awssdk.utils.Validate;
+import software.amazon.awssdk.utils.builder.CopyableBuilder;
+import software.amazon.awssdk.utils.builder.ToCopyableBuilder;
+
+@SdkInternalApi
+public class ConfiguredS3SdkHttpRequest
+        implements ToCopyableBuilder<ConfiguredS3SdkHttpRequest.Builder, ConfiguredS3SdkHttpRequest> {
+    private final SdkHttpRequest sdkHttpRequest;
+    private final Region signingRegionModification;
+
+    private ConfiguredS3SdkHttpRequest(Builder builder) {
+        this.sdkHttpRequest = Validate.notNull(builder.sdkHttpRequest, "sdkHttpRequest");
+        this.signingRegionModification = builder.signingRegionModification;
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    public SdkHttpRequest sdkHttpRequest() {
+        return sdkHttpRequest;
+    }
+
+    public Optional<Region> signingRegionModification() {
+        return Optional.ofNullable(signingRegionModification);
+    }
+
+    @Override
+    public Builder toBuilder() {
+        return builder().sdkHttpRequest(sdkHttpRequest).signingRegionModification(signingRegionModification);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+
+        ConfiguredS3SdkHttpRequest that = (ConfiguredS3SdkHttpRequest) o;
+
+        if (sdkHttpRequest != null ? ! sdkHttpRequest.equals(that.sdkHttpRequest) : that.sdkHttpRequest != null) {
+            return false;
+        }
+        return signingRegionModification != null ?
+            signingRegionModification.equals(that.signingRegionModification)
+            : that.signingRegionModification == null;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = sdkHttpRequest != null ? sdkHttpRequest.hashCode() : 0;
+        result = 31 * result + (signingRegionModification != null ? signingRegionModification.hashCode() : 0);
+        return result;
+    }
+
+    public static class Builder implements CopyableBuilder<Builder, ConfiguredS3SdkHttpRequest> {
+        private SdkHttpRequest sdkHttpRequest;
+        private Region signingRegionModification;
+
+        private Builder() {}
+
+        public Builder sdkHttpRequest(SdkHttpRequest sdkHttpRequest) {
+            this.sdkHttpRequest = sdkHttpRequest;
+            return this;
+        }
+
+        public Builder signingRegionModification(Region signingRegionModification) {
+            this.signingRegionModification = signingRegionModification;
+            return this;
+        }
+
+        @Override
+        public ConfiguredS3SdkHttpRequest build() {
+            return new ConfiguredS3SdkHttpRequest(this);
+        }
+    }
+
+}
diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/S3EndpointUtils.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/S3EndpointUtils.java
@@ -21,6 +21,7 @@
 import java.net.URISyntaxException;
 import java.util.Arrays;
 import java.util.List;
+
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.arns.Arn;
 import software.amazon.awssdk.core.SdkRequest;
@@ -56,11 +57,11 @@ private S3EndpointUtils() {
      * Returns a new instance of the given {@link SdkHttpRequest} by applying any endpoint changes based on
      * the given {@link S3Configuration} options.
      */
-    public static SdkHttpRequest applyEndpointConfiguration(SdkHttpRequest request,
-                                                            SdkRequest originalRequest,
-                                                            Region region,
-                                                            S3Configuration serviceConfiguration,
-                                                            boolean endpointOverridden) {
+    public static ConfiguredS3SdkHttpRequest applyEndpointConfiguration(SdkHttpRequest request,
+                                                                        SdkRequest originalRequest,
+                                                                        Region region,
+                                                                        S3Configuration serviceConfiguration,
+                                                                        boolean endpointOverridden) {
         String bucketName = originalRequest.getValueForField("Bucket", String.class).orElse(null);
         String key = originalRequest.getValueForField("Key", String.class).orElse(null);
 
@@ -82,15 +83,19 @@ public static SdkHttpRequest applyEndpointConfiguration(SdkHttpRequest request,
             }
         }
 
-        return mutableRequest.build();
+        return ConfiguredS3SdkHttpRequest.builder()
+                                         .sdkHttpRequest(mutableRequest.build())
+                                         .build();
     }
 
-    private static SdkHttpRequest applyEndpointConfigurationForAccessPointArn(SdkHttpRequest request,
-                                                                              Region region,
-                                                                              boolean endpointOverridden,
-                                                                              S3Configuration serviceConfiguration,
-                                                                              String bucketName,
-                                                                              String key) {
+    private static ConfiguredS3SdkHttpRequest applyEndpointConfigurationForAccessPointArn(
+            SdkHttpRequest request,
+            Region region,
+            boolean endpointOverridden,
+            S3Configuration serviceConfiguration,
+            String bucketName,
+            String key) {
+
         Arn resourceArn = Arn.fromString(bucketName);
         S3Resource s3Resource = S3ArnConverter.create().convertArn(resourceArn);
 
@@ -169,12 +174,17 @@ private static SdkHttpRequest applyEndpointConfigurationForAccessPointArn(SdkHtt
                                 .dualstackEnabled(dualstackEnabled)
                                 .toUri();
 
-        return request.toBuilder()
-                      .protocol(accessPointUri.getScheme())
-                      .host(accessPointUri.getHost())
-                      .port(accessPointUri.getPort())
-                      .encodedPath(key)
-                      .build();
+        SdkHttpRequest httpRequest = request.toBuilder()
+                                            .protocol(accessPointUri.getScheme())
+                                            .host(accessPointUri.getHost())
+                                            .port(accessPointUri.getPort())
+                                            .encodedPath(key)
+                                            .build();
+
+        return ConfiguredS3SdkHttpRequest.builder()
+                                         .sdkHttpRequest(httpRequest)
+                                         .signingRegionModification(Region.of(arnRegion))
+                                         .build();
     }
 
     /**

diff --git a/...java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptor.java b/...java/software/amazon/awssdk/services/s3/internal/handlers/EndpointAddressInterceptor.java
@@ -24,6 +24,7 @@
 import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute;
 import software.amazon.awssdk.http.SdkHttpRequest;
 import software.amazon.awssdk.services.s3.S3Configuration;
+import software.amazon.awssdk.services.s3.internal.ConfiguredS3SdkHttpRequest;
 import software.amazon.awssdk.services.s3.internal.S3EndpointUtils;
 
 @SdkInternalApi
@@ -32,11 +33,17 @@ public final class EndpointAddressInterceptor implements ExecutionInterceptor {
     @Override
     public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context,
                                             ExecutionAttributes executionAttributes) {
-        return S3EndpointUtils.applyEndpointConfiguration(
-            context.httpRequest(),
-            context.request(),
-            executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION),
-            (S3Configuration) executionAttributes.getAttribute(AwsSignerExecutionAttribute.SERVICE_CONFIG),
-            Boolean.TRUE.equals(executionAttributes.getAttribute(SdkExecutionAttribute.ENDPOINT_OVERRIDDEN)));
+        ConfiguredS3SdkHttpRequest configuredRequest =
+            S3EndpointUtils.applyEndpointConfiguration(
+                    context.httpRequest(),
+                    context.request(),
+                    executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION),
+                    (S3Configuration) executionAttributes.getAttribute(AwsSignerExecutionAttribute.SERVICE_CONFIG),
+                    Boolean.TRUE.equals(executionAttributes.getAttribute(SdkExecutionAttribute.ENDPOINT_OVERRIDDEN)));
+
+        configuredRequest.signingRegionModification().ifPresent(
+            region -> executionAttributes.putAttribute(AwsSignerExecutionAttribute.SIGNING_REGION, region));
+
+        return configuredRequest.sdkHttpRequest();
     }
 }