Skip to content

feat(s3): add disableExpect100ContinueForPuts to S3Configuration#6774

Open
joviegas wants to merge 1 commit intomasterfrom
joviegas/expect-100-continue
Open

feat(s3): add disableExpect100ContinueForPuts to S3Configuration#6774
joviegas wants to merge 1 commit intomasterfrom
joviegas/expect-100-continue

Conversation

@joviegas
Copy link
Contributor

Motivation and Context

Reported in #6459 (comment) by the Apache Hadoop S3A team — they configure ApacheHttpClient.builder().expectContinueEnabled(false) to disable Expect: 100-continue for their use case, but the SDK's StreamingRequestInterceptor re-adds the header anyway, making their setting ineffective. They requested a way to turn it off from the SDK side for rare emergencies where the header causes issues with proxies or S3-compatible stores.

We considered adding this control at the SdkHttpClient level, but since this behavior is specific to S3 (only S3's StreamingRequestInterceptor adds the header), exposing it on SdkHttpClient.Builder would mean adding public API surface across all HTTP clients and piping values from the HTTP layer up to S3 — when only Apache even has its own independent expectContinueEnabled setting. Putting it on S3Configuration keeps the control scoped to where the behavior lives.

Modifications

  • Added disableExpect100ContinueForPuts(Boolean) to S3Configuration (defaults to false, preserving existing behavior).
  • Updated StreamingRequestInterceptor to read the config from SdkExecutionAttribute.SERVICE_CONFIG and skip adding the Expect: 100-continue header when disabled.
  • Documented the Apache HTTP client nuance in javadoc — Apache independently adds Expect: 100-continue via its own expectContinueEnabled setting (which defaults to true), so disabling on S3Configuration alone won't suppress the header on the wire when using Apache.

Example — fully disabling Expect: 100-continue with Apache:

S3Client s3 = S3Client.builder()
    .httpClientBuilder(ApacheHttpClient.builder().expectContinueEnabled(false))
    .serviceConfiguration(S3Configuration.builder()
        .disableExpect100ContinueForPuts(true)
        .build())
    .build();

For all other HTTP clients (URL Connection, Netty, CRT), setting disableExpect100ContinueForPuts(true) on S3Configuration is sufficient.

Testing

  • Unit tests (StreamingRequestInterceptorTest): Parameterized tests covering disableExpect100ContinueForPuts with true, false, null, no service config, non-streaming requests (GetObject), and non-zero content-length — 7 scenarios via @MethodSource. Also covers RFC 9110 compliance (zero vs non-zero content length for PutObject/UploadPart).
  • Functional tests (Expect100ContinueHeaderTest): WireMock-based end-to-end tests across 5 HTTP client types (Apache, Apache with expectContinueEnabled=false, Apache with expectContinueEnabled=true, URL Connection, CRT sync, CRT async, Netty) × 14 config combinations × 2 operations (PutObject + UploadPart). Verifies the actual Expect header presence/absence on the wire.
  • Manually verified CRT wire behavior using Log.LogLevel.Trace — confirmed the generic AwsCrtHttpClient passes the Expect header through (does not strip it).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

License

  • I confirm that this pull request can be released under the Apache 2 license

@joviegas joviegas requested a review from a team as a code owner March 10, 2026 23:12
@joviegas joviegas force-pushed the joviegas/expect-100-continue branch from 61e752a to 2f98f89 Compare March 11, 2026 16:34
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant