Skip to content

Commit de7f9e6

Browse files
authored
Add validation for buffer lengthwhen multipart upload with unknown contentlength (#7024)
* add validation for buffer length when multipart upload with unknown content length * address comments * fix checkstyle
1 parent 1cb7981 commit de7f9e6

4 files changed

Lines changed: 51 additions & 0 deletions

File tree

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 an issue where S3 multipart uploads with unknown content length could hang indefinitely when apiCallBufferSizeInBytes was less than twice minimumPartSizeInBytes. The SDK now validates this at request time and fails fast with a descriptive error instead of deadlocking"
6+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import software.amazon.awssdk.core.async.AsyncRequestBody;
2121
import software.amazon.awssdk.core.async.CloseableAsyncRequestBody;
2222
import software.amazon.awssdk.core.async.SdkPublisher;
23+
import software.amazon.awssdk.core.exception.SdkClientException;
2324
import software.amazon.awssdk.services.s3.S3AsyncClient;
2425
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
2526
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
@@ -61,6 +62,13 @@ public CompletableFuture<PutObjectResponse> uploadObject(PutObjectRequest putObj
6162
AsyncRequestBody asyncRequestBody) {
6263
CompletableFuture<PutObjectResponse> returnFuture = new CompletableFuture<>();
6364

65+
if (maxMemoryUsageInBytes < 2 * partSizeInBytes) {
66+
returnFuture.completeExceptionally(SdkClientException.create(
67+
"apiCallBufferSizeInBytes (" + maxMemoryUsageInBytes + ") must be at least 2 x minimumPartSizeInBytes ("
68+
+ partSizeInBytes + ") when uploading content with an unknown content length"));
69+
return returnFuture;
70+
}
71+
6472
SdkPublisher<CloseableAsyncRequestBody> splitAsyncRequestBodyResponse =
6573
asyncRequestBody.splitCloseable(b -> b.chunkSizeInBytes(partSizeInBytes)
6674
.bufferSizeInBytes(maxMemoryUsageInBytes));

services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,11 @@ public interface Builder extends CopyableBuilder<Builder, MultipartConfiguration
174174
* <p>
175175
* Default value: If not specified, the SDK will use the equivalent of four parts worth of memory, so 32 Mib by default.
176176
* <p>
177+
* When uploading content with an unknown content length, this value
178+
* must be at least 2 x {@link #minimumPartSizeInBytes(Long)}.
179+
* The unknown-length upload path buffers one part while it buffers the next to determine whether a single-part or
180+
* multipart upload is needed, so it requires room for two parts. A value smaller than that will be rejected.
181+
* <p>
177182
* This setting does not apply if you are using an {@link AsyncResponseTransformer} implementation that downloads the
178183
* object into memory such as {@link AsyncResponseTransformer#toBytes}
179184
*

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.Assertions.assertThatThrownBy;
2020
import static org.mockito.ArgumentMatchers.any;
2121
import static org.mockito.Mockito.mock;
22+
import static org.mockito.Mockito.never;
2223
import static org.mockito.Mockito.times;
2324
import static org.mockito.Mockito.verify;
2425
import static org.mockito.Mockito.when;
@@ -145,6 +146,37 @@ void uploadObject_emptyBody_shouldSucceed() {
145146
assertThat(actualRequestBodies.get(0).contentLength()).hasValue(0L);
146147
}
147148

149+
@Test
150+
void uploadObject_apiCallBufferSizeLessThanTwicePartSize_shouldFailFastWithoutSplitting() {
151+
UploadWithUnknownContentLengthHelper helperWithSmallBuffer =
152+
new UploadWithUnknownContentLengthHelper(s3AsyncClient, PART_SIZE, PART_SIZE, 2 * PART_SIZE - 1, 50);
153+
154+
CloseableAsyncRequestBody asyncRequestBody = createMockAsyncRequestBody(PART_SIZE);
155+
156+
CompletableFuture<PutObjectResponse> future =
157+
helperWithSmallBuffer.uploadObject(createPutObjectRequest(), asyncRequestBody);
158+
159+
verifyFailureWithMessage(future, "must be at least 2 x minimumPartSizeInBytes");
160+
161+
verify(asyncRequestBody, never()).splitCloseable(any(Consumer.class));
162+
}
163+
164+
@Test
165+
void uploadObject_apiCallBufferSizeEqualToTwicePartSize_shouldNotFailFast() {
166+
UploadWithUnknownContentLengthHelper helperWithBoundaryBuffer =
167+
new UploadWithUnknownContentLengthHelper(s3AsyncClient, PART_SIZE, PART_SIZE, 2 * PART_SIZE, 50);
168+
169+
CloseableAsyncRequestBody asyncRequestBody = createMockAsyncRequestBody(PART_SIZE);
170+
SdkPublisher<CloseableAsyncRequestBody> mockPublisher = mock(SdkPublisher.class);
171+
when(asyncRequestBody.splitCloseable(any(Consumer.class))).thenReturn(mockPublisher);
172+
173+
CompletableFuture<PutObjectResponse> future =
174+
helperWithBoundaryBuffer.uploadObject(createPutObjectRequest(), asyncRequestBody);
175+
176+
assertThat(future).isNotCompleted();
177+
verify(asyncRequestBody, times(1)).splitCloseable(any(Consumer.class));
178+
}
179+
148180
@Test
149181
void uploadObject_withPartSizeExceedingLimit_shouldFailRequest() {
150182
CloseableAsyncRequestBody asyncRequestBody = createMockAsyncRequestBody(PART_SIZE + 1);

0 commit comments

Comments
 (0)