Skip to content

Commit dc7c27c

Browse files
joviegasjoviegas
committed
Propagate content-type from AsyncRequestBody for multipart uploads (Issue #6607)
1 parent 11502ed commit dc7c27c

File tree

5 files changed

+326
-3
lines changed

5 files changed

+326
-3
lines changed

‎.changes/next-release/bugfix-AmazonS3-d039ba6.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": "Fixed multipart uploads not propagating content-type from AsyncRequestBody when using S3AsyncClient with multipartEnabled(true). See Issue [#6607](https://github.com/aws/aws-sdk-java-v2/issues/6607)"
6+
}

‎services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelper.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ public UploadObjectHelper(S3AsyncClient s3AsyncClient,
6060

6161
public CompletableFuture<PutObjectResponse> uploadObject(PutObjectRequest putObjectRequest,
6262
AsyncRequestBody asyncRequestBody) {
63+
64+
// Propagate content-type from AsyncRequestBody if not explicitly set on the request
65+
if (putObjectRequest.contentType() == null && asyncRequestBody.contentType() != null) {
66+
putObjectRequest = putObjectRequest.toBuilder()
67+
.contentType(asyncRequestBody.contentType())
68+
.build();
69+
}
6370
Long contentLength = asyncRequestBody.contentLength().orElseGet(putObjectRequest::contentLength);
6471

6572
if (contentLength == null) {

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/S3MultipartClientContentTypeTest.java‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
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+
16+
package software.amazon.awssdk.services.s3.internal.multipart;
17+
18+
import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
19+
import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl;
20+
import static com.github.tomakehurst.wiremock.client.WireMock.containing;
21+
import static com.github.tomakehurst.wiremock.client.WireMock.findAll;
22+
import static com.github.tomakehurst.wiremock.client.WireMock.post;
23+
import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
24+
import static com.github.tomakehurst.wiremock.client.WireMock.put;
25+
import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
26+
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
27+
import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo;
28+
import static org.assertj.core.api.Assertions.assertThat;
29+
30+
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
31+
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
32+
import com.github.tomakehurst.wiremock.verification.LoggedRequest;
33+
import java.io.File;
34+
import java.io.IOException;
35+
import java.net.URI;
36+
import java.nio.ByteBuffer;
37+
import java.nio.file.Files;
38+
import java.util.List;
39+
import java.util.Optional;
40+
import java.util.stream.Stream;
41+
import org.junit.jupiter.api.AfterEach;
42+
import org.junit.jupiter.api.BeforeEach;
43+
import org.junit.jupiter.api.Test;
44+
import org.junit.jupiter.api.TestInstance;
45+
import org.junit.jupiter.params.ParameterizedTest;
46+
import org.junit.jupiter.params.provider.Arguments;
47+
import org.junit.jupiter.params.provider.MethodSource;
48+
import org.reactivestreams.Subscriber;
49+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
50+
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
51+
import software.amazon.awssdk.core.async.AsyncRequestBody;
52+
import software.amazon.awssdk.regions.Region;
53+
import software.amazon.awssdk.services.s3.S3AsyncClient;
54+
55+
/**
56+
* Functional tests to verify content-type propagation from AsyncRequestBody for multipart uploads.
57+
* <p>
58+
* These tests verify that when using S3AsyncClient with multipartEnabled(true), the content-type
59+
* from AsyncRequestBody is correctly propagated to the CreateMultipartUpload request, ensuring
60+
* consistent behavior between single-part and multipart uploads.
61+
*/
62+
@WireMockTest
63+
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
64+
class S3MultipartClientContentTypeTest {
65+
66+
private static final String BUCKET = "test-bucket";
67+
private static final String KEY = "test-key";
68+
private static final long MULTIPART_THRESHOLD = 10 * 1024; // 10KB
69+
private static final long PART_SIZE = 5 * 1024; // 5KB
70+
71+
private static final String CREATE_MULTIPART_RESPONSE =
72+
"<InitiateMultipartUploadResult><Bucket>" + BUCKET + "</Bucket>"
73+
+ "<Key>" + KEY + "</Key><UploadId>upload-id</UploadId></InitiateMultipartUploadResult>";
74+
75+
private static final String COMPLETE_MULTIPART_RESPONSE =
76+
"<CompleteMultipartUploadResult><Bucket>" + BUCKET + "</Bucket>"
77+
+ "<Key>" + KEY + "</Key><ETag>\"etag\"</ETag></CompleteMultipartUploadResult>";
78+
79+
private S3AsyncClient s3Client;
80+
81+
private static File createTempFile(String prefix, String suffix, int size) throws IOException {
82+
File file = File.createTempFile(prefix, suffix);
83+
file.deleteOnExit();
84+
Files.write(file.toPath(), new byte[size]);
85+
return file;
86+
}
87+
88+
static Stream<Arguments> uploadScenarios() throws IOException {
89+
File singlePartHtmlFile = createTempFile("single", ".html", 5 * 1024);
90+
File multipartHtmlFile = createTempFile("multi", ".html", 15 * 1024);
91+
92+
return Stream.of(
93+
Arguments.of("singlePart_htmlFile", singlePartHtmlFile, false, "text/html"),
94+
Arguments.of("multipart_htmlFile", multipartHtmlFile, true, "text/html")
95+
);
96+
}
97+
98+
@BeforeEach
99+
void setup(WireMockRuntimeInfo wiremock) {
100+
s3Client = S3AsyncClient.builder()
101+
.region(Region.US_EAST_1)
102+
.endpointOverride(URI.create(wiremock.getHttpBaseUrl()))
103+
.credentialsProvider(StaticCredentialsProvider.create(
104+
AwsBasicCredentials.create("key", "secret")))
105+
.forcePathStyle(true)
106+
.multipartEnabled(true)
107+
.multipartConfiguration(c -> c.thresholdInBytes(MULTIPART_THRESHOLD)
108+
.minimumPartSizeInBytes(PART_SIZE))
109+
.build();
110+
}
111+
112+
@AfterEach
113+
void teardown() {
114+
if (s3Client != null) {
115+
s3Client.close();
116+
}
117+
}
118+
119+
@ParameterizedTest(name = "{0}")
120+
@MethodSource("uploadScenarios")
121+
void putObject_withContentTypeFromRequestBody_shouldPropagateContentType(String scenario, File file,
122+
boolean isMultipart,
123+
String expectedContentType) {
124+
stubSuccessfulResponses(isMultipart);
125+
126+
s3Client.putObject(r -> r.bucket(BUCKET).key(KEY), AsyncRequestBody.fromFile(file)).join();
127+
128+
if (isMultipart) {
129+
assertCreateMultipartUploadContentType(expectedContentType);
130+
} else {
131+
assertPutObjectContentType(expectedContentType);
132+
}
133+
}
134+
135+
@ParameterizedTest(name = "{0}")
136+
@MethodSource("uploadScenarios")
137+
void putObject_withExplicitContentType_shouldNotOverride(String scenario, File file,
138+
boolean isMultipart, String ignoredContentType) {
139+
stubSuccessfulResponses(isMultipart);
140+
String explicitContentType = "custom/type";
141+
142+
s3Client.putObject(r -> r.bucket(BUCKET).key(KEY).contentType(explicitContentType),
143+
AsyncRequestBody.fromFile(file)).join();
144+
145+
if (isMultipart) {
146+
assertCreateMultipartUploadContentType(explicitContentType);
147+
} else {
148+
assertPutObjectContentType(explicitContentType);
149+
}
150+
}
151+
152+
@Test
153+
void putObject_withNullContentTypeFromRequestBody_shouldUseDefaultContentType() {
154+
stubSuccessfulResponses(true);
155+
byte[] content = new byte[15 * 1024]; // 15KB - above threshold
156+
157+
AsyncRequestBody bodyWithNullContentType = new AsyncRequestBody() {
158+
@Override
159+
public Optional<Long> contentLength() {
160+
return Optional.of((long) content.length);
161+
}
162+
163+
@Override
164+
public String contentType() {
165+
return null;
166+
}
167+
168+
@Override
169+
public void subscribe(Subscriber<? super ByteBuffer> subscriber) {
170+
AsyncRequestBody.fromBytes(content).subscribe(subscriber);
171+
}
172+
};
173+
174+
s3Client.putObject(r -> r.bucket(BUCKET).key(KEY), bodyWithNullContentType).join();
175+
176+
// When AsyncRequestBody.contentType() is null, should use SDK default
177+
assertCreateMultipartUploadContentType("binary/octet-stream");
178+
}
179+
180+
private void stubSuccessfulResponses(boolean isMultipart) {
181+
if (isMultipart) {
182+
stubFor(post(anyUrl()).willReturn(aResponse().withStatus(200).withBody(CREATE_MULTIPART_RESPONSE)));
183+
stubFor(put(anyUrl()).willReturn(aResponse().withStatus(200).withHeader("ETag", "\"etag\"")));
184+
stubFor(post(urlPathEqualTo("/" + BUCKET + "/" + KEY))
185+
.withQueryParam("uploadId", containing("upload-id"))
186+
.willReturn(aResponse().withStatus(200).withBody(COMPLETE_MULTIPART_RESPONSE)));
187+
} else {
188+
stubFor(put(anyUrl()).willReturn(aResponse().withStatus(200).withHeader("ETag", "\"etag\"")));
189+
}
190+
}
191+
192+
private void assertCreateMultipartUploadContentType(String expectedContentType) {
193+
List<LoggedRequest> requests = findAll(postRequestedFor(urlPathEqualTo("/" + BUCKET + "/" + KEY))
194+
.withQueryParam("uploads", containing("")));
195+
assertThat(requests)
196+
.as("Expected CreateMultipartUpload request")
197+
.hasSize(1);
198+
assertThat(requests.get(0).getHeader("Content-Type"))
199+
.as("Content-Type header in CreateMultipartUpload request")
200+
.isEqualTo(expectedContentType);
201+
}
202+
203+
private void assertPutObjectContentType(String expectedContentType) {
204+
List<LoggedRequest> requests = findAll(putRequestedFor(urlPathEqualTo("/" + BUCKET + "/" + KEY)));
205+
assertThat(requests)
206+
.as("Expected PutObject request")
207+
.hasSize(1);
208+
assertThat(requests.get(0).getHeader("Content-Type"))
209+
.as("Content-Type header in PutObject request")
210+
.isEqualTo(expectedContentType);
211+
}
212+
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadObjectHelperTest.java‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,18 @@ void uploadObject_unKnownContentLengthDoesNotExceedPartSize_shouldUploadInOneChu
144144

145145
CompletableFuture<PutObjectResponse> completedFuture =
146146
CompletableFuture.completedFuture(PutObjectResponse.builder().build());
147-
when(s3AsyncClient.putObject(putObjectRequest, asyncRequestBody)).thenReturn(completedFuture);
147+
when(s3AsyncClient.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class))).thenReturn(completedFuture);
148148
uploadHelper.uploadObject(putObjectRequest, asyncRequestBody).join();
149-
Mockito.verify(s3AsyncClient).putObject(putObjectRequest, asyncRequestBody);
149+
150+
ArgumentCaptor<PutObjectRequest> requestCaptor = ArgumentCaptor.forClass(PutObjectRequest.class);
151+
Mockito.verify(s3AsyncClient).putObject(requestCaptor.capture(), any(AsyncRequestBody.class));
152+
153+
PutObjectRequest capturedRequest = requestCaptor.getValue();
154+
assertThat(capturedRequest.bucket()).isEqualTo(BUCKET);
155+
assertThat(capturedRequest.key()).isEqualTo(KEY);
156+
assertThat(capturedRequest.contentLength()).isEqualTo(contentLength);
157+
// Content-type should be enriched from AsyncRequestBody (default is application/octet-stream)
158+
assertThat(capturedRequest.contentType()).isEqualTo("application/octet-stream");
150159
}
151160

152161
@ParameterizedTest
@@ -414,6 +423,48 @@ void uploadObject_partsFinishedOutOfOrder_shouldSortThemInCompleteMultipart() {
414423
assertThat(actualRequest.multipartUpload().parts()).isEqualTo(completedParts(numTotalParts));
415424
}
416425

426+
@Test
427+
void uploadObject_contentTypeNotSetOnRequest_shouldUseContentTypeFromRequestBody() {
428+
PutObjectRequest putObjectRequest = PutObjectRequest.builder()
429+
.bucket(BUCKET)
430+
.key(KEY)
431+
.build();
432+
433+
stubSuccessfulCreateMultipartCall(UPLOAD_ID, s3AsyncClient);
434+
stubSuccessfulUploadPartCalls(s3AsyncClient);
435+
stubSuccessfulCompleteMultipartCall(BUCKET, KEY, s3AsyncClient);
436+
437+
uploadHelper.uploadObject(putObjectRequest, AsyncRequestBody.fromFile(testFile)).join();
438+
439+
ArgumentCaptor<CreateMultipartUploadRequest> requestCaptor =
440+
ArgumentCaptor.forClass(CreateMultipartUploadRequest.class);
441+
verify(s3AsyncClient).createMultipartUpload(requestCaptor.capture());
442+
443+
assertThat(requestCaptor.getValue().contentType()).isEqualTo("application/octet-stream");
444+
}
445+
446+
@Test
447+
void uploadObject_contentTypeSetOnRequest_shouldNotOverride() {
448+
String explicitContentType = "my/content-type";
449+
PutObjectRequest putObjectRequest = PutObjectRequest.builder()
450+
.bucket(BUCKET)
451+
.key(KEY)
452+
.contentType(explicitContentType)
453+
.build();
454+
455+
stubSuccessfulCreateMultipartCall(UPLOAD_ID, s3AsyncClient);
456+
stubSuccessfulUploadPartCalls(s3AsyncClient);
457+
stubSuccessfulCompleteMultipartCall(BUCKET, KEY, s3AsyncClient);
458+
459+
uploadHelper.uploadObject(putObjectRequest, AsyncRequestBody.fromFile(testFile)).join();
460+
461+
ArgumentCaptor<CreateMultipartUploadRequest> requestCaptor =
462+
ArgumentCaptor.forClass(CreateMultipartUploadRequest.class);
463+
verify(s3AsyncClient).createMultipartUpload(requestCaptor.capture());
464+
465+
assertThat(requestCaptor.getValue().contentType()).isEqualTo(explicitContentType);
466+
}
467+
417468
private List<CompletedPart> completedParts(int totalNumParts) {
418469
return IntStream.range(1, totalNumParts + 1).mapToObj(i -> CompletedPart.builder().partNumber(i).build()).collect(Collectors.toList());
419470
}

‎services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelperTest.java‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import static org.assertj.core.api.Assertions.assertThat;
1919
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2020
import static org.mockito.ArgumentMatchers.any;
21-
import static org.mockito.ArgumentMatchers.eq;
2221
import static org.mockito.Mockito.mock;
2322
import static org.mockito.Mockito.times;
2423
import static org.mockito.Mockito.verify;
@@ -28,6 +27,7 @@
2827
import static software.amazon.awssdk.services.s3.internal.multipart.utils.MultipartUploadTestUtils.stubSuccessfulPutObjectCall;
2928
import static software.amazon.awssdk.services.s3.internal.multipart.utils.MultipartUploadTestUtils.stubSuccessfulUploadPartCalls;
3029

30+
import java.io.File;
3131
import java.io.FileInputStream;
3232
import java.io.FileNotFoundException;
3333
import java.io.IOException;
@@ -53,6 +53,7 @@
5353
import software.amazon.awssdk.services.s3.S3AsyncClient;
5454
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest;
5555
import software.amazon.awssdk.services.s3.model.CompletedPart;
56+
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
5657
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
5758
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
5859
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
@@ -255,4 +256,50 @@ private void verifyCompleteMultipartUploadRequest() {
255256
CompleteMultipartUploadRequest actualRequest = completeMpuArgumentCaptor.getValue();
256257
assertThat(actualRequest.multipartUpload().parts()).isEqualTo(createCompletedParts(NUM_TOTAL_PARTS));
257258
}
259+
260+
@Test
261+
void uploadObject_unknownContentLength_contentTypeNotSet_shouldUseContentTypeFromRequestBody() throws IOException {
262+
File tempFile = File.createTempFile("test-file", ".txt");
263+
tempFile.deleteOnExit();
264+
byte[] data = new byte[(int) (PART_SIZE * 2 + 1024)];
265+
java.nio.file.Files.write(tempFile.toPath(), data);
266+
267+
stubSuccessfulCreateMultipartCall(UPLOAD_ID, s3AsyncClient);
268+
stubSuccessfulUploadPartCalls(s3AsyncClient);
269+
stubSuccessfulCompleteMultipartCall(BUCKET, KEY, s3AsyncClient);
270+
271+
AsyncRequestBody fileBody = AsyncRequestBody.fromFile(tempFile);
272+
AsyncRequestBody unknownLengthBody = new AsyncRequestBody() {
273+
@Override
274+
public java.util.Optional<Long> contentLength() {
275+
return java.util.Optional.empty();
276+
}
277+
278+
@Override
279+
public String contentType() {
280+
return fileBody.contentType();
281+
}
282+
283+
@Override
284+
public void subscribe(org.reactivestreams.Subscriber<? super java.nio.ByteBuffer> s) {
285+
fileBody.subscribe(s);
286+
}
287+
};
288+
289+
PutObjectRequest putObjectRequest = createPutObjectRequest();
290+
291+
UploadObjectHelper uploadObjectHelper = new UploadObjectHelper(s3AsyncClient,
292+
new MultipartConfigurationResolver(software.amazon.awssdk.services.s3.multipart.MultipartConfiguration.builder()
293+
.minimumPartSizeInBytes(PART_SIZE)
294+
.thresholdInBytes(PART_SIZE)
295+
.build()));
296+
297+
uploadObjectHelper.uploadObject(putObjectRequest, unknownLengthBody).join();
298+
299+
ArgumentCaptor<CreateMultipartUploadRequest> requestCaptor =
300+
ArgumentCaptor.forClass(CreateMultipartUploadRequest.class);
301+
verify(s3AsyncClient).createMultipartUpload(requestCaptor.capture());
302+
303+
assertThat(requestCaptor.getValue().contentType()).isEqualTo("text/plain");
304+
}
258305
}

0 commit comments

Comments
 (0)