From 3d21f248005d43211d26f3d08e597d006a31da9b Mon Sep 17 00:00:00 2001 From: jencymaryjoseph <35571282+jencymaryjoseph@users.noreply.github.com> Date: Mon, 11 May 2026 12:42:18 -0700 Subject: [PATCH] Fix presigned url multipart download for empty objects --- ...dUrlExtensionMultipartIntegrationTest.java | 35 ++++++++ .../AsyncPresignedUrlExtensionTestSuite.java | 40 ++++++++++ ...ignedUrlMultipartDownloaderSubscriber.java | 13 ++- .../multipart/PresignedUrlDownloadHelper.java | 50 +++++++++++- ...ignedUrlMultipartDownloaderSubscriber.java | 11 ++- ...dUrlMultipartDownloaderSubscriberTest.java | 63 +++++++++++++++ ...ipartDownloaderSubscriberWiremockTest.java | 79 +++++++++++++++++++ 7 files changed, 286 insertions(+), 5 deletions(-) create mode 100644 services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionMultipartIntegrationTest.java diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionMultipartIntegrationTest.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionMultipartIntegrationTest.java new file mode 100644 index 000000000000..16e41325817f --- /dev/null +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionMultipartIntegrationTest.java @@ -0,0 +1,35 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package software.amazon.awssdk.services.s3.presignedurl; + +import org.junit.jupiter.api.BeforeAll; +import software.amazon.awssdk.services.s3.S3AsyncClient; + + +public class AsyncPresignedUrlExtensionMultipartIntegrationTest extends AsyncPresignedUrlExtensionTestSuite { + + @BeforeAll + static void setUpIntegrationTest() { + S3AsyncClient s3AsyncClient = s3AsyncClientBuilder() + .multipartEnabled(true) + .build(); + presignedUrlExtension = s3AsyncClient.presignedUrlExtension(); + } + + @Override + protected S3AsyncClient createS3AsyncClient() { + return s3AsyncClientBuilder().build(); + } +} diff --git a/services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionTestSuite.java b/services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionTestSuite.java index eee244ca3485..d32c7038317a 100644 --- a/services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionTestSuite.java +++ b/services/s3/src/it/java/software/amazon/awssdk/services/s3/presignedurl/AsyncPresignedUrlExtensionTestSuite.java @@ -16,6 +16,7 @@ import static org.apache.commons.lang3.RandomStringUtils.randomAscii; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import java.io.ByteArrayInputStream; import java.net.URL; @@ -47,6 +48,7 @@ import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; import software.amazon.awssdk.services.s3.model.GetObjectResponse; import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.services.s3.model.S3Exception; import software.amazon.awssdk.services.s3.presigner.S3Presigner; import software.amazon.awssdk.services.s3.presigner.model.PresignedGetObjectRequest; import software.amazon.awssdk.services.s3.presignedurl.model.PresignedUrlDownloadRequest; @@ -240,6 +242,44 @@ public void close() {} } } + @Test + void getObject_emptyObject_toBytes_shouldSucceed() throws Exception { + String emptyKey = uploadTestObject("empty-object-bytes", ""); + + PresignedUrlDownloadRequest request = createRequestForKey(emptyKey); + ResponseBytes response = + presignedUrlExtension.getObject(request, AsyncResponseTransformer.toBytes()) + .get(30, TimeUnit.SECONDS); + + assertThat(response.asByteArray()).isEmpty(); + assertThat(response.response().contentLength()).isEqualTo(0L); + } + + @Test + void getObject_emptyObject_toFile_shouldSucceed() throws Exception { + String emptyKey = uploadTestObject("empty-object-file", ""); + + PresignedUrlDownloadRequest request = createRequestForKey(emptyKey); + Path downloadFile = temporaryFolder.resolve("empty-download-" + UUID.randomUUID() + ".bin"); + GetObjectResponse response = + presignedUrlExtension.getObject(request, downloadFile) + .get(30, TimeUnit.SECONDS); + + assertThat(downloadFile).exists(); + assertThat(downloadFile.toFile().length()).isEqualTo(0L); + } + + @Test + void getObject_emptyObject_withRange_shouldThrow416() throws Exception { + String emptyKey = uploadTestObject("empty-object-range", ""); + + PresignedUrlDownloadRequest request = createRequestForKey(emptyKey, "bytes=0-1024"); + + assertThatThrownBy(() -> presignedUrlExtension.getObject(request, AsyncResponseTransformer.toBytes()) + .get(30, TimeUnit.SECONDS)) + .hasCauseInstanceOf(S3Exception.class); + } + static Stream basicFunctionalityTestData() { return Stream.of( Arguments.of("getObject_withValidUrl_returnsContent", diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriber.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriber.java index 8a7c898b3619..390ea6cfbc74 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriber.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriber.java @@ -133,11 +133,18 @@ private void sendFirstRequest(AsyncResponseTransformer { - if (error != null || isCompletedExceptionally.get()) { - handlePartError(error, 0); + if (error != null) { + if (PresignedUrlDownloadHelper.isRangeNotSatisfiable(error)) { + resultFuture.completeExceptionally( + new PresignedUrlDownloadHelper.EmptyObjectRangeNotSatisfiableException(error)); + synchronized (subscriptionLock) { + subscription.cancel(); + } + } else { + handlePartError(error, 0); + } return; } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlDownloadHelper.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlDownloadHelper.java index f62ec1d7d202..3f5c955abad7 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlDownloadHelper.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlDownloadHelper.java @@ -16,12 +16,14 @@ package software.amazon.awssdk.services.s3.internal.multipart; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SplittingTransformerConfiguration; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.services.s3.S3AsyncClient; import software.amazon.awssdk.services.s3.model.GetObjectResponse; +import software.amazon.awssdk.services.s3.model.S3Exception; import software.amazon.awssdk.services.s3.presignedurl.AsyncPresignedUrlExtension; import software.amazon.awssdk.services.s3.presignedurl.model.PresignedUrlDownloadRequest; import software.amazon.awssdk.utils.Logger; @@ -60,6 +62,33 @@ public CompletableFuture downloadObject( return asyncPresignedUrlExtension.getObject(presignedRequest, asyncResponseTransformer); } + CompletableFuture resultFuture = new CompletableFuture<>(); + doMultipartDownload(presignedRequest, asyncResponseTransformer) + .whenComplete((result, error) -> { + Throwable cause = error instanceof CompletionException ? error.getCause() : error; + if (cause instanceof EmptyObjectRangeNotSatisfiableException) { + log.debug(() -> "Received 416 on first request, falling back to non-range GET for empty object"); + asyncPresignedUrlExtension.getObject(presignedRequest, asyncResponseTransformer) + .whenComplete((r, e) -> { + if (e != null) { + resultFuture.completeExceptionally(e); + } else { + resultFuture.complete(r); + } + }); + } else if (error != null) { + resultFuture.completeExceptionally(error); + } else { + resultFuture.complete(result); + } + }); + return resultFuture; + } + + private CompletableFuture doMultipartDownload( + PresignedUrlDownloadRequest presignedRequest, + AsyncResponseTransformer asyncResponseTransformer) { + SplittingTransformerConfiguration splittingConfig = SplittingTransformerConfiguration.builder() .bufferSizeInBytes(bufferSizeInBytes) .build(); @@ -112,4 +141,23 @@ static SdkClientException missingContentRangeHeader() { static SdkClientException invalidContentLength() { return SdkClientException.create("Invalid or missing Content-Length in response"); } -} + + /** + * Returns true if the error is a 416 Range Not Satisfiable response from S3. + * Used by subscribers to detect empty object responses on the first range request. + */ + static boolean isRangeNotSatisfiable(Throwable error) { + Throwable cause = error instanceof CompletionException ? error.getCause() : error; + return cause instanceof S3Exception && ((S3Exception) cause).statusCode() == 416; + } + + /** + * Marker exception wrapping a 416 on the first range request, signaling an empty object. + * Used to distinguish from 416 errors on subsequent requests which should propagate as failures. + */ + static class EmptyObjectRangeNotSatisfiableException extends RuntimeException { + EmptyObjectRangeNotSatisfiableException(Throwable cause) { + super("Object is empty (416 on first range request)", cause); + } + } +} \ No newline at end of file diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriber.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriber.java index d76f0a8e8e6b..a4b4326b464b 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriber.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriber.java @@ -130,7 +130,16 @@ private void makeRangeRequest(int partIndex, .getObject(partRequest, asyncResponseTransformer) .whenComplete((response, error) -> { if (error != null) { - handleError(error); + if (partIndex == 0 && PresignedUrlDownloadHelper.isRangeNotSatisfiable(error)) { + log.debug(() -> "Received 416 on first range request, object is empty"); + resultFuture.completeExceptionally( + new PresignedUrlDownloadHelper.EmptyObjectRangeNotSatisfiableException(error)); + synchronized (lock) { + subscription.cancel(); + } + } else { + handleError(error); + } return; } if (validatePart(response, partIndex, asyncResponseTransformer)) { diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriberTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriberTest.java index 0062029e1b83..0ef414745ea5 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriberTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/ParallelPresignedUrlMultipartDownloaderSubscriberTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.services.s3.internal.multipart; import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.absent; import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.matching; @@ -234,6 +235,68 @@ void onNext_withNullTransformer_shouldThrowNPE() { .isInstanceOf(NullPointerException.class); } + @Test + void emptyObject_416OnFirstRequest_shouldFallbackToNonRangeGet() { + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .withHeader("Range", matching("bytes=.*")) + .willReturn(aResponse() + .withStatus(416) + .withBody("InvalidRange" + + "The requested range is not satisfiable"))); + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .withHeader("Range", absent()) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Length", "0") + .withHeader("ETag", "\"empty-etag\"") + .withBody(new byte[0]))); + + tempFile = createTempFileUnchecked(); + PresignedUrlDownloadRequest request = PresignedUrlDownloadRequest.builder() + .presignedUrl(presignedUrl) + .build(); + + s3AsyncClient.presignedUrlExtension() + .getObject(request, AsyncResponseTransformer.toFile(tempFile)) + .join(); + + assertThat(tempFile.toFile()).exists(); + assertThat(tempFile.toFile().length()).isEqualTo(0L); + } + + @Test + void multipartObject_416OnSecondRequest_shouldFailWithError() { + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .inScenario("416-on-second") + .whenScenarioStateIs("Started") + .withHeader("Range", matching("bytes=0-15")) + .willReturn(aResponse() + .withStatus(206) + .withHeader("Content-Length", "16") + .withHeader("Content-Range", "bytes 0-15/32") + .withHeader("ETag", "\"test-etag\"") + .withBody(Arrays.copyOfRange(TEST_DATA, 0, 16))) + .willSetStateTo("first-done")); + + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .inScenario("416-on-second") + .whenScenarioStateIs("first-done") + .willReturn(aResponse() + .withStatus(416) + .withBody("InvalidRange" + + "The requested range is not satisfiable"))); + + tempFile = createTempFileUnchecked(); + PresignedUrlDownloadRequest request = PresignedUrlDownloadRequest.builder() + .presignedUrl(presignedUrl) + .build(); + + assertThatThrownBy(() -> s3AsyncClient.presignedUrlExtension() + .getObject(request, AsyncResponseTransformer.toFile(tempFile)) + .join()) + .isInstanceOf(CompletionException.class); + } + private static Path createTempFile() throws IOException { Path path = Files.createTempFile("parallel-test-" + UUID.randomUUID(), ".tmp"); Files.deleteIfExists(path); diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriberWiremockTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriberWiremockTest.java index 2ff0e4da46b7..90875fa524be 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriberWiremockTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/PresignedUrlMultipartDownloaderSubscriberWiremockTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.services.s3.internal.multipart; import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.absent; import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; @@ -260,6 +261,84 @@ void onNext_withNullTransformer_shouldThrowException() { .hasMessageContaining("onNext must not be called with null asyncResponseTransformer"); } + @ParameterizedTest(name = "presignedUrlDownload_emptyObject_shouldFallbackToNonRangeGet [{0}]") + @MethodSource("transformerTypes") + @SuppressWarnings("unchecked") + void presignedUrlDownload_emptyObject_shouldFallbackToNonRangeGet(String transformerType) throws IOException { + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .withHeader("Range", matching("bytes=.*")) + .willReturn(aResponse() + .withStatus(416) + .withBody("InvalidRange" + + "The requested range is not satisfiable"))); + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .withHeader("Range", absent()) + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Length", "0") + .withHeader("ETag", "\"empty-etag\"") + .withBody(new byte[0]))); + + PresignedUrlDownloadRequest request = PresignedUrlDownloadRequest.builder() + .presignedUrl(presignedUrl) + .build(); + Object result = executeDownload(request, transformerType).join(); + if ("toBytes".equals(transformerType)) { + ResponseBytes bytes = (ResponseBytes) result; + assertThat(bytes.asByteArray()).isEmpty(); + } else { + assertThat(tempFile.toFile()).exists(); + assertThat(Files.size(tempFile)).isEqualTo(0L); + } + } + + @ParameterizedTest(name = "presignedUrlDownload_416OnSecondRequest_shouldFailWithError [{0}]") + @MethodSource("transformerTypes") + void presignedUrlDownload_416OnSecondRequest_shouldFailWithError(String transformerType) { + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .inScenario("416-on-second") + .whenScenarioStateIs("Started") + .withHeader("Range", matching("bytes=0-15")) + .willReturn(aResponse() + .withStatus(206) + .withHeader("Content-Length", "16") + .withHeader("Content-Range", "bytes 0-15/32") + .withHeader("ETag", "\"test-etag\"") + .withBody(Arrays.copyOfRange(TEST_DATA, 0, 16))) + .willSetStateTo("first-done")); + + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .inScenario("416-on-second") + .whenScenarioStateIs("first-done") + .willReturn(aResponse() + .withStatus(416) + .withBody("InvalidRange" + + "The requested range is not satisfiable"))); + + PresignedUrlDownloadRequest request = PresignedUrlDownloadRequest.builder() + .presignedUrl(presignedUrl) + .build(); + assertThatThrownBy(() -> executeDownload(request, transformerType).join()) + .hasRootCauseInstanceOf(S3Exception.class); + } + + @ParameterizedTest(name = "presignedUrlDownload_withRangeHeader_emptyObject_shouldThrow416 [{0}]") + @MethodSource("transformerTypes") + void presignedUrlDownload_withRangeHeader_emptyObject_shouldThrow416(String transformerType) { + stubFor(get(urlEqualTo(PRESIGNED_URL_PATH)) + .willReturn(aResponse() + .withStatus(416) + .withBody("InvalidRange" + + "The requested range is not satisfiable"))); + + PresignedUrlDownloadRequest request = PresignedUrlDownloadRequest.builder() + .presignedUrl(presignedUrl) + .range("bytes=0-1024") + .build(); + assertThatThrownBy(() -> executeDownload(request, transformerType).join()) + .hasRootCauseInstanceOf(S3Exception.class); + } + @AfterEach void cleanup() { if (tempFile != null && Files.exists(tempFile)) {