Skip to content

Commit 4fae33d

Browse files
joviegasjoviegas
authored
Fix to handle temporary Redirect 307 for S3 Cross Region client (#4197)
* Fix to handle temporary Redirect 307 for S3 Cross Region client * Handled Review comments
1 parent f527d4b commit 4fae33d

File tree

7 files changed

+66
-38
lines changed

7 files changed

+66
-38
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-4620073.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Amazon S3",
4+
"contributor": "",
5+
"description": "Handle Temporary redirect error 307 when client configured in us-east-1 accesses cross region."
6+
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionCrtIntegrationTest.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,14 @@
2020
import org.junit.jupiter.api.AfterAll;
2121
import org.junit.jupiter.api.BeforeAll;
2222
import org.junit.jupiter.api.BeforeEach;
23+
import org.junit.jupiter.api.Test;
24+
import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider;
25+
import software.amazon.awssdk.http.apache.ApacheHttpClient;
26+
import software.amazon.awssdk.regions.Region;
2327
import software.amazon.awssdk.services.s3.S3AsyncClient;
28+
import software.amazon.awssdk.services.s3.S3Client;
29+
import software.amazon.awssdk.services.s3.model.ListObjectsRequest;
30+
import software.amazon.awssdk.services.s3.model.ListObjectsResponse;
2431

2532
public class S3CrossRegionCrtIntegrationTest extends S3CrossRegionAsyncIntegrationTestBase {
2633
private static final String BUCKET = temporaryBucketName(S3CrossRegionCrtIntegrationTest.class);
@@ -41,7 +48,7 @@ public void initialize() {
4148
crossRegionS3Client = S3AsyncClient.crtBuilder()
4249
.region(CROSS_REGION)
4350
.crossRegionAccessEnabled(true)
44-
.build();
51+
.build();
4552
}
4653
@Override
4754
protected String bucketName() {

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/crossregion/S3CrossRegionSyncIntegrationTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import software.amazon.awssdk.core.ResponseBytes;
2727
import software.amazon.awssdk.core.sync.RequestBody;
2828
import software.amazon.awssdk.core.sync.ResponseTransformer;
29+
import software.amazon.awssdk.regions.Region;
2930
import software.amazon.awssdk.services.s3.S3Client;
3031
import software.amazon.awssdk.services.s3.model.DeleteObjectRequest;
3132
import software.amazon.awssdk.services.s3.model.DeleteObjectResponse;
@@ -59,7 +60,7 @@ static void clearClass() {
5960
@BeforeEach
6061
public void initialize() {
6162
crossRegionS3Client = S3Client.builder()
62-
.region(CROSS_REGION)
63+
.region(Region.US_EAST_1)
6364
.crossRegionAccessEnabled(true)
6465
.build();
6566
}

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crossregion/utils/CrossRegionUtils.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
@SdkInternalApi
3434
public final class CrossRegionUtils {
3535
public static final int REDIRECT_STATUS_CODE = 301;
36+
public static final int TEMPORARY_REDIRECT_STATUS_CODE = 307;
3637
public static final String AMZ_BUCKET_REGION_HEADER = "x-amz-bucket-region";
3738
private static final ApiName API_NAME = ApiName.builder().version("cross-region").name("hll").build();
3839
private static final Consumer<AwsRequestOverrideConfiguration.Builder> USER_AGENT_APPLIER = b -> b.addApiName(API_NAME);
@@ -48,9 +49,14 @@ public static Optional<String> getBucketRegionFromException(S3Exception exceptio
4849
}
4950

5051
public static boolean isS3RedirectException(Throwable exception) {
51-
Throwable exceptionToBeChecked = exception instanceof CompletionException ? exception.getCause() : exception ;
52+
Throwable exceptionToBeChecked = exception instanceof CompletionException ? exception.getCause() : exception;
5253
return exceptionToBeChecked instanceof S3Exception
53-
&& ((S3Exception) exceptionToBeChecked).statusCode() == REDIRECT_STATUS_CODE;
54+
&& isRedirectError((S3Exception) exceptionToBeChecked);
55+
}
56+
57+
private static boolean isRedirectError(S3Exception exceptionToBeChecked) {
58+
int statusCode = exceptionToBeChecked.statusCode();
59+
return statusCode == REDIRECT_STATUS_CODE || statusCode == TEMPORARY_REDIRECT_STATUS_CODE;
5460
}
5561

5662

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionAsyncClientRedirectTest.java

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,10 @@ public void setup() {
4747
}
4848

4949
@Override
50-
protected void stubRedirectSuccessSuccess() {
50+
protected void stubRedirectSuccessSuccess(Integer redirect) {
5151
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
52-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(301, CROSS_REGION.id(), null, null))))
52+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, CROSS_REGION.id(),
53+
null, null))))
5354
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()))
5455
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()));
5556
}
@@ -74,9 +75,9 @@ protected void stubServiceClientConfiguration() {
7475
}
7576

7677
@Override
77-
protected void stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse() {
78+
protected void stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse(Integer redirect) {
7879
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
79-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(301, CROSS_REGION.id(), null,
80+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, CROSS_REGION.id(), null,
8081
null))))
8182
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()
8283
));
@@ -110,17 +111,17 @@ protected void stubHeadBucketRedirect() {
110111
}
111112

112113
@Override
113-
protected void stubRedirectWithNoRegionAndThenSuccess() {
114+
protected void stubRedirectWithNoRegionAndThenSuccess(Integer redirect) {
114115
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
115-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(301, null, null, null))))
116+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, null, null, null))))
116117
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()))
117118
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()));
118119
}
119120

120121
@Override
121-
protected void stubRedirectThenError() {
122+
protected void stubRedirectThenError(Integer redirect) {
122123
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
123-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(301, CROSS_REGION.id(), null,
124+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, CROSS_REGION.id(), null,
124125
null))))
125126
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(400, null,
126127
"InvalidArgument", "Invalid id"))));

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3CrossRegionSyncClientRedirectTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,23 +78,23 @@ protected void stubHeadBucketRedirect() {
7878
}
7979

8080
@Override
81-
protected void stubRedirectWithNoRegionAndThenSuccess() {
81+
protected void stubRedirectWithNoRegionAndThenSuccess(Integer redirect) {
8282
when(mockDelegateClient.listObjects(any(ListObjectsRequest.class)))
83-
.thenThrow(redirectException(301, null, null, null))
83+
.thenThrow(redirectException(redirect, null, null, null))
8484
.thenReturn(ListObjectsResponse.builder().contents(S3_OBJECTS).build());
8585
}
8686

8787
@Override
88-
protected void stubRedirectThenError() {
88+
protected void stubRedirectThenError(Integer redirect) {
8989
when(mockDelegateClient.listObjects(any(ListObjectsRequest.class)))
90-
.thenThrow(redirectException(301, CROSS_REGION.id(), null, null))
90+
.thenThrow(redirectException(redirect, CROSS_REGION.id(), null, null))
9191
.thenThrow(redirectException(400, null, "InvalidArgument", "Invalid id"));
9292
}
9393

9494
@Override
95-
protected void stubRedirectSuccessSuccess() {
95+
protected void stubRedirectSuccessSuccess(Integer redirect) {
9696
when(mockDelegateClient.listObjects(any(ListObjectsRequest.class)))
97-
.thenThrow(redirectException(301, CROSS_REGION.id(), null, null))
97+
.thenThrow(redirectException(redirect, CROSS_REGION.id(), null, null))
9898
.thenReturn(ListObjectsResponse.builder().contents(S3_OBJECTS).build())
9999
.thenReturn(ListObjectsResponse.builder().contents(S3_OBJECTS).build());
100100
}
@@ -120,7 +120,7 @@ protected void stubServiceClientConfiguration() {
120120
}
121121

122122
@Override
123-
protected void stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse() {
123+
protected void stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse(Integer redirect) {
124124
when(mockDelegateClient.listObjects(any(ListObjectsRequest.class)))
125125
.thenThrow(redirectException(301, CROSS_REGION.id(), null, null))
126126
.thenReturn(ListObjectsResponse.builder().contents(S3_OBJECTS).build());

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/S3DecoratorRedirectTestBase.java

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import java.util.Collections;
2222
import java.util.List;
2323
import org.junit.jupiter.api.Test;
24+
import org.junit.jupiter.params.ParameterizedTest;
25+
import org.junit.jupiter.params.provider.ValueSource;
2426
import org.mockito.ArgumentCaptor;
2527
import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
2628
import software.amazon.awssdk.awscore.exception.AwsServiceException;
@@ -51,10 +53,11 @@ public abstract class S3DecoratorRedirectTestBase {
5153
protected static final S3ServiceClientConfiguration CONFIGURED_ENDPOINT_PROVIDER =
5254
S3ServiceClientConfiguration.builder().endpointProvider(S3EndpointProvider.defaultProvider()).build();
5355

54-
@Test
55-
void decoratorAttemptsToRetryWithRegionNameInErrorResponse() throws Throwable {
56+
@ParameterizedTest
57+
@ValueSource(ints = {301, 307})
58+
void decoratorAttemptsToRetryWithRegionNameInErrorResponse(Integer redirect) throws Throwable {
5659
stubServiceClientConfiguration();
57-
stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse();
60+
stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse(redirect);
5861
// Assert retrieved listObject
5962
ListObjectsResponse listObjectsResponse = apiCallToService();
6063
assertThat(listObjectsResponse.contents()).isEqualTo(S3_OBJECTS);
@@ -68,10 +71,11 @@ void decoratorAttemptsToRetryWithRegionNameInErrorResponse() throws Throwable {
6871
verifyHeadBucketServiceCall(0);
6972
}
7073

71-
@Test
72-
void decoratorUsesCache_when_CrossRegionAlreadyPresent() throws Throwable {
74+
@ParameterizedTest
75+
@ValueSource(ints = {301, 307})
76+
void decoratorUsesCache_when_CrossRegionAlreadyPresent(Integer redirect) throws Throwable {
7377
stubServiceClientConfiguration();
74-
stubRedirectSuccessSuccess();
78+
stubRedirectSuccessSuccess(redirect);
7579

7680
ListObjectsResponse listObjectsResponse = apiCallToService();
7781
assertThat(listObjectsResponse.contents()).isEqualTo(S3_OBJECTS);
@@ -93,20 +97,22 @@ void decoratorUsesCache_when_CrossRegionAlreadyPresent() throws Throwable {
9397
* The redirected call fails because of incorrect parameters passed
9498
* This exception should be reported correctly
9599
*/
96-
@Test
97-
void apiCallFailure_when_CallFailsAfterRedirection() {
100+
@ParameterizedTest
101+
@ValueSource(ints = {301, 307})
102+
void apiCallFailure_when_CallFailsAfterRedirection(Integer redirectError) {
98103
stubServiceClientConfiguration();
99-
stubRedirectThenError();
104+
stubRedirectThenError(redirectError);
100105
assertThatExceptionOfType(S3Exception.class)
101106
.isThrownBy(() -> apiCallToService())
102107
.withMessageContaining("Invalid id (Service: S3, Status Code: 400, Request ID: 1, Extended Request ID: A1)");
103108
verifyHeadBucketServiceCall(0);
104109
}
105110

106-
@Test
107-
void headBucketCalled_when_RedirectDoesNotHasRegionName() throws Throwable {
111+
@ParameterizedTest
112+
@ValueSource(ints = {301, 307})
113+
void headBucketCalled_when_RedirectDoesNotHasRegionName(Integer redirect) throws Throwable {
108114
stubServiceClientConfiguration();
109-
stubRedirectWithNoRegionAndThenSuccess();
115+
stubRedirectWithNoRegionAndThenSuccess(redirect);
110116
stubHeadBucketRedirect();
111117
ListObjectsResponse listObjectsResponse = apiCallToService();
112118
assertThat(listObjectsResponse.contents()).isEqualTo(S3_OBJECTS);
@@ -119,10 +125,11 @@ void headBucketCalled_when_RedirectDoesNotHasRegionName() throws Throwable {
119125
verifyHeadBucketServiceCall(1);
120126
}
121127

122-
@Test
123-
void headBucketCalledAndCached__when_RedirectDoesNotHasRegionName() throws Throwable {
128+
@ParameterizedTest
129+
@ValueSource(ints = {301, 307})
130+
void headBucketCalledAndCached__when_RedirectDoesNotHasRegionName(Integer redirect) throws Throwable {
124131
stubServiceClientConfiguration();
125-
stubRedirectWithNoRegionAndThenSuccess();
132+
stubRedirectWithNoRegionAndThenSuccess(redirect);
126133
stubHeadBucketRedirect();
127134
ListObjectsResponse listObjectsResponse = apiCallToService();
128135
assertThat(listObjectsResponse.contents()).isEqualTo(S3_OBJECTS);
@@ -165,11 +172,11 @@ void requestsAreNotOverridden_when_NoBucketInRequest() throws Throwable {
165172

166173
protected abstract void stubHeadBucketRedirect();
167174

168-
protected abstract void stubRedirectWithNoRegionAndThenSuccess();
175+
protected abstract void stubRedirectWithNoRegionAndThenSuccess(Integer redirect);
169176

170-
protected abstract void stubRedirectThenError();
177+
protected abstract void stubRedirectThenError(Integer redirect);
171178

172-
protected abstract void stubRedirectSuccessSuccess();
179+
protected abstract void stubRedirectSuccessSuccess(Integer redirect);
173180

174181
protected AwsServiceException redirectException(int statusCode, String region, String errorCode, String errorMessage) {
175182
SdkHttpFullResponse.Builder sdkHttpFullResponseBuilder = SdkHttpFullResponse.builder();
@@ -210,5 +217,5 @@ void verifyTheEndPointProviderOverridden(int attempt,
210217

211218
protected abstract void stubServiceClientConfiguration();
212219

213-
protected abstract void stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse();
220+
protected abstract void stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse(Integer redirect);
214221
}

0 commit comments

Comments
 (0)