Skip to content

feat(s3): add expectContinueEnabled to S3Configuration#6774

Merged
joviegas merged 5 commits intomasterfrom
joviegas/expect-100-continue
Mar 12, 2026
Merged

feat(s3): add expectContinueEnabled to S3Configuration#6774
joviegas merged 5 commits intomasterfrom
joviegas/expect-100-continue

Conversation

@joviegas
Copy link
Copy Markdown
Contributor

@joviegas joviegas commented Mar 10, 2026

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 expectContinueEnabled(Boolean) to S3Configuration (defaults to true, preserving existing behavior).
  • Updated StreamingRequestInterceptor to read the config from SdkExecutionAttribute.SERVICE_CONFIG and skip adding the Expect: 100-continue header when expectContinueEnabled is set to false.
  • Documented the Apache HTTP client nuance in javadoc — Apache independently adds Expect: 100-continue via its own expectContinueEnabled setting (which defaults to true), so setting expectContinueEnabled(false) 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()
        .expectContinueEnabled(false)
        .build())
    .build();

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

Testing

  • Unit tests (StreamingRequestInterceptorTest): Parameterized tests covering expectContinueEnabled with true, false, null, no service config, non-streaming requests (GetObject), zero and non-zero content-length — 8 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
@joviegas joviegas added the api-surface-area-approved-by-team Indicate API surface area introduced by this PR has been approved by team label Mar 12, 2026
@joviegas joviegas changed the title feat(s3): add disableExpect100ContinueForPuts to S3Configuration feat(s3): add expectContinueEnabled to S3Configuration Mar 12, 2026
@joviegas joviegas enabled auto-merge March 12, 2026 21:18
@sonarqubecloud
Copy link
Copy Markdown

@joviegas joviegas added this pull request to the merge queue Mar 12, 2026
Merged via the queue into master with commit 536eb46 Mar 12, 2026
39 of 40 checks passed
@github-actions
Copy link
Copy Markdown

This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api-surface-area-approved-by-team Indicate API surface area introduced by this PR has been approved by team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants