Skip to content

Commit 105d545

Browse files
L-ApplinL-Applin
authored
Allow redirect for 400 - IllegalLocationConstraintException in s3 cross region client (aws#5678)
* add 400 IllegalLocationConstraint to redirect error * add 400 IllegalLocationConstraint to redirect error * changelog * checkstyle * Added test for 400 with IllegalLocationConstrain error code
1 parent 9ab4e22 commit 105d545

File tree

7 files changed

+135
-32
lines changed

7 files changed

+135
-32
lines changed

‎.changes/next-release/bugfix-AmazonS3-527812c.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": "Update the S3 client to correctly handle redirect cases for opt-in regions when crossRegionAccessEnabled is used."
6+
}

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
public final class CrossRegionUtils {
3737
public static final int REDIRECT_STATUS_CODE = 301;
3838
public static final int TEMPORARY_REDIRECT_STATUS_CODE = 307;
39+
public static final String ILLEGAL_LOCATION_CONSTRAINT_EXCEPTION_ERROR_CODE = "IllegalLocationConstraintException";
3940
public static final String AMZ_BUCKET_REGION_HEADER = "x-amz-bucket-region";
4041
private static final List<Integer> REDIRECT_STATUS_CODES =
4142
Arrays.asList(REDIRECT_STATUS_CODE, TEMPORARY_REDIRECT_STATUS_CODE);
@@ -59,7 +60,16 @@ public static boolean isS3RedirectException(Throwable exception) {
5960
}
6061

6162
private static boolean isRedirectError(S3Exception exceptionToBeChecked) {
62-
if (REDIRECT_STATUS_CODES.stream().anyMatch(status -> status.equals(exceptionToBeChecked.statusCode()))) {
63+
boolean matchRedirectStatusCode =
64+
REDIRECT_STATUS_CODES.stream()
65+
.anyMatch(status -> status.equals(exceptionToBeChecked.statusCode()));
66+
if (matchRedirectStatusCode) {
67+
return true;
68+
}
69+
boolean is400IllegalLocationConstraintException =
70+
exceptionToBeChecked.statusCode() == 400
71+
&& ILLEGAL_LOCATION_CONSTRAINT_EXCEPTION_ERROR_CODE.equals(exceptionToBeChecked.awsErrorDetails().errorCode());
72+
if (is400IllegalLocationConstraintException) {
6373
return true;
6474
}
6575
return getBucketRegionFromException(exceptionToBeChecked).isPresent();

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@ public void setup() {
4949
@Override
5050
protected void stubRedirectSuccessSuccess(Integer redirect) {
5151
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
52-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, CROSS_REGION.id(),
53-
null, null))))
52+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect,
53+
CROSS_REGION.id(),
54+
errorCodeFromRedirect(redirect),
55+
null))))
5456
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()))
5557
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()));
5658
}
@@ -77,7 +79,10 @@ protected void stubServiceClientConfiguration() {
7779
@Override
7880
protected void stubClientAPICallWithFirstRedirectThenSuccessWithRegionInErrorResponse(Integer redirect) {
7981
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
80-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, CROSS_REGION.id(), null, null))))
82+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect,
83+
CROSS_REGION.id(),
84+
errorCodeFromRedirect(redirect),
85+
null))))
8186
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()
8287
));
8388
}
@@ -112,15 +117,20 @@ protected void stubHeadBucketRedirect() {
112117
@Override
113118
protected void stubRedirectWithNoRegionAndThenSuccess(Integer redirect) {
114119
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
115-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, null, null, null))))
120+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect,
121+
null,
122+
errorCodeFromRedirect(redirect),
123+
null))))
116124
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()))
117125
.thenReturn(CompletableFuture.completedFuture(ListObjectsResponse.builder().contents(S3_OBJECTS).build()));
118126
}
119127

120128
@Override
121129
protected void stubRedirectThenError(Integer redirect) {
122130
when(mockDelegateAsyncClient.listObjects(any(ListObjectsRequest.class)))
123-
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect, CROSS_REGION.id(), null,
131+
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(redirect,
132+
CROSS_REGION.id(),
133+
errorCodeFromRedirect(redirect),
124134
null))))
125135
.thenReturn(CompletableFutureUtils.failedFuture(new CompletionException(redirectException(400, null,
126136
"InvalidArgument", "Invalid id"))));

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

Lines changed: 58 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,13 @@
2626
import static software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionRedirectTestBase.X_AMZ_BUCKET_REGION;
2727

2828
import java.net.URI;
29+
import java.time.Duration;
2930
import java.util.Arrays;
3031
import java.util.Collections;
3132
import java.util.List;
3233
import java.util.concurrent.CompletableFuture;
3334
import java.util.concurrent.CompletionException;
35+
import java.util.concurrent.ExecutionException;
3436
import java.util.function.Consumer;
3537
import java.util.stream.Collectors;
3638
import java.util.stream.Stream;
@@ -80,6 +82,7 @@
8082

8183
class S3CrossRegionAsyncClientTest {
8284

85+
private static final String ILLEGAL_LOCATION_CONSTRAINT_EXCEPTION_ERROR_CODE = "IllegalLocationConstraintException";
8386
private static final String ERROR_RESPONSE_FORMAT = "<Error>\\n\\t<Code>%s</Code>\\n</Error>";
8487
private static final String BUCKET = "bucket";
8588
private static final String KEY = "key";
@@ -97,26 +100,38 @@ void setUp() {
97100

98101
private static Stream<Arguments> stubSuccessfulRedirectResponses() {
99102
Consumer<MockAsyncHttpClient> redirectStubConsumer = mockAsyncHttpClient ->
100-
mockAsyncHttpClient.stubResponses(customHttpResponseWithUnknownErrorCode(301, CROSS_REGION.id()), successHttpResponse());
103+
mockAsyncHttpClient.stubResponses(customHttpResponseWithUnknownErrorCode(301, CROSS_REGION.id()),
104+
successHttpResponse());
101105

102106
Consumer<MockAsyncHttpClient> successStubConsumer = mockAsyncHttpClient ->
103107
mockAsyncHttpClient.stubResponses(successHttpResponse(), successHttpResponse());
104108

105109
Consumer<MockAsyncHttpClient> tempRedirectStubConsumer = mockAsyncHttpClient ->
106-
mockAsyncHttpClient.stubResponses(customHttpResponseWithUnknownErrorCode(307, CROSS_REGION.id()), successHttpResponse());
110+
mockAsyncHttpClient.stubResponses(customHttpResponseWithUnknownErrorCode(307, CROSS_REGION.id()),
111+
successHttpResponse());
112+
113+
Consumer<MockAsyncHttpClient> locationConstraintExceptionConsumer = mockAsyncHttpClient ->
114+
mockAsyncHttpClient.stubResponses(customHttpResponse(400,
115+
ILLEGAL_LOCATION_CONSTRAINT_EXCEPTION_ERROR_CODE,
116+
CROSS_REGION.id()), successHttpResponse());
117+
107118

108119
return Stream.of(
109120
Arguments.of(redirectStubConsumer, BucketEndpointProvider.class, "Redirect Error with region in x-amz-bucket-header"),
110-
Arguments.of(successStubConsumer, BucketEndpointProvider.class, "Success response" ),
111-
Arguments.of(tempRedirectStubConsumer, BucketEndpointProvider.class, "Permanent redirect Error with region in x-amz-bucket-header" )
121+
Arguments.of(successStubConsumer, BucketEndpointProvider.class, "Success response"),
122+
Arguments.of(tempRedirectStubConsumer, BucketEndpointProvider.class,
123+
"Permanent redirect Error with region in x-amz-bucket-header"),
124+
Arguments.of(locationConstraintExceptionConsumer, BucketEndpointProvider.class,
125+
"Redirect error: 400 IllegalLocationConstraintException with region in x-amz-bucket-header")
112126
);
113127
}
114128

115129

116130
private static Stream<Arguments> stubFailureResponses() {
117131

118132
List<SdkHttpMethod> noregionOnHeadBucketHttpMethodListMethodList = Arrays.asList(SdkHttpMethod.GET, SdkHttpMethod.HEAD);
119-
List<SdkHttpMethod> regionOnHeadBucketHttpMethodList = Arrays.asList(SdkHttpMethod.GET, SdkHttpMethod.HEAD, SdkHttpMethod.GET);
133+
List<SdkHttpMethod> regionOnHeadBucketHttpMethodList = Arrays.asList(SdkHttpMethod.GET, SdkHttpMethod.HEAD,
134+
SdkHttpMethod.GET);
120135
List<String> noRegionOnHeadBucket = Arrays.asList(OVERRIDE_CONFIGURED_REGION.toString(),
121136
OVERRIDE_CONFIGURED_REGION.toString());
122137

@@ -129,7 +144,8 @@ private static Stream<Arguments> stubFailureResponses() {
129144
customHttpResponseWithUnknownErrorCode(301, null),
130145
successHttpResponse(), successHttpResponse());
131146
return Stream.of(
132-
Arguments.of(redirectFailedWithNoRegionFailure, 301, 2, noRegionOnHeadBucket, noregionOnHeadBucketHttpMethodListMethodList)
147+
Arguments.of(redirectFailedWithNoRegionFailure, 301, 2, noRegionOnHeadBucket,
148+
noregionOnHeadBucketHttpMethodListMethodList)
133149
);
134150
}
135151

@@ -159,8 +175,8 @@ public static HttpExecuteResponse customHttpResponse(int statusCode, String erro
159175
@ParameterizedTest(name = "{index} - {2}.")
160176
@MethodSource("stubSuccessfulRedirectResponses")
161177
void given_CrossRegionClientWithNoOverrideConfig_when_StandardOperationIsPerformed_then_SuccessfullyIntercepts(Consumer<MockAsyncHttpClient> stubConsumer,
162-
Class<?> endpointProviderType,
163-
String testCase) {
178+
Class<?> endpointProviderType,
179+
String testCase) {
164180
stubConsumer.accept(mockAsyncHttpClient);
165181
S3AsyncClient crossRegionClient = new S3CrossRegionAsyncClient(s3Client);
166182
crossRegionClient.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join();
@@ -170,8 +186,9 @@ void given_CrossRegionClientWithNoOverrideConfig_when_StandardOperationIsPerform
170186
@ParameterizedTest(name = "{index} - {2}.")
171187
@MethodSource("stubSuccessfulRedirectResponses")
172188
void given_CrossRegionClientWithExistingOverrideConfig_when_StandardOperationIsPerformed_then_SuccessfullyIntercepts(Consumer<MockAsyncHttpClient> stubConsumer,
173-
Class<?> endpointProviderType,
174-
String testCase) {
189+
Class<
190+
?> endpointProviderType,
191+
String testCase) {
175192
stubConsumer.accept(mockAsyncHttpClient);
176193
S3AsyncClient crossRegionClient = new S3CrossRegionAsyncClient(s3Client);
177194
GetObjectRequest request = GetObjectRequest.builder()
@@ -187,8 +204,8 @@ void given_CrossRegionClientWithExistingOverrideConfig_when_StandardOperationIsP
187204
@ParameterizedTest(name = "{index} - {2}.")
188205
@MethodSource("stubSuccessfulRedirectResponses")
189206
void given_CrossRegionClient_when_PaginatedOperationIsPerformed_then_DoesNotIntercept(Consumer<MockAsyncHttpClient> stubConsumer,
190-
Class<?> endpointProviderType,
191-
String testCase) throws Exception {
207+
Class<?> endpointProviderType,
208+
String testCase) throws Exception {
192209
stubConsumer.accept(mockAsyncHttpClient);
193210
S3AsyncClient crossRegionClient = new S3CrossRegionAsyncClient(s3Client);
194211
ListObjectsV2Publisher publisher =
@@ -201,8 +218,8 @@ void given_CrossRegionClient_when_PaginatedOperationIsPerformed_then_DoesNotInte
201218
@ParameterizedTest(name = "{index} - {2}.")
202219
@MethodSource("stubSuccessfulRedirectResponses")
203220
void given_CrossRegionClientCreatedWithWrapping_when_OperationIsPerformed_then_SuccessfullyIntercepts(Consumer<MockAsyncHttpClient> stubConsumer,
204-
Class<?> endpointProviderType,
205-
String testCase) {
221+
Class<?> endpointProviderType,
222+
String testCase) {
206223
stubConsumer.accept(mockAsyncHttpClient);
207224
S3AsyncClient crossRegionClient = clientBuilder().crossRegionAccessEnabled(true).build();
208225
crossRegionClient.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join();
@@ -264,7 +281,7 @@ void given_CrossRegionClient_when_CallsHeadObjectErrors_then_ShouldTerminateTheA
264281

265282
String errorMessage = String.format("software.amazon.awssdk.services.s3.model.S3Exception: "
266283
+ "(Service: S3, Status Code: %d, Request ID: null)"
267-
, statusCode);
284+
, statusCode);
268285
assertThatExceptionOfType(CompletionException.class)
269286
.isThrownBy(() -> crossRegionClient.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join())
270287
.withMessageContaining(errorMessage);
@@ -395,7 +412,6 @@ void given_CrossRegionClient_when_StandardOperation_then_ContainsUserAgent() {
395412
}
396413

397414

398-
399415
@ParameterizedTest(name = "{index} - {2}.")
400416
@MethodSource("stubSuccessfulRedirectResponses")
401417
void given_CrossRegionAccessEnabled_when_SuccessfulResponse_then_EndpointIsUpdated(Consumer<MockAsyncHttpClient> stubConsumer,
@@ -416,7 +432,7 @@ void given_SimpleClient_when_StandardOperation_then_DoesNotContainCrossRegionUse
416432
}
417433

418434
@Test
419-
void given_US_EAST_1_Client_resolvesToGlobalEndpoints_when_crossRegion_is_False(){
435+
void given_US_EAST_1_Client_resolvesToGlobalEndpoints_when_crossRegion_is_False() {
420436
mockAsyncHttpClient.stubResponses(successHttpResponse());
421437
S3AsyncClient s3Client = clientBuilder().region(Region.US_EAST_1).build();
422438
s3Client.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join();
@@ -425,7 +441,7 @@ void given_US_EAST_1_Client_resolvesToGlobalEndpoints_when_crossRegion_is_False(
425441
}
426442

427443
@Test
428-
void given_US_EAST_1_Client_resolveToRegionalEndpoints_when_crossRegion_is_True(){
444+
void given_US_EAST_1_Client_resolveToRegionalEndpoints_when_crossRegion_is_True() {
429445
mockAsyncHttpClient.stubResponses(successHttpResponse());
430446
S3AsyncClient s3Client = clientBuilder().crossRegionAccessEnabled(true).region(Region.US_EAST_1).build();
431447
s3Client.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes()).join();
@@ -478,6 +494,30 @@ void given_globalRegion_Client_Updates_region_to_useast1_and_useGlobalEndpointFl
478494

479495
}
480496

497+
@Test
498+
void given_CrossRegionClient_when400Error_WithoutIllegalLocationConstraint_DoesNotRedirect() {
499+
String region = Region.AWS_GLOBAL.id();
500+
mockAsyncHttpClient.stubResponses(customHttpResponse(400, "UnknownError", null));
501+
S3EndpointProvider mockEndpointProvider = Mockito.mock(S3EndpointProvider.class);
502+
when(mockEndpointProvider.resolveEndpoint(ArgumentMatchers.any(S3EndpointParams.class)))
503+
.thenReturn(CompletableFuture.completedFuture(Endpoint.builder().url(URI.create("https://bucket.s3.amazonaws.com")).build()));
504+
505+
S3AsyncClient s3Client = clientBuilder().crossRegionAccessEnabled(true)
506+
.region(Region.of(region))
507+
.endpointProvider(mockEndpointProvider).build();
508+
509+
CompletableFuture<ResponseBytes<GetObjectResponse>> f =
510+
s3Client.getObject(r -> r.bucket(BUCKET).key(KEY), AsyncResponseTransformer.toBytes());
511+
512+
assertThat(f).failsWithin(Duration.ofSeconds(5))
513+
.withThrowableOfType(ExecutionException.class)
514+
.withCauseInstanceOf(S3Exception.class)
515+
.withMessageContaining("Status Code: 400");
516+
517+
assertThat(mockAsyncHttpClient.getRequests()).hasSize(1);
518+
}
519+
520+
481521
private S3AsyncClientBuilder clientBuilder() {
482522
return S3AsyncClient.builder()
483523
.httpClient(mockAsyncHttpClient)

0 commit comments

Comments
 (0)