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-AWSSDKforJavav2-7d3f9c1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"type": "bugfix",
"category": "AWS SDK for Java v2",
"contributor": "",
"description": "`getObject(request, path)` and other streaming-to-file downloads now validate the destination file client-side, on both sync and async clients, before sending a request. A destination that can never be written - an existing file with the default `CREATE_NEW`, or a missing file with `WRITE_TO_POSITION` - now fails immediately with an `SdkClientException` (caused by `FileAlreadyExistsException` or `NoSuchFileException`) instead of after a wasted request."
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,10 @@ default String name() {

/**
* Creates an {@link AsyncResponseTransformer} that writes all the content to the given file. In the event of an error, the
* SDK will attempt to delete the file (whatever has been written to it so far). If the file already exists, an exception will
* be thrown.
* SDK will attempt to delete the file (whatever has been written to it so far). If the file already exists, the
* operation's returned future completes exceptionally with an
* {@link software.amazon.awssdk.core.exception.SdkClientException} caused by a
* {@link java.nio.file.FileAlreadyExistsException}, and no request is sent.
*
* <p>The file's parent directories must already exist. The SDK will not auto-create directories, and a
* {@link java.nio.file.NoSuchFileException} will be thrown if they are missing.
Expand All @@ -205,6 +207,13 @@ static <ResponseT> AsyncResponseTransformer<ResponseT, ResponseT> toFile(Path pa
* Creates an {@link AsyncResponseTransformer} that writes all the content to the given file with the specified
* {@link FileTransformerConfiguration}.
*
* <p>Before any request is sent, the destination is checked against the configured
* {@link FileTransformerConfiguration.FileWriteOption}: {@code CREATE_NEW} requires that the file does not yet
* exist, and {@code WRITE_TO_POSITION} requires that it does. If the check fails, the operation's returned future
* completes exceptionally with an {@link software.amazon.awssdk.core.exception.SdkClientException} - caused by a
* {@link java.nio.file.FileAlreadyExistsException} or {@link java.nio.file.NoSuchFileException} - and no request is
* sent.
*
* <p>The file's parent directories must already exist. The SDK will not auto-create directories, and a
* {@link java.nio.file.NoSuchFileException} will be thrown if they are missing.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package software.amazon.awssdk.core.internal.async;

import static software.amazon.awssdk.core.FileTransformerConfiguration.FileWriteOption.CREATE_NEW;
import static software.amazon.awssdk.core.FileTransformerConfiguration.FileWriteOption.CREATE_OR_APPEND_TO_EXISTING;
import static software.amazon.awssdk.core.FileTransformerConfiguration.FileWriteOption.WRITE_TO_POSITION;
import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely;
Expand All @@ -24,6 +25,7 @@
import java.nio.ByteBuffer;
import java.nio.channels.AsynchronousFileChannel;
import java.nio.channels.CompletionHandler;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.OpenOption;
Expand Down Expand Up @@ -139,6 +141,9 @@ private AsynchronousFileChannel createChannel(Path path) throws IOException {

@Override
public CompletableFuture<ResponseT> prepare() {
// Throw synchronously, don't fail the future: the pipeline dispatches regardless of the future's state, so a
// failed future would still send the request.
validateDestination();
fileChannel = null;
cf = new CompletableFuture<>();
cf.whenComplete((r, t) -> {
Expand All @@ -151,6 +156,23 @@ public CompletableFuture<ResponseT> prepare() {
return cf.thenApply(ignored -> response);
}

/**
* Rejects a destination the {@link #createChannel(Path)} open could never accept, before it costs a request; keep
* this in sync with that open's {@link OpenOption}s. The open stays authoritative, so a file that appears or
* disappears after this check still fails there. ({@code notExists} is not {@code !exists}: both are false when the
* filesystem is uncertain, which defers the verdict to the open.)
*/
private void validateDestination() {
if (configuration.fileWriteOption() == CREATE_NEW && Files.exists(path)) {
throw SdkClientException.create("Cannot write to the existing file " + path + " with file write option "
+ CREATE_NEW, new FileAlreadyExistsException(path.toString()));
}
if (configuration.fileWriteOption() == WRITE_TO_POSITION && Files.notExists(path)) {
throw SdkClientException.create("Cannot write to the missing file " + path + " with file write option "
+ WRITE_TO_POSITION, new NoSuchFileException(path.toString()));
}
}

@Override
public void onResponse(ResponseT response) {
this.response = response;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,15 @@ public void onResponse(T response) {
}

this.delegate = getDelegateTransformer(contentRangePair.get().left());
CompletableFuture<T> delegateFuture = delegate.prepare();
CompletableFuture<T> delegateFuture;
try {
delegateFuture = delegate.prepare();
} catch (RuntimeException e) {
// prepare() validates the destination and can throw. Complete the part future here instead of letting
// the throw escape this callback, which would leave the part hanging.
handleError(e);
return;
}
CompletableFutureUtils.forwardResultTo(delegateFuture, future);
CompletableFutureUtils.forwardExceptionTo(future, delegateFuture);
transformerCount.incrementAndGet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import software.amazon.awssdk.core.internal.http.AmazonSyncHttpClient;
import software.amazon.awssdk.core.internal.http.CombinedResponseHandler;
import software.amazon.awssdk.core.internal.http.InterruptMonitor;
import software.amazon.awssdk.core.internal.sync.ValidatingResponseTransformer;
import software.amazon.awssdk.core.metrics.CoreMetric;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.core.sync.ResponseTransformer;
Expand All @@ -60,6 +61,11 @@ public <InputT extends SdkRequest, OutputT extends SdkResponse, ReturnT> ReturnT
ResponseTransformer<OutputT, ReturnT> responseTransformer) {

return measureApiCall(executionParams, () -> {
// Let the transformer reject the call before it costs a request. Async does this in prepare().
if (responseTransformer instanceof ValidatingResponseTransformer) {
((ValidatingResponseTransformer<?>) responseTransformer).validate();
}

// Running beforeExecution interceptors and modifyRequest interceptors.
ExecutionContext executionContext = invokeInterceptorsAndCreateExecutionContext(executionParams);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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.core.internal.sync;

import software.amazon.awssdk.annotations.SdkInternalApi;
import software.amazon.awssdk.core.sync.ResponseTransformer;

/**
* A {@link ResponseTransformer} that can reject a call before the SDK dispatches the request. This is the synchronous
* counterpart to {@link software.amazon.awssdk.core.async.AsyncResponseTransformer#prepare()}, kept internal so no
* public API is added.
*
* @param <ResponseT> Type of unmarshalled response POJO.
*/
@SdkInternalApi
public interface ValidatingResponseTransformer<ResponseT> extends ResponseTransformer<ResponseT, ResponseT> {

/**
* Called by the SDK immediately before the request is dispatched. Throwing fails the call without sending a
* request.
*/
void validate();
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.core.internal.http.InterruptMonitor;
import software.amazon.awssdk.core.internal.sync.ValidatingResponseTransformer;
import software.amazon.awssdk.core.retry.RetryPolicy;
import software.amazon.awssdk.http.AbortableInputStream;
import software.amazon.awssdk.utils.IoUtils;
Expand Down Expand Up @@ -109,7 +110,8 @@ default String name() {

/**
* Creates a response transformer that writes all response content to the specified file. If the file already exists
* then a {@link FileAlreadyExistsException} will be thrown.
* then a {@link FileAlreadyExistsException} will be thrown, as the cause of an {@link SdkClientException}, without
* sending a request.
*
* <p>The file's parent directories must already exist. The SDK will not auto-create directories, and a
* {@link NoSuchFileException} will be thrown if they are missing.
Expand All @@ -119,7 +121,17 @@ default String name() {
* @return ResponseTransformer instance.
*/
static <ResponseT> ResponseTransformer<ResponseT, ResponseT> toFile(Path path) {
return new ResponseTransformer<ResponseT, ResponseT>() {
return new ValidatingResponseTransformer<ResponseT>() {
@Override
public void validate() {
// Reject a destination the Files.copy below could never write, before the request is sent. That copy
// uses CREATE_NEW semantics and stays authoritative, so a file appearing later still fails there.
if (Files.exists(path)) {
throw SdkClientException.create("Cannot write to the existing file " + path,
new FileAlreadyExistsException(path.toString()));
}
}

@Override
public ResponseT transform(ResponseT response, AbortableInputStream inputStream) throws Exception {
try {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/*
* 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.core.client.handler;

import static org.assertj.core.api.Assertions.assertThat;
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.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;

import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import software.amazon.awssdk.core.SdkRequest;
import software.amazon.awssdk.core.SdkResponse;
import software.amazon.awssdk.core.async.AsyncResponseTransformer;
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
import software.amazon.awssdk.core.client.config.SdkClientOption;
import software.amazon.awssdk.core.exception.SdkClientException;
import software.amazon.awssdk.core.exception.SdkServiceException;
import software.amazon.awssdk.core.http.HttpResponseHandler;
import software.amazon.awssdk.core.protocol.VoidSdkResponse;
import software.amazon.awssdk.core.retry.RetryPolicy;
import software.amazon.awssdk.core.runtime.transform.Marshaller;
import software.amazon.awssdk.core.sync.ResponseTransformer;
import software.amazon.awssdk.http.AbortableInputStream;
import software.amazon.awssdk.http.ExecutableHttpRequest;
import software.amazon.awssdk.http.HttpExecuteResponse;
import software.amazon.awssdk.http.SdkHttpClient;
import software.amazon.awssdk.http.SdkHttpResponse;
import software.amazon.awssdk.http.async.SdkAsyncHttpClient;
import software.amazon.awssdk.retries.DefaultRetryStrategy;
import utils.HttpTestUtils;
import utils.ValidSdkObjects;

/**
* Verifies that downloading to a destination that violates the file write option precondition fails client-side, before
* any request is dispatched, on both the sync and async client handlers.
*/
class FileTransformerFailFastTest {

@TempDir
Path tempDir;

private final SdkRequest request = mock(SdkRequest.class);
private final Marshaller<SdkRequest> marshaller = mock(Marshaller.class);
private final HttpResponseHandler<SdkResponse> responseHandler = mock(HttpResponseHandler.class);
private final HttpResponseHandler<SdkServiceException> errorResponseHandler = mock(HttpResponseHandler.class);

/**
* Mirrors the generated {@code getObject(request, path)} overload, which evaluates
* {@code ResponseTransformer.toFile(path)} inside the call, so the failure surfaces from the call and no request is
* dispatched.
*/
@Test
void syncExecute_toFileDestinationExists_failsFastWithoutDispatchingRequest() throws Exception {
SdkHttpClient httpClient = mock(SdkHttpClient.class);
SdkSyncClientHandler handler = new SdkSyncClientHandler(syncClientConfiguration(httpClient));

Path existingFile = Files.createFile(tempDir.resolve("sync-existing-dest.bin"));

assertThatThrownBy(() -> handler.execute(clientExecutionParams(), ResponseTransformer.toFile(existingFile)))
.isInstanceOf(SdkClientException.class)
.hasRootCauseInstanceOf(FileAlreadyExistsException.class);

verifyNoInteractions(httpClient);
}

/**
* Guards against over-eager rejection: a destination that does not exist yet must still be requested and written.
*/
@Test
void syncExecute_toFileDestinationDoesNotExist_dispatchesRequestAndWritesFile() throws Exception {
SdkHttpClient httpClient = mock(SdkHttpClient.class);
ExecutableHttpRequest httpClientCall = mock(ExecutableHttpRequest.class);
when(httpClient.prepareRequest(any())).thenReturn(httpClientCall);
when(httpClientCall.call()).thenReturn(
HttpExecuteResponse.builder()
.response(SdkHttpResponse.builder().statusCode(200).build())
.responseBody(AbortableInputStream.create(
new ByteArrayInputStream("hello".getBytes(StandardCharsets.UTF_8))))
.build());
when(marshaller.marshall(request)).thenReturn(ValidSdkObjects.sdkHttpFullRequest().build());
when(responseHandler.handle(any(), any())).thenReturn(VoidSdkResponse.builder().build());

SdkSyncClientHandler handler = new SdkSyncClientHandler(syncClientConfiguration(httpClient));
Path freshPath = tempDir.resolve("fresh-dest.bin");

handler.execute(clientExecutionParams(), ResponseTransformer.toFile(freshPath));

verify(httpClient).prepareRequest(any());
assertThat(freshPath).hasContent("hello");
}

@Test
void asyncExecute_toFileDestinationExists_failsFastWithoutDispatchingRequest() throws Exception {
SdkAsyncHttpClient httpClient = mock(SdkAsyncHttpClient.class);
SdkAsyncClientHandler handler = new SdkAsyncClientHandler(asyncClientConfiguration(httpClient));

Path existingFile = Files.createFile(tempDir.resolve("async-existing-dest.bin"));

CompletableFuture<SdkResponse> responseFuture =
handler.execute(clientExecutionParams(), AsyncResponseTransformer.toFile(existingFile));

assertThatThrownBy(() -> responseFuture.get(5, TimeUnit.SECONDS))
.hasRootCauseInstanceOf(FileAlreadyExistsException.class);
verify(httpClient, never()).execute(any());
}

private ClientExecutionParams<SdkRequest, SdkResponse> clientExecutionParams() {
when(request.overrideConfiguration()).thenReturn(Optional.empty());
return new ClientExecutionParams<SdkRequest, SdkResponse>()
.withInput(request)
.withMarshaller(marshaller)
.withResponseHandler(responseHandler)
.withErrorResponseHandler(errorResponseHandler);
}

private SdkClientConfiguration syncClientConfiguration(SdkHttpClient httpClient) {
return HttpTestUtils.testClientConfiguration().toBuilder()
.option(SdkClientOption.SYNC_HTTP_CLIENT, httpClient)
.option(SdkClientOption.RETRY_STRATEGY, DefaultRetryStrategy.doNotRetry())
.build();
}

private SdkClientConfiguration asyncClientConfiguration(SdkAsyncHttpClient httpClient) {
ScheduledExecutorService scheduledExecutor = mock(ScheduledExecutorService.class);
return HttpTestUtils.testClientConfiguration().toBuilder()
.option(SdkClientOption.ASYNC_HTTP_CLIENT, httpClient)
.option(SdkClientOption.RETRY_POLICY, RetryPolicy.none())
.option(SdkClientOption.RETRY_STRATEGY, DefaultRetryStrategy.doNotRetry())
.option(SdkClientOption.SCHEDULED_EXECUTOR_SERVICE, scheduledExecutor)
.build();
}
}
Loading
Loading