From a442920774687f3eee5f34b01b765df84c774d2c Mon Sep 17 00:00:00 2001 From: David Ho Date: Fri, 4 Sep 2026 16:03:08 -0700 Subject: [PATCH 1/4] Fix CRT S3 getObject file handling --- .../next-release/bugfix-AWSS3-9d4c2a1.json | 6 ++ .../internal/crt/DefaultS3CrtAsyncClient.java | 16 ++-- .../s3/internal/crt/S3CrtAsyncHttpClient.java | 24 +++++ .../S3InternalSdkHttpExecutionAttribute.java | 3 + .../services/s3/crt/CrtDownloadErrorTest.java | 89 +++++++++++++++++++ .../crt/DefaultS3CrtAsyncClientTest.java | 37 ++++++++ .../crt/S3CrtAsyncHttpClientTest.java | 36 ++++++++ 7 files changed, 206 insertions(+), 5 deletions(-) create mode 100644 .changes/next-release/bugfix-AWSS3-9d4c2a1.json diff --git a/.changes/next-release/bugfix-AWSS3-9d4c2a1.json b/.changes/next-release/bugfix-AWSS3-9d4c2a1.json new file mode 100644 index 000000000000..a685bae55410 --- /dev/null +++ b/.changes/next-release/bugfix-AWSS3-9d4c2a1.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "Amazon S3", + "contributor": "", + "description": "Prevent the CRT-based S3 async client from replacing a pre-existing destination or leaving behind a file when `getObject(request, Path)` fails or is cancelled. Existing destinations now surface `FileAlreadyExistsException`; callers that require replacement can use `AsyncResponseTransformer.toFile` with `FileTransformerConfiguration.defaultCreateOrReplaceExisting()`." +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClient.java index 8ef59ac7cca8..6f0fef9c5f8f 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClient.java @@ -91,6 +91,8 @@ public final class DefaultS3CrtAsyncClient extends DelegatingS3AsyncClient imple public static final ExecutionAttribute RESPONSE_FILE_PATH = new ExecutionAttribute<>("responseFilePath"); public static final ExecutionAttribute RESPONSE_FILE_OPTION = new ExecutionAttribute<>("responseFileOption"); + public static final ExecutionAttribute RESPONSE_FILE_DELETE_ON_FAILURE = + new ExecutionAttribute<>("responseFileDeleteOnFailure"); private static final String CRT_CLIENT_CLASSPATH = "software.amazon.awssdk.crt.s3.S3Client"; private final CopyObjectHelper copyObjectHelper; @@ -124,10 +126,12 @@ public CompletableFuture getObject(GetObjectRequest getObject AwsRequestOverrideConfiguration overrideConfig = getObjectRequest.overrideConfiguration() - .map(config -> config.toBuilder().putExecutionAttribute(RESPONSE_FILE_PATH, destinationPath)) - .orElseGet(() -> AwsRequestOverrideConfiguration.builder() - .putExecutionAttribute(RESPONSE_FILE_PATH, - destinationPath)) + .map(AwsRequestOverrideConfiguration::toBuilder) + .orElseGet(AwsRequestOverrideConfiguration::builder) + .putExecutionAttribute(RESPONSE_FILE_PATH, destinationPath) + .putExecutionAttribute(RESPONSE_FILE_OPTION, + S3MetaRequestOptions.ResponseFileOption.CREATE_NEW) + .putExecutionAttribute(RESPONSE_FILE_DELETE_ON_FAILURE, true) .build(); return getObject(getObjectRequest.toBuilder().overrideConfiguration(overrideConfig).build(), responseTransformer); @@ -439,7 +443,9 @@ public void afterMarshalling(Context.AfterMarshalling context, .put(S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_PATH, executionAttributes.getAttribute(RESPONSE_FILE_PATH)) .put(S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_OPTION, - executionAttributes.getAttribute(RESPONSE_FILE_OPTION)); + executionAttributes.getAttribute(RESPONSE_FILE_OPTION)) + .put(S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_DELETE_ON_FAILURE, + executionAttributes.getAttribute(RESPONSE_FILE_DELETE_ON_FAILURE)); SdkRequest request = context.request(); if (request instanceof AwsRequest) { diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java index e2406b7bd962..139b5cf70168 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java @@ -24,6 +24,7 @@ import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.OPERATION_NAME; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.REQUEST_CHECKSUM_CALCULATION; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_CHECKSUM_VALIDATION; +import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_DELETE_ON_FAILURE; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_OPTION; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_PATH; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.SIGNING_NAME; @@ -32,6 +33,7 @@ import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely; import java.net.URI; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Path; import java.time.Duration; import java.util.ArrayList; @@ -43,6 +45,7 @@ import software.amazon.awssdk.core.checksums.RequestChecksumCalculation; import software.amazon.awssdk.core.checksums.ResponseChecksumValidation; import software.amazon.awssdk.core.interceptor.trait.HttpChecksum; +import software.amazon.awssdk.crt.CrtRuntimeException; import software.amazon.awssdk.crt.auth.credentials.CredentialsProvider; import software.amazon.awssdk.crt.auth.signing.AwsSigningConfig; import software.amazon.awssdk.crt.http.HttpHeader; @@ -70,6 +73,7 @@ */ @SdkInternalApi public final class S3CrtAsyncHttpClient implements SdkAsyncHttpClient { + private static final String CRT_RESPONSE_FILE_ALREADY_EXISTS = "AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS"; private final S3Client crtS3Client; @@ -159,6 +163,7 @@ public CompletableFuture execute(AsyncExecuteRequest asyncRequest) { Path responseFilePath = httpExecutionAttributes.getAttribute(RESPONSE_FILE_PATH); S3MetaRequestOptions.ResponseFileOption responseFileOption = httpExecutionAttributes.getAttribute(RESPONSE_FILE_OPTION); + Boolean responseFileDeleteOnFailure = httpExecutionAttributes.getAttribute(RESPONSE_FILE_DELETE_ON_FAILURE); S3CrtResponseHandlerAdapter responseHandler = new S3CrtResponseHandlerAdapter( @@ -186,6 +191,9 @@ public CompletableFuture execute(AsyncExecuteRequest asyncRequest) { if (responseFileOption != null) { requestOptions = requestOptions.withResponseFileOption(responseFileOption); } + if (responseFileDeleteOnFailure != null) { + requestOptions = requestOptions.withResponseFileDeleteOnFailure(responseFileDeleteOnFailure); + } CrtCredentialsProviderAdapter requestCredentialsAdapter = httpExecutionAttributes.getAttribute(S3InternalSdkHttpExecutionAttribute.CRT_CREDENTIALS_PROVIDER_ADAPTER); @@ -204,6 +212,16 @@ public CompletableFuture execute(AsyncExecuteRequest asyncRequest) { if (requestCredentialsAdapter != null) { requestCredentialsAdapter.close(); } + if (isResponseFileAlreadyExistsError(t, responseFilePath)) { + FileAlreadyExistsException fileAlreadyExistsException = + new FileAlreadyExistsException(responseFilePath.toString()); + fileAlreadyExistsException.addSuppressed(t); + // Native initialization failed before creating a meta-request. Complete its placeholder first because + // completing executeFuture invokes the response adapter synchronously, which otherwise waits for a timeout. + s3MetaRequestFuture.complete(null); + executeFuture.completeExceptionally(fileAlreadyExistsException); + return executeFuture; + } throw t; } finally { signingConfig.close(); @@ -216,6 +234,12 @@ public CompletableFuture execute(AsyncExecuteRequest asyncRequest) { return executeFuture; } + private static boolean isResponseFileAlreadyExistsError(Throwable throwable, Path responseFilePath) { + return responseFilePath != null + && throwable instanceof CrtRuntimeException + && CRT_RESPONSE_FILE_ALREADY_EXISTS.equals(((CrtRuntimeException) throwable).errorName); + } + private AwsSigningConfig awsSigningConfig(Region signingRegion, SdkHttpExecutionAttributes httpExecutionAttributes) { CrtCredentialsProviderAdapter requestAdapter = httpExecutionAttributes.getAttribute(S3InternalSdkHttpExecutionAttribute.CRT_CREDENTIALS_PROVIDER_ADAPTER); diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3InternalSdkHttpExecutionAttribute.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3InternalSdkHttpExecutionAttribute.java index c9138539c81c..7ddf40998a48 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3InternalSdkHttpExecutionAttribute.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3InternalSdkHttpExecutionAttribute.java @@ -64,6 +64,9 @@ public final class S3InternalSdkHttpExecutionAttribute extends SdkHttpExecuti public static final S3InternalSdkHttpExecutionAttribute RESPONSE_FILE_OPTION = new S3InternalSdkHttpExecutionAttribute<>(S3MetaRequestOptions.ResponseFileOption.class); + public static final S3InternalSdkHttpExecutionAttribute RESPONSE_FILE_DELETE_ON_FAILURE = + new S3InternalSdkHttpExecutionAttribute<>(Boolean.class); + public static final S3InternalSdkHttpExecutionAttribute CRT_CREDENTIALS_PROVIDER_ADAPTER = new S3InternalSdkHttpExecutionAttribute<>(CrtCredentialsProviderAdapter.class); diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java index e7c26f5469ac..5e497abef510 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java @@ -15,27 +15,39 @@ package software.amazon.awssdk.services.s3.crt; +import static com.github.tomakehurst.wiremock.client.WireMock.anyRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.exactly; import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.client.WireMock.head; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.verify; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; import com.github.tomakehurst.wiremock.junit5.WireMockTest; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CompletableFuture; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.io.TempDir; +import software.amazon.awssdk.core.FileTransformerConfiguration; import software.amazon.awssdk.core.async.AsyncResponseTransformer; import software.amazon.awssdk.crt.Log; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.s3.S3AsyncClient; +import software.amazon.awssdk.services.s3.model.GetObjectResponse; import software.amazon.awssdk.services.s3.model.S3Exception; @WireMockTest @@ -44,6 +56,8 @@ public class CrtDownloadErrorTest { private static final String BUCKET = "my-bucket"; private static final String KEY = "my-key"; private S3AsyncClient s3; + @TempDir + private Path tempDir; @BeforeAll public static void setUpBeforeAll() { @@ -120,6 +134,81 @@ public void getObject_headObjectOk_getObjectOk_operationSucceeds() { assertThat(objectContent.getBytes(StandardCharsets.UTF_8)).isEqualTo(content); } + @Test + public void getObjectToPath_success_writesFile() throws Exception { + String requestPath = String.format("/%s/%s", BUCKET, KEY); + byte[] content = "hello".getBytes(StandardCharsets.UTF_8); + stubFor(head(urlPathEqualTo(requestPath)) + .willReturn(WireMock.aResponse() + .withStatus(200) + .withHeader("ETag", "etag") + .withHeader("Content-Length", Integer.toString(content.length)))); + stubFor(get(urlPathEqualTo(requestPath)) + .willReturn(WireMock.aResponse().withStatus(200).withBody(content))); + Path destination = tempDir.resolve("download"); + + s3.getObject(r -> r.bucket(BUCKET).key(KEY), destination).join(); + + assertThat(Files.readAllBytes(destination)).isEqualTo(content); + } + + @Test + public void getObjectWithReplaceTransformer_existingFile_replacesFile() throws Exception { + String requestPath = String.format("/%s/%s", BUCKET, KEY); + byte[] content = "hello".getBytes(StandardCharsets.UTF_8); + stubFor(head(urlPathEqualTo(requestPath)) + .willReturn(WireMock.aResponse() + .withStatus(200) + .withHeader("ETag", "etag") + .withHeader("Content-Length", Integer.toString(content.length)))); + stubFor(get(urlPathEqualTo(requestPath)) + .willReturn(WireMock.aResponse().withStatus(200).withBody(content))); + Path destination = tempDir.resolve("download"); + Files.write(destination, "original".getBytes(StandardCharsets.UTF_8)); + + s3.getObject(r -> r.bucket(BUCKET).key(KEY), + AsyncResponseTransformer.toFile( + destination, FileTransformerConfiguration.defaultCreateOrReplaceExisting())).join(); + + assertThat(Files.readAllBytes(destination)).isEqualTo(content); + } + + @Test + public void getObjectToPath_existingFile_failsAsynchronouslyAndPreservesFile() throws Exception { + String requestPath = String.format("/%s/%s", BUCKET, KEY); + byte[] objectContent = "hello".getBytes(StandardCharsets.UTF_8); + stubFor(head(urlPathEqualTo(requestPath)) + .willReturn(WireMock.aResponse() + .withStatus(200) + .withHeader("ETag", "etag") + .withHeader("Content-Length", Integer.toString(objectContent.length)))); + stubFor(get(urlPathEqualTo(requestPath)) + .willReturn(WireMock.aResponse().withStatus(200).withBody(objectContent))); + Path destination = tempDir.resolve("download"); + byte[] originalContent = "original".getBytes(StandardCharsets.UTF_8); + Files.write(destination, originalContent); + + CompletableFuture future = + assertDoesNotThrow(() -> s3.getObject(r -> r.bucket(BUCKET).key(KEY), destination)); + + assertThatThrownBy(future::join) + .hasRootCauseInstanceOf(FileAlreadyExistsException.class) + .hasRootCauseMessage(destination.toString()); + assertThat(Files.readAllBytes(destination)).isEqualTo(originalContent); + verify(exactly(0), anyRequestedFor(anyUrl())); + } + + @Test + public void getObjectToPath_failedDownload_deletesFile() { + String requestPath = String.format("/%s/%s", BUCKET, KEY); + stubFor(head(urlPathEqualTo(requestPath)).willReturn(WireMock.aResponse().withStatus(404))); + Path destination = tempDir.resolve("download"); + + assertThatThrownBy(s3.getObject(r -> r.bucket(BUCKET).key(KEY), destination)::join) + .hasCauseInstanceOf(S3Exception.class); + assertThat(destination).doesNotExist(); + } + @Test public void getObject_headObjectThrows_operationThrows() { String path = String.format("/%s/%s", BUCKET, KEY); diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClientTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClientTest.java index ca34f9c045ca..06d7412be4f1 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClientTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/DefaultS3CrtAsyncClientTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.nio.file.Path; import java.util.concurrent.atomic.AtomicReference; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -33,6 +34,7 @@ import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.interceptor.SdkInternalExecutionAttribute; +import software.amazon.awssdk.crt.s3.S3MetaRequestOptions.ResponseFileOption; import software.amazon.awssdk.http.SdkHttpExecutionAttributes; import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; import software.amazon.awssdk.identity.spi.IdentityProvider; @@ -41,6 +43,7 @@ import software.amazon.awssdk.services.s3.S3AsyncClient; import software.amazon.awssdk.services.s3.endpoints.S3ClientContextParams; import software.amazon.awssdk.services.s3.internal.crossregion.S3CrossRegionAsyncClient; +import software.amazon.awssdk.testutils.RandomTempFile; import software.amazon.awssdk.utils.AttributeMap; import software.amazon.awssdk.utils.MapUtils; @@ -90,6 +93,40 @@ public void beforeExecution(Context.BeforeExecution context, ExecutionAttributes } } + @Test + void getObjectWithPath_shouldConfigureResponseFile() { + AtomicReference capturedAttributes = new AtomicReference<>(); + ExecutionInterceptor captor = new ExecutionInterceptor() { + @Override + public void beforeTransmission(Context.BeforeTransmission context, ExecutionAttributes executionAttributes) { + capturedAttributes.set( + executionAttributes.getAttribute(SdkInternalExecutionAttribute.SDK_HTTP_EXECUTION_ATTRIBUTES)); + throw new RuntimeException("STOP"); + } + }; + + DefaultS3CrtAsyncClient.DefaultS3CrtClientBuilder builder = + (DefaultS3CrtAsyncClient.DefaultS3CrtClientBuilder) S3CrtAsyncClient.builder(); + builder.addExecutionInterceptor(captor); + Path destination = RandomTempFile.randomUncreatedFile().toPath(); + + try (S3AsyncClient client = builder.region(Region.US_EAST_1) + .credentialsProvider(StaticCredentialsProvider.create( + AwsBasicCredentials.create("key", "secret"))) + .build()) { + assertThatThrownBy(() -> client.getObject(r -> r.bucket("bucket").key("key"), destination).join()) + .hasMessageContaining("STOP"); + } + + SdkHttpExecutionAttributes attributes = capturedAttributes.get(); + assertThat(attributes.getAttribute(S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_PATH)) + .isEqualTo(destination); + assertThat(attributes.getAttribute(S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_OPTION)) + .isEqualTo(ResponseFileOption.CREATE_NEW); + assertThat(attributes.getAttribute(S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_DELETE_ON_FAILURE)) + .isTrue(); + } + @ParameterizedTest @ValueSource(longs = {0, -1L}) void invalidConfig_shouldThrowException(long value) { diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java index 40702c70ef65..c07e566a6ed3 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClientTest.java @@ -17,6 +17,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -26,6 +27,7 @@ import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.OPERATION_NAME; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.REQUEST_CHECKSUM_CALCULATION; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_CHECKSUM_VALIDATION; +import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_DELETE_ON_FAILURE; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_OPTION; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.RESPONSE_FILE_PATH; import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.CRT_CREDENTIALS_PROVIDER_ADAPTER; @@ -34,6 +36,7 @@ import static software.amazon.awssdk.services.s3.internal.crt.S3InternalSdkHttpExecutionAttribute.USE_S3_EXPRESS_AUTH; import java.net.URI; +import java.nio.file.FileAlreadyExistsException; import java.nio.file.Path; import java.time.Duration; import java.util.HashMap; @@ -52,6 +55,7 @@ import software.amazon.awssdk.core.checksums.RequestChecksumCalculation; import software.amazon.awssdk.core.checksums.ResponseChecksumValidation; import software.amazon.awssdk.core.interceptor.trait.HttpChecksum; +import software.amazon.awssdk.crt.CrtRuntimeException; import software.amazon.awssdk.crt.auth.credentials.CredentialsProvider; import software.amazon.awssdk.crt.auth.signing.AwsSigningConfig; import software.amazon.awssdk.crt.http.HttpProxyEnvironmentVariableSetting; @@ -618,11 +622,43 @@ public void responseFilePathAndOption_shouldPassToCrt() { .putHttpExecutionAttribute(OPERATION_NAME, "GetObject") .putHttpExecutionAttribute(RESPONSE_FILE_PATH, path) .putHttpExecutionAttribute(RESPONSE_FILE_OPTION, S3MetaRequestOptions.ResponseFileOption.CREATE_OR_APPEND) + .putHttpExecutionAttribute(RESPONSE_FILE_DELETE_ON_FAILURE, true) .build(); S3MetaRequestOptions actual = makeRequest(asyncExecuteRequest); assertThat(actual.getResponseFilePath()).isEqualTo(path); assertThat(actual.getResponseFileOption()).isEqualTo(S3MetaRequestOptions.ResponseFileOption.CREATE_OR_APPEND); + assertThat(actual.getResponseFileDeleteOnFailure()).isTrue(); + } + + @Test + public void responseFileDeleteOnFailureNotSpecified_shouldUseCrtDefault() { + Path path = RandomTempFile.randomUncreatedFile().toPath(); + AsyncExecuteRequest asyncExecuteRequest = getExecuteRequestBuilder() + .putHttpExecutionAttribute(OPERATION_NAME, "GetObject") + .putHttpExecutionAttribute(RESPONSE_FILE_PATH, path) + .build(); + + assertThat(makeRequest(asyncExecuteRequest).getResponseFileDeleteOnFailure()).isFalse(); + } + + @Test + public void responseFileAlreadyExists_shouldFailFutureWithFileAlreadyExistsException() { + Path path = RandomTempFile.randomUncreatedFile().toPath(); + CrtRuntimeException crtException = + new CrtRuntimeException("aws_last_error: AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS(14367), file exists"); + when(s3Client.makeMetaRequest(any(S3MetaRequestOptions.class))).thenThrow(crtException); + AsyncExecuteRequest asyncExecuteRequest = getExecuteRequestBuilder() + .putHttpExecutionAttribute(OPERATION_NAME, "GetObject") + .putHttpExecutionAttribute(RESPONSE_FILE_PATH, path) + .putHttpExecutionAttribute(RESPONSE_FILE_OPTION, S3MetaRequestOptions.ResponseFileOption.CREATE_NEW) + .build(); + + CompletableFuture future = assertDoesNotThrow(() -> asyncHttpClient.execute(asyncExecuteRequest)); + + assertThatThrownBy(future::join) + .hasRootCauseInstanceOf(FileAlreadyExistsException.class) + .hasRootCauseMessage(path.toString()); } @Test From a317e9b3a2efd0bec7a8cba750fa6a7d618d6d8d Mon Sep 17 00:00:00 2001 From: David Ho Date: Fri, 4 Sep 2026 16:11:57 -0700 Subject: [PATCH 2/4] Inline error message --- .../awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java index 139b5cf70168..0ccc0e0f60cc 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/S3CrtAsyncHttpClient.java @@ -73,7 +73,6 @@ */ @SdkInternalApi public final class S3CrtAsyncHttpClient implements SdkAsyncHttpClient { - private static final String CRT_RESPONSE_FILE_ALREADY_EXISTS = "AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS"; private final S3Client crtS3Client; @@ -237,7 +236,7 @@ public CompletableFuture execute(AsyncExecuteRequest asyncRequest) { private static boolean isResponseFileAlreadyExistsError(Throwable throwable, Path responseFilePath) { return responseFilePath != null && throwable instanceof CrtRuntimeException - && CRT_RESPONSE_FILE_ALREADY_EXISTS.equals(((CrtRuntimeException) throwable).errorName); + && "AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS".equals(((CrtRuntimeException) throwable).errorName); } private AwsSigningConfig awsSigningConfig(Region signingRegion, SdkHttpExecutionAttributes httpExecutionAttributes) { From adac8f8ff6118713792acd53d9b1ee20789361b4 Mon Sep 17 00:00:00 2001 From: David Ho Date: Fri, 4 Sep 2026 16:40:52 -0700 Subject: [PATCH 3/4] Update chnagelog and refactor tests --- .../next-release/bugfix-AWSS3-9d4c2a1.json | 2 +- .../services/s3/crt/CrtDownloadErrorTest.java | 40 +++++++------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/.changes/next-release/bugfix-AWSS3-9d4c2a1.json b/.changes/next-release/bugfix-AWSS3-9d4c2a1.json index a685bae55410..3a961c4deb75 100644 --- a/.changes/next-release/bugfix-AWSS3-9d4c2a1.json +++ b/.changes/next-release/bugfix-AWSS3-9d4c2a1.json @@ -2,5 +2,5 @@ "type": "bugfix", "category": "Amazon S3", "contributor": "", - "description": "Prevent the CRT-based S3 async client from replacing a pre-existing destination or leaving behind a file when `getObject(request, Path)` fails or is cancelled. Existing destinations now surface `FileAlreadyExistsException`; callers that require replacement can use `AsyncResponseTransformer.toFile` with `FileTransformerConfiguration.defaultCreateOrReplaceExisting()`." + "description": "Prevent the CRT-based S3 async client from replacing a pre-existing destination when using `getObject(request, Path)`. Failed or cancelled downloads now delete files they create. Existing destinations surface `FileAlreadyExistsException`; callers that require replacement can use `AsyncResponseTransformer.toFile` with `FileTransformerConfiguration.defaultCreateOrReplaceExisting()`." } diff --git a/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java b/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java index 5e497abef510..2a345298a010 100644 --- a/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java +++ b/services/s3/src/test/java/software/amazon/awssdk/services/s3/crt/CrtDownloadErrorTest.java @@ -136,15 +136,8 @@ public void getObject_headObjectOk_getObjectOk_operationSucceeds() { @Test public void getObjectToPath_success_writesFile() throws Exception { - String requestPath = String.format("/%s/%s", BUCKET, KEY); byte[] content = "hello".getBytes(StandardCharsets.UTF_8); - stubFor(head(urlPathEqualTo(requestPath)) - .willReturn(WireMock.aResponse() - .withStatus(200) - .withHeader("ETag", "etag") - .withHeader("Content-Length", Integer.toString(content.length)))); - stubFor(get(urlPathEqualTo(requestPath)) - .willReturn(WireMock.aResponse().withStatus(200).withBody(content))); + stubSuccessfulDownload(content); Path destination = tempDir.resolve("download"); s3.getObject(r -> r.bucket(BUCKET).key(KEY), destination).join(); @@ -154,15 +147,8 @@ public void getObjectToPath_success_writesFile() throws Exception { @Test public void getObjectWithReplaceTransformer_existingFile_replacesFile() throws Exception { - String requestPath = String.format("/%s/%s", BUCKET, KEY); byte[] content = "hello".getBytes(StandardCharsets.UTF_8); - stubFor(head(urlPathEqualTo(requestPath)) - .willReturn(WireMock.aResponse() - .withStatus(200) - .withHeader("ETag", "etag") - .withHeader("Content-Length", Integer.toString(content.length)))); - stubFor(get(urlPathEqualTo(requestPath)) - .willReturn(WireMock.aResponse().withStatus(200).withBody(content))); + stubSuccessfulDownload(content); Path destination = tempDir.resolve("download"); Files.write(destination, "original".getBytes(StandardCharsets.UTF_8)); @@ -175,15 +161,8 @@ public void getObjectWithReplaceTransformer_existingFile_replacesFile() throws E @Test public void getObjectToPath_existingFile_failsAsynchronouslyAndPreservesFile() throws Exception { - String requestPath = String.format("/%s/%s", BUCKET, KEY); - byte[] objectContent = "hello".getBytes(StandardCharsets.UTF_8); - stubFor(head(urlPathEqualTo(requestPath)) - .willReturn(WireMock.aResponse() - .withStatus(200) - .withHeader("ETag", "etag") - .withHeader("Content-Length", Integer.toString(objectContent.length)))); - stubFor(get(urlPathEqualTo(requestPath)) - .willReturn(WireMock.aResponse().withStatus(200).withBody(objectContent))); + byte[] content = "hello".getBytes(StandardCharsets.UTF_8); + stubSuccessfulDownload(content); Path destination = tempDir.resolve("download"); byte[] originalContent = "original".getBytes(StandardCharsets.UTF_8); Files.write(destination, originalContent); @@ -221,4 +200,15 @@ public void getObject_headObjectThrows_operationThrows() { .hasCauseInstanceOf(S3Exception.class) .hasMessageContaining("Status Code: 403"); } + + private void stubSuccessfulDownload(byte[] content) { + String path = String.format("/%s/%s", BUCKET, KEY); + stubFor(head(urlPathEqualTo(path)) + .willReturn(WireMock.aResponse() + .withStatus(200) + .withHeader("ETag", "etag") + .withHeader("Content-Length", Integer.toString(content.length)))); + stubFor(get(urlPathEqualTo(path)) + .willReturn(WireMock.aResponse().withStatus(200).withBody(content))); + } } From 8d6d36e98d8d71d27869d267c40569401a46ff16 Mon Sep 17 00:00:00 2001 From: David Ho Date: Wed, 9 Sep 2026 14:09:42 -0700 Subject: [PATCH 4/4] Update changelog --- .changes/next-release/bugfix-AWSS3-9d4c2a1.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/next-release/bugfix-AWSS3-9d4c2a1.json b/.changes/next-release/bugfix-AWSS3-9d4c2a1.json index 3a961c4deb75..fb4af5705d98 100644 --- a/.changes/next-release/bugfix-AWSS3-9d4c2a1.json +++ b/.changes/next-release/bugfix-AWSS3-9d4c2a1.json @@ -2,5 +2,5 @@ "type": "bugfix", "category": "Amazon S3", "contributor": "", - "description": "Prevent the CRT-based S3 async client from replacing a pre-existing destination when using `getObject(request, Path)`. Failed or cancelled downloads now delete files they create. Existing destinations surface `FileAlreadyExistsException`; callers that require replacement can use `AsyncResponseTransformer.toFile` with `FileTransformerConfiguration.defaultCreateOrReplaceExisting()`." + "description": "Prevent the CRT-based S3 async client from replacing a pre-existing destination when using `getObject(request, Path)`. Failed or cancelled downloads now delete files they create. Existing destinations surface `FileAlreadyExistsException`; callers that require replacement can use `AsyncResponseTransformer.toFile` with `FileTransformerConfiguration.defaultCreateOrReplaceExisting()`. This fixes an inconsistency introduced in 2.32.11." }