Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/next-release/bugfix-AmazonS3-0f180ea.json
Original file line number Diff line number Diff line change
@@ -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."
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -154,7 +155,8 @@ public URL getUrl(GetUrlRequest getUrlRequest) {
getObjectRequest,
resolvedRegion,
s3Configuration,
endpointOverridden);
endpointOverridden)
.sdkHttpRequest();

try {
return httpRequest.getUri().toURL();
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand Down Expand Up @@ -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();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}
}
Loading