-
Notifications
You must be signed in to change notification settings - Fork 951
Skip 'Expect: 100-continue' header for zero-length S3 streaming requests #6465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
32853df
7ce501e
04e9ec5
14e35ba
795d9d6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "type": "bugfix", | ||
| "category": "Amazon S3", | ||
| "contributor": "", | ||
| "description": "Fix StreamingRequestInterceptor to skip Expect: 100-continue header when PutObject or UploadPart requests have zero content length." | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| import software.amazon.awssdk.core.interceptor.Context; | ||
| import software.amazon.awssdk.core.interceptor.ExecutionAttributes; | ||
| import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; | ||
| import software.amazon.awssdk.core.sync.RequestBody; | ||
| import software.amazon.awssdk.http.SdkHttpRequest; | ||
| import software.amazon.awssdk.services.s3.model.PutObjectRequest; | ||
| import software.amazon.awssdk.services.s3.model.UploadPartRequest; | ||
|
|
@@ -32,9 +33,38 @@ public final class StreamingRequestInterceptor implements ExecutionInterceptor { | |
| @Override | ||
| public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, | ||
| ExecutionAttributes executionAttributes) { | ||
| if (context.request() instanceof PutObjectRequest || context.request() instanceof UploadPartRequest) { | ||
| if (shouldAddExpectContinueHeader(context)) { | ||
| return context.httpRequest().toBuilder().putHeader("Expect", "100-continue").build(); | ||
| } | ||
| return context.httpRequest(); | ||
| } | ||
|
|
||
| /** | ||
| * Determines whether to add 'Expect: 100-continue' header to streaming requests. | ||
| * | ||
| * Per RFC 9110 Section 10.1.1, clients MUST NOT send 100-continue for requests without content. | ||
| * | ||
| * Note: Empty Content length check currently applies to sync clients only. Sync HTTP clients (e.g., Apache HttpClient) may | ||
| * reuse connections, and sending empty content with Expect header can cause issues if the server has already closed the | ||
| * connection. | ||
| * | ||
| * @param context the HTTP request modification context | ||
| * @return true if Expect header should be added, false otherwise | ||
| */ | ||
| private boolean shouldAddExpectContinueHeader(Context.ModifyHttpRequest context) { | ||
| // Must be a streaming request type | ||
| if (context.request() instanceof PutObjectRequest | ||
| || context.request() instanceof UploadPartRequest) { | ||
| // Zero Content length check | ||
| return context.requestBody() | ||
| .flatMap(RequestBody::optionalContentLength) | ||
L-Applin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| .map(length -> length != 0L) | ||
| .orElse(true); | ||
|
||
| } | ||
| return false; | ||
| } | ||
|
|
||
|
|
||
|
|
||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
StreamingRequestInterceptoris an internal class, can we remove the mention of it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done