Skip to content

fix(sdk-core): Validate download file before sending request - #7372

Open
joviegas wants to merge 1 commit into
masterfrom
joviegas/get-object-file-validate
Open

fix(sdk-core): Validate download file before sending request#7372
joviegas wants to merge 1 commit into
masterfrom
joviegas/get-object-file-validate

Conversation

@joviegas

Copy link
Copy Markdown
Contributor

Motivation

getObject(request, path) and the other streaming-to-file operations check the destination
file only after the response starts arriving. So when the file can never be written, the SDK
still sends the request and downloads part of the object before failing:

  • default CREATE_NEW and the file already exists -> FileAlreadyExistsException
  • WRITE_TO_POSITION and the file is missing -> NoSuchFileException

Checking the file needs no network call, so this should fail before any request is sent. The
current behavior wastes a request and bandwidth on a download that cannot succeed.

Affects sync and async clients, and every service with a streaming output operation, not just
S3.

Modification

Validate the destination before sending the request, on both clients. The file open at write
time remains the real check; the new check only fails early. If the file is created or deleted
between the check and the open, the open still fails exactly as it does today.

  • Async (FileAsyncResponseTransformer.prepare()): validate the destination first and
    throw synchronously. Completing the returned future exceptionally is not enough, because the
    request pipeline sends the request regardless of that future's state. The client handler
    converts the throw into a failed future, so getObject(...) still returns a future as before.
  • Async multipart path (FileAsyncResponseTransformerPublisher): the delegate prepare()
    is now called inside a try/catch that routes the error to handleError, so the part's future
    fails instead of hanging.
  • Sync: ResponseTransformer has no hook that runs before the request, so one is added
    internally:
    • new @SdkInternalApi interface ValidatingResponseTransformer (extends
      ResponseTransformer) with a single void validate(). The existing anonymous class in
      toFile(Path) implements it in place, so no code moves and no public API is added.
    • BaseSyncClientHandler.execute(...) calls validate() before sending the request, guarded
      by instanceof.
  • The failure is an SdkClientException caused by the original FileAlreadyExistsException or
    NoSuchFileException.
  • Javadoc on AsyncResponseTransformer.toFile and ResponseTransformer.toFile now states that
    the destination is validated before the request is sent.

Testing

  • junits

Backward compatibility (verified)

Ran getObject to an existing file against the code before and after this change, with a mock
HTTP client. Same exception type and same root cause on both clients.

Async - returned future completes exceptionally, before and after:

Before After
Top exception SdkClientException SdkClientException
Root cause FileAlreadyExistsException FileAlreadyExistsException
Requests sent 1 0

Sync - getObject throws, before and after:

Before After
Top exception SdkClientException SdkClientException
Cause chain -> NonRetryableException -> IOException -> FileAlreadyExistsException -> FileAlreadyExistsException
Requests sent 1 0

What changed for callers:

  • Exception message text changed on both clients.
  • Sync only: the NonRetryableException and IOException wrapper layers are gone, so
    FileAlreadyExistsException is now the direct cause. Catching SdkClientException or
    searching the cause chain works as before.
  • The failure now happens before any request is sent. Sync still throws from getObject; async
    still returns a failed future.

Testing

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
  • My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

  • I confirm that this pull request can be released under the Apache 2 license

@joviegas
joviegas requested a review from a team as a code owner September 13, 2026 02:11
@joviegas
joviegas force-pushed the joviegas/get-object-file-validate branch from d88d6eb to 9aa0017 Compare September 14, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant