diff --git a/.changes/next-release/bugfix-AmazonS3-expectcontinue.json b/.changes/next-release/bugfix-AmazonS3-expectcontinue.json new file mode 100644 index 000000000000..c0b57ac9ad6b --- /dev/null +++ b/.changes/next-release/bugfix-AmazonS3-expectcontinue.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Amazon S3", + "contributor": "", + "description": "Honor an explicit `S3Configuration.expectContinueEnabled(false)` when cross region access is enabled." +} 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 697601df9ab9..bc3006899f8d 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,8 +248,8 @@ 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. + * When cross region access is enabled, the header is sent by default so S3 can issue its cross region redirect before + * the body is streamed. Setting this to {@code false} is still honored, but opts out of that protection. *

* 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 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 cce812afa646..f78e9fdd23c0 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 @@ -29,6 +29,7 @@ import software.amazon.awssdk.services.s3.model.PutObjectRequest; import software.amazon.awssdk.services.s3.model.UploadPartRequest; import software.amazon.awssdk.utils.AttributeMap; +import software.amazon.awssdk.utils.Logger; /** * Interceptor to add an 'Expect: 100-continue' header to the HTTP Request if it represents a PUT Object or Upload Part @@ -38,6 +39,8 @@ //TODO: This should be generalized for all streaming requests public final class StreamingRequestInterceptor implements ExecutionInterceptor { + private static final Logger log = Logger.loggerFor(StreamingRequestInterceptor.class); + private static final String DECODED_CONTENT_LENGTH_HEADER = "x-amz-decoded-content-length"; private static final String CONTENT_LENGTH_HEADER = "Content-Length"; @@ -58,16 +61,24 @@ 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)) { + S3Configuration s3Config = getS3Configuration(executionAttributes); + boolean expectContinueEnabled = s3Config == null || s3Config.expectContinueEnabled(); + boolean crossRegionAccessEnabled = isCrossRegionAccessEnabled(executionAttributes); + + // For cross region PUT, the header defaults to on: sending the body unconditionally to the wrong region where S3 + // responds with a 3xx and closes the connection can surface as an I/O error before the client can retry based on the + // region in the 3xx response. Users who explicitly disable expectContinueEnabled opt out of this. + if (crossRegionAccessEnabled && expectContinueEnabled) { return true; } - S3Configuration s3Config = getS3Configuration(executionAttributes); - - if (s3Config != null && !s3Config.expectContinueEnabled()) { + if (!expectContinueEnabled) { + if (crossRegionAccessEnabled) { + log.debug(() -> "Expect: 100-continue is explicitly disabled while cross region access is enabled. The " + + "header will not be added, so the first call to a bucket whose region " + + "has not yet been resolved may fail with an I/O error instead of being transparently " + + "redirected."); + } return false; } diff --git a/services/s3/src/main/resources/codegen-resources/customization.config b/services/s3/src/main/resources/codegen-resources/customization.config index 83abac701f09..b02db0b1c09a 100644 --- a/services/s3/src/main/resources/codegen-resources/customization.config +++ b/services/s3/src/main/resources/codegen-resources/customization.config @@ -317,7 +317,7 @@ "enableEndpointAuthSchemeParams": true, "customClientContextParams":{ "CrossRegionAccessEnabled":{ - "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.", + "documentation":"Enables cross-region bucket access for this client. Note: When enabling this feature, the S3 client sets the 'Expect: 100-continue' header by default so that S3 can issue its cross-region redirect before the request body is sent. This can be overridden by explicitly setting S3Configuration.expectContinueEnabled to false, in which case the first request to a not yet resolved bucket may fail with an I/O error and rely on normal retries.", "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 index 20e11e55098c..e312cdf140ff 100644 --- 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 @@ -66,7 +66,7 @@ void setup() { @ParameterizedTest(name = "expect 100-continue enabled = {0}") @CsvSource({"true", "false"}) - void sync_alwaysAdds(boolean enabled) { + void sync_crossRegion_honorsExpectContinueEnabled(boolean enabled) { try (S3Client s3 = S3Client.builder() .httpClient(mockSyncHttp) @@ -83,13 +83,13 @@ void sync_alwaysAdds(boolean enabled) { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(HttpExecuteRequest.class); verify(mockSyncHttp).prepareRequest(requestCaptor.capture()); - assertHasExpect100Continue(requestCaptor.getValue().httpRequest()); + assertExpect100Continue(requestCaptor.getValue().httpRequest(), enabled); } } @ParameterizedTest(name = "expect 100-continue enabled = {0}") @CsvSource({"true", "false"}) - void async_alwaysAdds(boolean enabled) { + void async_crossRegion_honorsExpectContinueEnabled(boolean enabled) { try (S3AsyncClient s3 = S3AsyncClient.builder() .httpClient(mockAsyncHttp) .region(Region.US_WEST_2) @@ -105,12 +105,16 @@ void async_alwaysAdds(boolean enabled) { ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(AsyncExecuteRequest.class); verify(mockAsyncHttp).execute(requestCaptor.capture()); - assertHasExpect100Continue(requestCaptor.getValue().request()); + assertExpect100Continue(requestCaptor.getValue().request(), enabled); } } - private static void assertHasExpect100Continue(SdkHttpRequest httpRequest) { - assertThat(httpRequest.firstMatchingHeader("Expect")) - .hasValueSatisfying(v -> assertThat(v).isEqualToIgnoringCase("100-continue")); + private static void assertExpect100Continue(SdkHttpRequest httpRequest, boolean expectHeaderPresent) { + if (expectHeaderPresent) { + assertThat(httpRequest.firstMatchingHeader("Expect")) + .hasValueSatisfying(v -> assertThat(v).isEqualToIgnoringCase("100-continue")); + } else { + assertThat(httpRequest.firstMatchingHeader("Expect")).isEmpty(); + } } }