fix(sdk-core): Validate download file before sending request - #7372
Open
joviegas wants to merge 1 commit into
Open
fix(sdk-core): Validate download file before sending request#7372joviegas wants to merge 1 commit into
joviegas wants to merge 1 commit into
Conversation
joviegas
force-pushed
the
joviegas/get-object-file-validate
branch
from
September 14, 2026 00:36
d88d6eb to
9aa0017
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
getObject(request, path)and the other streaming-to-file operations check the destinationfile 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:
CREATE_NEWand the file already exists ->FileAlreadyExistsExceptionWRITE_TO_POSITIONand the file is missing ->NoSuchFileExceptionChecking 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.
FileAsyncResponseTransformer.prepare()): validate the destination first andthrow 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.FileAsyncResponseTransformerPublisher): the delegateprepare()is now called inside a try/catch that routes the error to
handleError, so the part's futurefails instead of hanging.
ResponseTransformerhas no hook that runs before the request, so one is addedinternally:
@SdkInternalApiinterfaceValidatingResponseTransformer(extendsResponseTransformer) with a singlevoid validate(). The existing anonymous class intoFile(Path)implements it in place, so no code moves and no public API is added.BaseSyncClientHandler.execute(...)callsvalidate()before sending the request, guardedby
instanceof.SdkClientExceptioncaused by the originalFileAlreadyExistsExceptionorNoSuchFileException.AsyncResponseTransformer.toFileandResponseTransformer.toFilenow states thatthe destination is validated before the request is sent.
Testing
Backward compatibility (verified)
Ran
getObjectto an existing file against the code before and after this change, with a mockHTTP client. Same exception type and same root cause on both clients.
Async - returned future completes exceptionally, before and after:
SdkClientExceptionSdkClientExceptionFileAlreadyExistsExceptionFileAlreadyExistsExceptionSync -
getObjectthrows, before and after:SdkClientExceptionSdkClientExceptionNonRetryableException->IOException->FileAlreadyExistsExceptionFileAlreadyExistsExceptionWhat changed for callers:
NonRetryableExceptionandIOExceptionwrapper layers are gone, soFileAlreadyExistsExceptionis now the direct cause. CatchingSdkClientExceptionorsearching the cause chain works as before.
getObject; asyncstill returns a failed future.
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License