diff --git a/.changes/next-release/bugfix-AmazonS3-61c4a30.json b/.changes/next-release/bugfix-AmazonS3-61c4a30.json new file mode 100644 index 000000000000..efcd58ddc47e --- /dev/null +++ b/.changes/next-release/bugfix-AmazonS3-61c4a30.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Amazon S3", + "contributor": "", + "description": "Always set 'Expect: 100-continue' when using PUT operations across regions; this enables the correct redirect behavior when the initial request goes to an incorrect region." +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Configuration.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Configuration.java index 50e5f27c9a82..697601df9ab9 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Configuration.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3Configuration.java @@ -248,6 +248,9 @@ public boolean chunkedEncodingEnabled() { * By default, the SDK sends the {@code Expect: 100-continue} header for these operations, allowing the server to * reject the request before the client sends the full payload. Setting this to {@code false} disables this behavior. *

+ * If enabling cross region access on the client, this setting has no effect as the client needs to set this header + * for correct redirect behavior. + *

* Note: When using the {@code ApacheHttpClient} (Apache 4), the Apache 4 client also independently adds the * {@code Expect: 100-continue} header by default via its own {@code expectContinueEnabled} setting. To fully * suppress the header on the wire, you must also disable it on the Apache4 HTTP client builder using @@ -268,6 +271,9 @@ public boolean expectContinueEnabled() { *

* The default value is 1048576 bytes (1 MB). *

+ * If enabling cross region access on the client, this setting has no effect as the client needs to set this header + * for correct redirect behavior. + *

* Note: When using the {@code ApacheHttpClient} (Apache 4), the Apache 4 client also independently adds the * {@code Expect: 100-continue} header by default without any threshold via its own {@code expectContinueEnabled} * setting. To benefit from the `expectContinueThresholdInBytes` you must disable {@code expectContinueEnabled} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/StreamingRequestInterceptor.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/StreamingRequestInterceptor.java index 45c7cfe15788..cce812afa646 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/StreamingRequestInterceptor.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/StreamingRequestInterceptor.java @@ -22,10 +22,13 @@ import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute; +import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; import software.amazon.awssdk.http.SdkHttpRequest; import software.amazon.awssdk.services.s3.S3Configuration; +import software.amazon.awssdk.services.s3.endpoints.S3ClientContextParams; import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.UploadPartRequest; +import software.amazon.awssdk.utils.AttributeMap; /** * Interceptor to add an 'Expect: 100-continue' header to the HTTP Request if it represents a PUT Object or Upload Part @@ -55,6 +58,13 @@ private boolean shouldAddExpectContinueHeader(Context.ModifyHttpRequest context, return false; } + // The header is necessary for cross region PUT because sending the body unconditionally to the wrong region where S3 + // will respond with a 3xx and close the connection will cause I/O errors rather than resulting in the client retrying + // based on the region given in the 3xx response. + if (isCrossRegionAccessEnabled(executionAttributes)) { + return true; + } + S3Configuration s3Config = getS3Configuration(executionAttributes); if (s3Config != null && !s3Config.expectContinueEnabled()) { @@ -88,4 +98,12 @@ private Optional getContentLengthHeader(SdkHttpRequest httpRequest) { ? decodedLength : httpRequest.firstMatchingHeader(CONTENT_LENGTH_HEADER); } + + private boolean isCrossRegionAccessEnabled(ExecutionAttributes executionAttributes) { + Optional ctxParams = executionAttributes.getOptionalAttribute( + SdkInternalExecutionAttribute.CLIENT_CONTEXT_PARAMS); + + return ctxParams.map(p -> Boolean.TRUE.equals(p.get(S3ClientContextParams.CROSS_REGION_ACCESS_ENABLED))) + .orElse(false); + } } diff --git a/services/s3/src/main/resources/codegen-resources/customization.config b/services/s3/src/main/resources/codegen-resources/customization.config index 4dee2d9e88d9..53090b4e7803 100644 --- a/services/s3/src/main/resources/codegen-resources/customization.config +++ b/services/s3/src/main/resources/codegen-resources/customization.config @@ -316,7 +316,7 @@ "enableEndpointAuthSchemeParams": true, "customClientContextParams":{ "CrossRegionAccessEnabled":{ - "documentation":"Enables cross-region bucket access for this client", + "documentation":"Enables cross-region bucket access for this client. Note: When enabling this feature, the S3 client will always set the 'Expect: 100-continue' header; the S3Configuration.expectContinueEnabled and S3Configuration.expectContinueThresholdInBytes configurations have no effect.", "type":"boolean" } }, diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/Expect100ContinueTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/Expect100ContinueTest.java new file mode 100644 index 000000000000..20e11e55098c --- /dev/null +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crossregion/Expect100ContinueTest.java @@ -0,0 +1,116 @@ +/* + * Copyright 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.crossregion; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.concurrent.CompletableFuture; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.mockito.ArgumentCaptor; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.core.async.AsyncRequestBody; +import software.amazon.awssdk.core.sync.RequestBody; +import software.amazon.awssdk.http.HttpExecuteRequest; +import software.amazon.awssdk.http.SdkHttpClient; +import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.http.async.AsyncExecuteRequest; +import software.amazon.awssdk.http.async.SdkAsyncHttpClient; +import software.amazon.awssdk.http.async.SdkAsyncHttpResponseHandler; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3AsyncClient; +import software.amazon.awssdk.services.s3.S3Client; + +public class Expect100ContinueTest { + private static final AwsCredentialsProvider TEST_CREDS = StaticCredentialsProvider.create( + AwsBasicCredentials.create("akid", "skid")); + private SdkHttpClient mockSyncHttp; + private SdkAsyncHttpClient mockAsyncHttp; + + @BeforeEach + void setup() { + mockSyncHttp = mock(SdkHttpClient.class); + when(mockSyncHttp.prepareRequest(any(HttpExecuteRequest.class))).thenThrow(new RuntimeException("expect 100 continue")); + + mockAsyncHttp = mock(SdkAsyncHttpClient.class); + CompletableFuture cf = new CompletableFuture(); + cf.completeExceptionally(new RuntimeException("expect 100 continue")); + when(mockAsyncHttp.execute(any(AsyncExecuteRequest.class))).thenAnswer(i -> { + AsyncExecuteRequest req = i.getArgument(0); + SdkAsyncHttpResponseHandler handler = req.responseHandler(); + handler.onError(new RuntimeException("expect 100 continue")); + return CompletableFuture.completedFuture(null); + }); + } + + @ParameterizedTest(name = "expect 100-continue enabled = {0}") + @CsvSource({"true", "false"}) + void sync_alwaysAdds(boolean enabled) { + + try (S3Client s3 = S3Client.builder() + .httpClient(mockSyncHttp) + .region(Region.US_WEST_2) + .credentialsProvider(TEST_CREDS) + .crossRegionAccessEnabled(true) + .serviceConfiguration(o -> o.expectContinueEnabled(enabled) + .expectContinueThresholdInBytes(1L)) + .build()) { + RequestBody requestBody = RequestBody.fromBytes(new byte[16]); + assertThatThrownBy(() -> s3.putObject(o -> o.bucket("bucket").key("key"), requestBody)) + .hasMessage("expect 100 continue"); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(HttpExecuteRequest.class); + + verify(mockSyncHttp).prepareRequest(requestCaptor.capture()); + assertHasExpect100Continue(requestCaptor.getValue().httpRequest()); + } + } + + @ParameterizedTest(name = "expect 100-continue enabled = {0}") + @CsvSource({"true", "false"}) + void async_alwaysAdds(boolean enabled) { + try (S3AsyncClient s3 = S3AsyncClient.builder() + .httpClient(mockAsyncHttp) + .region(Region.US_WEST_2) + .credentialsProvider(TEST_CREDS) + .crossRegionAccessEnabled(true) + .serviceConfiguration(o -> o.expectContinueEnabled(enabled) + .expectContinueThresholdInBytes(1L)) + .build()) { + AsyncRequestBody requestBody = AsyncRequestBody.fromBytes(new byte[16]); + assertThatThrownBy(s3.putObject(o -> o.bucket("bucket").key("key"), requestBody)::join) + .hasMessageContaining("expect 100 continue"); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(AsyncExecuteRequest.class); + + verify(mockAsyncHttp).execute(requestCaptor.capture()); + assertHasExpect100Continue(requestCaptor.getValue().request()); + } + } + + private static void assertHasExpect100Continue(SdkHttpRequest httpRequest) { + assertThat(httpRequest.firstMatchingHeader("Expect")) + .hasValueSatisfying(v -> assertThat(v).isEqualToIgnoringCase("100-continue")); + } +}