Respect explicit opt out for Expect: 100-continue even on cross region calls

.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."
+}

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.
      * <p>
-     * 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.
      * <p>
      * <b>Note:</b> 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

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;
         }
 

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"
     }
   },

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<HttpExecuteRequest> 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<AsyncExecuteRequest> 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();
+        }
     }
 }