Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AmazonS3-31226da.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "Amazon S3",
"contributor": "",
"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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import software.amazon.awssdk.core.async.AsyncRequestBody;
import software.amazon.awssdk.core.async.CloseableAsyncRequestBody;
import software.amazon.awssdk.core.async.SdkPublisher;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.services.s3.S3AsyncClient;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
Expand Down Expand Up @@ -61,6 +62,13 @@ public CompletableFuture<PutObjectResponse> uploadObject(PutObjectRequest putObj
AsyncRequestBody asyncRequestBody) {
CompletableFuture<PutObjectResponse> returnFuture = new CompletableFuture<>();

if (maxMemoryUsageInBytes < 2 * partSizeInBytes) {
returnFuture.completeExceptionally(SdkClientException.create(
"apiCallBufferSizeInBytes (" + maxMemoryUsageInBytes + ") must be at least 2 x minimumPartSizeInBytes ("
+ partSizeInBytes + ") when uploading content with an unknown content length"));
return returnFuture;
}

SdkPublisher<CloseableAsyncRequestBody> splitAsyncRequestBodyResponse =
asyncRequestBody.splitCloseable(b -> b.chunkSizeInBytes(partSizeInBytes)
.bufferSizeInBytes(maxMemoryUsageInBytes));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ public interface Builder extends CopyableBuilder<Builder, MultipartConfiguration
* <p>
* Default value: If not specified, the SDK will use the equivalent of four parts worth of memory, so 32 Mib by default.
* <p>
* When uploading content with an unknown content length, this value
* must be at least 2 x {@link #minimumPartSizeInBytes(Long)}.
* The unknown-length upload path buffers one part while it buffers the next to determine whether a single-part or
* multipart upload is needed, so it requires room for two parts. A value smaller than that will be rejected.
* <p>
* This setting does not apply if you are using an {@link AsyncResponseTransformer} implementation that downloads the
* object into memory such as {@link AsyncResponseTransformer#toBytes}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -145,6 +146,37 @@ void uploadObject_emptyBody_shouldSucceed() {
assertThat(actualRequestBodies.get(0).contentLength()).hasValue(0L);
}

@Test
void uploadObject_apiCallBufferSizeLessThanTwicePartSize_shouldFailFastWithoutSplitting() {
UploadWithUnknownContentLengthHelper helperWithSmallBuffer =
new UploadWithUnknownContentLengthHelper(s3AsyncClient, PART_SIZE, PART_SIZE, 2 * PART_SIZE - 1, 50);

CloseableAsyncRequestBody asyncRequestBody = createMockAsyncRequestBody(PART_SIZE);

CompletableFuture<PutObjectResponse> future =
helperWithSmallBuffer.uploadObject(createPutObjectRequest(), asyncRequestBody);

verifyFailureWithMessage(future, "must be at least 2 x minimumPartSizeInBytes");

verify(asyncRequestBody, never()).splitCloseable(any(Consumer.class));
}

@Test
void uploadObject_apiCallBufferSizeEqualToTwicePartSize_shouldNotFailFast() {
UploadWithUnknownContentLengthHelper helperWithBoundaryBuffer =
new UploadWithUnknownContentLengthHelper(s3AsyncClient, PART_SIZE, PART_SIZE, 2 * PART_SIZE, 50);

CloseableAsyncRequestBody asyncRequestBody = createMockAsyncRequestBody(PART_SIZE);
SdkPublisher<CloseableAsyncRequestBody> mockPublisher = mock(SdkPublisher.class);
when(asyncRequestBody.splitCloseable(any(Consumer.class))).thenReturn(mockPublisher);

CompletableFuture<PutObjectResponse> future =
helperWithBoundaryBuffer.uploadObject(createPutObjectRequest(), asyncRequestBody);

assertThat(future).isNotCompleted();
verify(asyncRequestBody, times(1)).splitCloseable(any(Consumer.class));
}

@Test
void uploadObject_withPartSizeExceedingLimit_shouldFailRequest() {
CloseableAsyncRequestBody asyncRequestBody = createMockAsyncRequestBody(PART_SIZE + 1);
Expand Down
Loading