diff --git a/.changes/next-release/bugfix-AmazonS3-31226da.json b/.changes/next-release/bugfix-AmazonS3-31226da.json new file mode 100644 index 000000000000..249e5003a12b --- /dev/null +++ b/.changes/next-release/bugfix-AmazonS3-31226da.json @@ -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" +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelper.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelper.java index aba0c8e63221..6ffa63f56a00 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelper.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelper.java @@ -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; @@ -61,6 +62,13 @@ public CompletableFuture uploadObject(PutObjectRequest putObj AsyncRequestBody asyncRequestBody) { CompletableFuture 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 splitAsyncRequestBodyResponse = asyncRequestBody.splitCloseable(b -> b.chunkSizeInBytes(partSizeInBytes) .bufferSizeInBytes(maxMemoryUsageInBytes)); diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java index f1d5ff35c60c..64bc2f308463 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/multipart/MultipartConfiguration.java @@ -174,6 +174,11 @@ public interface Builder extends CopyableBuilder * Default value: If not specified, the SDK will use the equivalent of four parts worth of memory, so 32 Mib by default. *

+ * 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. + *

* This setting does not apply if you are using an {@link AsyncResponseTransformer} implementation that downloads the * object into memory such as {@link AsyncResponseTransformer#toBytes} * diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelperTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelperTest.java index c32b791d52f0..daf17f82b5a3 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelperTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/UploadWithUnknownContentLengthHelperTest.java @@ -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; @@ -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 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 mockPublisher = mock(SdkPublisher.class); + when(asyncRequestBody.splitCloseable(any(Consumer.class))).thenReturn(mockPublisher); + + CompletableFuture 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);