Skip to content

Commit 5f885cb

Browse files
joviegasjoviegas
authored
Skip 'Expect: 100-continue' header for zero-length S3 streaming requests (#6465)
* Donot send Expect 100-continue when RequestBody is of Zero content length * Handled Review commments * Handled Review commments and added tests * Handled Review commments from Zoe
1 parent 7a71d5f commit 5f885cb

File tree

5 files changed

+466
-3
lines changed

5 files changed

+466
-3
lines changed

‎.changes/next-release/bugfix-AmazonS3-bb097c9.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "Amazon S3",
4+
"contributor": "",
5+
"description": "Skip Expect: 100-continue header for PutObject and UploadPart requests with zero content length"
6+
}

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/handlers/StreamingRequestInterceptor.java‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.awssdk.services.s3.internal.handlers;
1717

18+
import java.util.Optional;
1819
import software.amazon.awssdk.annotations.SdkInternalApi;
1920
import software.amazon.awssdk.core.interceptor.Context;
2021
import software.amazon.awssdk.core.interceptor.ExecutionAttributes;
@@ -29,12 +30,42 @@
2930
@SdkInternalApi
3031
//TODO: This should be generalized for all streaming requests
3132
public final class StreamingRequestInterceptor implements ExecutionInterceptor {
33+
34+
private static final String DECODED_CONTENT_LENGTH_HEADER = "x-amz-decoded-content-length";
35+
private static final String CONTENT_LENGTH_HEADER = "Content-Length";
36+
3237
@Override
3338
public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context,
3439
ExecutionAttributes executionAttributes) {
35-
if (context.request() instanceof PutObjectRequest || context.request() instanceof UploadPartRequest) {
40+
if (shouldAddExpectContinueHeader(context)) {
3641
return context.httpRequest().toBuilder().putHeader("Expect", "100-continue").build();
3742
}
3843
return context.httpRequest();
3944
}
45+
46+
private boolean shouldAddExpectContinueHeader(Context.ModifyHttpRequest context) {
47+
// Only applies to streaming operations
48+
if (!(context.request() instanceof PutObjectRequest
49+
|| context.request() instanceof UploadPartRequest)) {
50+
return false;
51+
}
52+
return getContentLengthHeader(context.httpRequest())
53+
.map(Long::parseLong)
54+
.map(length -> length != 0L)
55+
.orElse(true);
56+
}
57+
58+
/**
59+
* Retrieves content length header value.
60+
* Checks x-amz-decoded-content-length first, then falls back to Content-Length.
61+
*
62+
* @param httpRequest the HTTP request
63+
* @return Optional containing the content length header value, or empty if not present
64+
*/
65+
private Optional<String> getContentLengthHeader(SdkHttpRequest httpRequest) {
66+
Optional<String> decodedLength = httpRequest.firstMatchingHeader(DECODED_CONTENT_LENGTH_HEADER);
67+
return decodedLength.isPresent()
68+
? decodedLength
69+
: httpRequest.firstMatchingHeader(CONTENT_LENGTH_HEADER);
70+
}
4071
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/functionaltests/Expect100ContinueHeaderTest.java‎

Lines changed: 329 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,329 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
package software.amazon.awssdk.services.s3.functionaltests;
16+
17+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
18+
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
19+
import static com.github.tomakehurst.wiremock.client.WireMock.findAll;
20+
import static com.github.tomakehurst.wiremock.client.WireMock.put;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
23+
import static org.assertj.core.api.Assertions.assertThat;
24+
25+
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
26+
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
27+
import com.github.tomakehurst.wiremock.verification.LoggedRequest;
28+
import java.io.ByteArrayInputStream;
29+
import java.net.URI;
30+
import java.nio.ByteBuffer;
31+
import java.nio.charset.StandardCharsets;
32+
import java.util.List;
33+
import java.util.concurrent.CompletableFuture;
34+
import java.util.function.Function;
35+
import java.util.stream.Stream;
36+
import org.junit.jupiter.api.AfterEach;
37+
import org.junit.jupiter.api.BeforeEach;
38+
import org.junit.jupiter.api.TestInstance;
39+
import org.junit.jupiter.params.ParameterizedTest;
40+
import org.junit.jupiter.params.provider.Arguments;
41+
import org.junit.jupiter.params.provider.MethodSource;
42+
import org.reactivestreams.Publisher;
43+
import org.reactivestreams.Subscription;
44+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
45+
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
46+
import software.amazon.awssdk.core.async.AsyncRequestBody;
47+
import software.amazon.awssdk.core.checksums.RequestChecksumCalculation;
48+
import software.amazon.awssdk.core.checksums.ResponseChecksumValidation;
49+
import software.amazon.awssdk.core.sync.RequestBody;
50+
import software.amazon.awssdk.http.ContentStreamProvider;
51+
import software.amazon.awssdk.http.SdkHttpClient;
52+
import software.amazon.awssdk.http.SdkHttpConfigurationOption;
53+
import software.amazon.awssdk.http.apache.ApacheHttpClient;
54+
import software.amazon.awssdk.http.async.SdkAsyncHttpClient;
55+
import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient;
56+
import software.amazon.awssdk.regions.Region;
57+
import software.amazon.awssdk.services.s3.S3AsyncClient;
58+
import software.amazon.awssdk.services.s3.S3Client;
59+
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
60+
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
61+
import software.amazon.awssdk.utils.AttributeMap;
62+
63+
/**
64+
* Functional tests to verify RFC 9110 compliance for Expect: 100-continue header behavior.
65+
* <p>
66+
* Per RFC 9110 Section 10.1.1, clients MUST NOT send 100-continue for requests without content.
67+
* These tests verify the header is correctly omitted for zero-length bodies and included for
68+
* non-empty bodies in both sync and async S3 operations.
69+
*/
70+
@WireMockTest(httpsEnabled = true)
71+
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
72+
class Expect100ContinueHeaderTest {
73+
74+
private static final String BUCKET = "test-bucket";
75+
private static final String KEY = "test-key";
76+
private static final String UPLOAD_ID = "test-upload-id";
77+
78+
private S3Client syncClient;
79+
private S3AsyncClient asyncClient;
80+
81+
@BeforeEach
82+
void setup(WireMockRuntimeInfo wmRuntimeInfo) {
83+
URI endpointOverride = URI.create(wmRuntimeInfo.getHttpsBaseUrl());
84+
StaticCredentialsProvider credentialsProvider = StaticCredentialsProvider.create(
85+
AwsBasicCredentials.create("akid", "skid"));
86+
87+
SdkHttpClient apacheHttpClient = ApacheHttpClient.builder()
88+
.buildWithDefaults(AttributeMap.builder()
89+
.put(SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES, Boolean.TRUE)
90+
.build());
91+
92+
syncClient = S3Client.builder()
93+
.httpClient(apacheHttpClient)
94+
.region(Region.US_EAST_1)
95+
.endpointOverride(endpointOverride)
96+
.requestChecksumCalculation(RequestChecksumCalculation.WHEN_REQUIRED)
97+
.responseChecksumValidation(ResponseChecksumValidation.WHEN_REQUIRED)
98+
.forcePathStyle(true)
99+
.credentialsProvider(credentialsProvider)
100+
.build();
101+
102+
SdkAsyncHttpClient nettyHttpClient = NettyNioAsyncHttpClient.builder()
103+
.buildWithDefaults(AttributeMap.builder()
104+
.put(SdkHttpConfigurationOption.TRUST_ALL_CERTIFICATES, Boolean.TRUE)
105+
.build());
106+
107+
asyncClient = S3AsyncClient.builder()
108+
.httpClient(nettyHttpClient)
109+
.region(Region.US_EAST_1)
110+
.endpointOverride(endpointOverride)
111+
.requestChecksumCalculation(RequestChecksumCalculation.WHEN_REQUIRED)
112+
.responseChecksumValidation(ResponseChecksumValidation.WHEN_REQUIRED)
113+
.forcePathStyle(true)
114+
.credentialsProvider(credentialsProvider)
115+
.build();
116+
}
117+
118+
@AfterEach
119+
void teardown() {
120+
if (syncClient != null) {
121+
syncClient.close();
122+
}
123+
if (asyncClient != null) {
124+
asyncClient.close();
125+
}
126+
}
127+
128+
@ParameterizedTest(name = "{0}_withEmptyBody_doesNotSendExpectHeader")
129+
@MethodSource("syncRequestProvider")
130+
void syncOperation_withEmptyBody_doesNotSendExpectHeader(String operationType,
131+
Function<RequestBody, Void> executeRequest) {
132+
stubAnyPutRequest();
133+
executeRequest.apply(RequestBody.empty());
134+
assertExpectHeaderNotPresent();
135+
}
136+
137+
@ParameterizedTest(name = "{0}_withEmptyStringBody_doesNotSendExpectHeader")
138+
@MethodSource("syncRequestProvider")
139+
void syncOperation_withEmptyStringBody_doesNotSendExpectHeader(String operationType,
140+
Function<RequestBody, Void> executeRequest) {
141+
stubAnyPutRequest();
142+
executeRequest.apply(RequestBody.fromString(""));
143+
assertExpectHeaderNotPresent();
144+
}
145+
146+
@ParameterizedTest(name = "{0}_withNonEmptyBody_sendsExpectHeader")
147+
@MethodSource("syncRequestProvider")
148+
void syncOperation_withNonEmptyBody_sendsExpectHeader(String operationType,
149+
Function<RequestBody, Void> executeRequest) {
150+
stubAnyPutRequest();
151+
executeRequest.apply(RequestBody.fromString("test content"));
152+
assertExpectHeaderPresent();
153+
}
154+
155+
@ParameterizedTest(name = "{0}_withContentProviderEmptyBody_sendsExpectHeader")
156+
@MethodSource("syncRequestProvider")
157+
void syncOperation_withContentProviderEmptyBody_sendsExpectHeader(String operationType,
158+
Function<RequestBody, Void> executeRequest) {
159+
stubAnyPutRequest();
160+
ContentStreamProvider emptyProvider = () -> new ByteArrayInputStream(new byte[0]);
161+
RequestBody emptyBody = RequestBody.fromContentProvider(emptyProvider, "application/octet-stream");
162+
executeRequest.apply(emptyBody);
163+
assertExpectHeaderPresent();
164+
}
165+
166+
@ParameterizedTest(name = "{0}_withContentProviderNonEmptyBody_sendsExpectHeader")
167+
@MethodSource("syncRequestProvider")
168+
void syncOperation_withContentProviderNonEmptyBody_sendsExpectHeader(String operationType,
169+
Function<RequestBody, Void> executeRequest) {
170+
stubAnyPutRequest();
171+
byte[] content = "test content".getBytes(StandardCharsets.UTF_8);
172+
ContentStreamProvider contentProvider = () -> new ByteArrayInputStream(content);
173+
RequestBody providerBody = RequestBody.fromContentProvider(contentProvider, "application/octet-stream");
174+
executeRequest.apply(providerBody);
175+
assertExpectHeaderPresent();
176+
}
177+
178+
@ParameterizedTest(name = "{0}_withEmptyBody_doesNotSendExpectHeader")
179+
@MethodSource("asyncRequestProvider")
180+
void asyncOperation_withEmptyBody_doesNotSendExpectHeader(String operationType,
181+
Function<AsyncRequestBody, CompletableFuture<?>> executeRequest) {
182+
stubAnyPutRequest();
183+
executeRequest.apply(AsyncRequestBody.empty()).join();
184+
assertExpectHeaderNotPresent();
185+
}
186+
187+
@ParameterizedTest(name = "{0}_withEmptyStringBody_doesNotSendExpectHeader")
188+
@MethodSource("asyncRequestProvider")
189+
void asyncOperation_withEmptyStringBody_doesNotSendExpectHeader(String operationType,
190+
Function<AsyncRequestBody, CompletableFuture<?>> executeRequest) {
191+
stubAnyPutRequest();
192+
executeRequest.apply(AsyncRequestBody.fromString("")).join();
193+
assertExpectHeaderNotPresent();
194+
}
195+
196+
@ParameterizedTest(name = "{0}_withNonEmptyBody_sendsExpectHeader")
197+
@MethodSource("asyncRequestProvider")
198+
void asyncOperation_withNonEmptyBody_sendsExpectHeader(String operationType,
199+
Function<AsyncRequestBody, CompletableFuture<?>> executeRequest) {
200+
stubAnyPutRequest();
201+
executeRequest.apply(AsyncRequestBody.fromString("test content")).join();
202+
assertExpectHeaderPresent();
203+
}
204+
205+
@ParameterizedTest(name = "{0}_withPublisherEmptyBody_sendsExpectHeader")
206+
@MethodSource("asyncRequestProvider")
207+
void asyncOperation_withPublisherEmptyBody_sendsExpectHeader(String operationType,
208+
Function<AsyncRequestBody, CompletableFuture<?>> executeRequest) {
209+
stubAnyPutRequest();
210+
Publisher<ByteBuffer> emptyPublisher = subscriber -> {
211+
subscriber.onSubscribe(new Subscription() {
212+
@Override
213+
public void request(long n) {
214+
subscriber.onComplete();
215+
}
216+
217+
@Override
218+
public void cancel() {
219+
// No action.
220+
}
221+
});
222+
};
223+
224+
AsyncRequestBody emptyBody = AsyncRequestBody.fromPublisher(emptyPublisher);
225+
executeRequest.apply(emptyBody).join();
226+
assertExpectHeaderPresent();
227+
}
228+
229+
@ParameterizedTest(name = "{0}_withPublisherNonEmptyBody_sendsExpectHeader")
230+
@MethodSource("asyncRequestProvider")
231+
void asyncOperation_withPublisherNonEmptyBody_sendsExpectHeader(String operationType,
232+
Function<AsyncRequestBody, CompletableFuture<?>> executeRequest) {
233+
stubAnyPutRequest();
234+
byte[] content = "test content".getBytes(StandardCharsets.UTF_8);
235+
Publisher<ByteBuffer> contentPublisher = subscriber -> {
236+
subscriber.onSubscribe(new Subscription() {
237+
private boolean sent = false;
238+
239+
@Override
240+
public void request(long n) {
241+
if (!sent && n > 0) {
242+
sent = true;
243+
subscriber.onNext(ByteBuffer.wrap(content));
244+
subscriber.onComplete();
245+
}
246+
}
247+
248+
@Override
249+
public void cancel() {
250+
// No action.
251+
}
252+
});
253+
};
254+
255+
AsyncRequestBody publisherBody = AsyncRequestBody.fromPublisher(contentPublisher);
256+
executeRequest.apply(publisherBody).join();
257+
assertExpectHeaderPresent();
258+
}
259+
260+
private Stream<Arguments> syncRequestProvider() {
261+
return Stream.of(
262+
Arguments.of("PutObject", (Function<RequestBody, Void>) body -> {
263+
syncClient.putObject(basePutObjectRequest().build(), body);
264+
return null;
265+
}),
266+
Arguments.of("UploadPart", (Function<RequestBody, Void>) body -> {
267+
syncClient.uploadPart(baseUploadPartRequest().build(), body);
268+
return null;
269+
})
270+
);
271+
}
272+
273+
private Stream<Arguments> asyncRequestProvider() {
274+
return Stream.of(
275+
Arguments.of("PutObject", (Function<AsyncRequestBody, CompletableFuture<?>>) body ->
276+
asyncClient.putObject(basePutObjectRequest().build(), body)),
277+
Arguments.of("UploadPart", (Function<AsyncRequestBody, CompletableFuture<?>>) body ->
278+
asyncClient.uploadPart(baseUploadPartRequest().build(), body))
279+
);
280+
}
281+
282+
private PutObjectRequest.Builder basePutObjectRequest() {
283+
return PutObjectRequest.builder()
284+
.bucket(BUCKET)
285+
.key(KEY);
286+
}
287+
288+
private UploadPartRequest.Builder baseUploadPartRequest() {
289+
return UploadPartRequest.builder()
290+
.bucket(BUCKET)
291+
.key(KEY)
292+
.uploadId(UPLOAD_ID)
293+
.partNumber(1);
294+
}
295+
296+
private void stubAnyPutRequest() {
297+
stubFor(put(anyUrl())
298+
.willReturn(aResponse()
299+
.withStatus(200)
300+
.withHeader("ETag", "\"test-etag\"")));
301+
}
302+
303+
private void assertExpectHeaderNotPresent() {
304+
LoggedRequest request = getSingleCapturedRequest();
305+
306+
assertThat(request.header("Expect") == null || !request.header("Expect").isPresent())
307+
.as("Expect header should not be present for empty body per RFC 9110 Section 10.1.1")
308+
.isTrue();
309+
}
310+
311+
private void assertExpectHeaderPresent() {
312+
LoggedRequest request = getSingleCapturedRequest();
313+
314+
assertThat(request.header("Expect"))
315+
.as("Expect: 100-continue header should be present for non-empty body")
316+
.isNotNull()
317+
.satisfies(header -> assertThat(header.firstValue()).isEqualTo("100-continue"));
318+
}
319+
320+
private LoggedRequest getSingleCapturedRequest() {
321+
List<LoggedRequest> requests = findAll(putRequestedFor(anyUrl()));
322+
323+
assertThat(requests)
324+
.as("Expected exactly one HTTP request to be captured")
325+
.hasSize(1);
326+
327+
return requests.get(0);
328+
}
329+
}

0 commit comments

Comments
 (0)