Skip to content

Commit 04e9ec5

Browse files
joviegasjoviegas
committed
Handled Review commments and added tests
1 parent 7ce501e commit 04e9ec5

File tree

2 files changed

+334
-1
lines changed

2 files changed

+334
-1
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,13 @@ private boolean shouldAddExpectContinueHeader(Context.ModifyHttpRequest context)
5555
.orElse(true);
5656
}
5757

58-
// Check x-amz-decoded-content-length first, fall back to Content-Length
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+
*/
5965
private Optional<String> getContentLengthHeader(SdkHttpRequest httpRequest) {
6066
Optional<String> decodedLength = httpRequest.firstMatchingHeader(DECODED_CONTENT_LENGTH_HEADER);
6167
return decodedLength.isPresent()

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

Lines changed: 327 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,327 @@
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+
public 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+
}
220+
});
221+
};
222+
223+
AsyncRequestBody emptyBody = AsyncRequestBody.fromPublisher(emptyPublisher);
224+
executeRequest.apply(emptyBody).join();
225+
assertExpectHeaderPresent();
226+
}
227+
228+
@ParameterizedTest(name = "{0}_withPublisherNonEmptyBody_sendsExpectHeader")
229+
@MethodSource("asyncRequestProvider")
230+
void asyncOperation_withPublisherNonEmptyBody_sendsExpectHeader(String operationType,
231+
Function<AsyncRequestBody, CompletableFuture<?>> executeRequest) {
232+
stubAnyPutRequest();
233+
byte[] content = "test content".getBytes(StandardCharsets.UTF_8);
234+
Publisher<ByteBuffer> contentPublisher = subscriber -> {
235+
subscriber.onSubscribe(new Subscription() {
236+
private boolean sent = false;
237+
238+
@Override
239+
public void request(long n) {
240+
if (!sent && n > 0) {
241+
sent = true;
242+
subscriber.onNext(ByteBuffer.wrap(content));
243+
subscriber.onComplete();
244+
}
245+
}
246+
247+
@Override
248+
public void cancel() {
249+
}
250+
});
251+
};
252+
253+
AsyncRequestBody publisherBody = AsyncRequestBody.fromPublisher(contentPublisher);
254+
executeRequest.apply(publisherBody).join();
255+
assertExpectHeaderPresent();
256+
}
257+
258+
private Stream<Arguments> syncRequestProvider() {
259+
return Stream.of(
260+
Arguments.of("PutObject", (Function<RequestBody, Void>) body -> {
261+
syncClient.putObject(basePutObjectRequest().build(), body);
262+
return null;
263+
}),
264+
Arguments.of("UploadPart", (Function<RequestBody, Void>) body -> {
265+
syncClient.uploadPart(baseUploadPartRequest().build(), body);
266+
return null;
267+
})
268+
);
269+
}
270+
271+
private Stream<Arguments> asyncRequestProvider() {
272+
return Stream.of(
273+
Arguments.of("PutObject", (Function<AsyncRequestBody, CompletableFuture<?>>) body ->
274+
asyncClient.putObject(basePutObjectRequest().build(), body)),
275+
Arguments.of("UploadPart", (Function<AsyncRequestBody, CompletableFuture<?>>) body ->
276+
asyncClient.uploadPart(baseUploadPartRequest().build(), body))
277+
);
278+
}
279+
280+
private PutObjectRequest.Builder basePutObjectRequest() {
281+
return PutObjectRequest.builder()
282+
.bucket(BUCKET)
283+
.key(KEY);
284+
}
285+
286+
private UploadPartRequest.Builder baseUploadPartRequest() {
287+
return UploadPartRequest.builder()
288+
.bucket(BUCKET)
289+
.key(KEY)
290+
.uploadId(UPLOAD_ID)
291+
.partNumber(1);
292+
}
293+
294+
private void stubAnyPutRequest() {
295+
stubFor(put(anyUrl())
296+
.willReturn(aResponse()
297+
.withStatus(200)
298+
.withHeader("ETag", "\"test-etag\"")));
299+
}
300+
301+
private void assertExpectHeaderNotPresent() {
302+
LoggedRequest request = getSingleCapturedRequest();
303+
304+
assertThat(request.header("Expect") == null || !request.header("Expect").isPresent())
305+
.as("Expect header should not be present for empty body per RFC 9110 Section 10.1.1")
306+
.isTrue();
307+
}
308+
309+
private void assertExpectHeaderPresent() {
310+
LoggedRequest request = getSingleCapturedRequest();
311+
312+
assertThat(request.header("Expect"))
313+
.as("Expect: 100-continue header should be present for non-empty body")
314+
.isNotNull()
315+
.satisfies(header -> assertThat(header.firstValue()).isEqualTo("100-continue"));
316+
}
317+
318+
private LoggedRequest getSingleCapturedRequest() {
319+
List<LoggedRequest> requests = findAll(putRequestedFor(anyUrl()));
320+
321+
assertThat(requests)
322+
.as("Expected exactly one HTTP request to be captured")
323+
.hasSize(1);
324+
325+
return requests.get(0);
326+
}
327+
}

0 commit comments

Comments
 (0)