Skip to content

Fix overflow and precision issues in total parts calculation in presigned URL multipart download#7026

Open
jencymaryjoseph wants to merge 1 commit into
feature/master/pre-signed-url-getobjectfrom
jencyjos/pre-signed-url-getobject/fix-calculate-total-parts-overflow
Open

Fix overflow and precision issues in total parts calculation in presigned URL multipart download#7026
jencymaryjoseph wants to merge 1 commit into
feature/master/pre-signed-url-getobjectfrom
jencyjos/pre-signed-url-getobject/fix-calculate-total-parts-overflow

Conversation

@jencymaryjoseph

Copy link
Copy Markdown
Contributor

Motivation and Context

The S3 presigned URL multipart download feature uses range-based GETs to download large objects in chunks. The number of parts is calculated client-side from the total object size and the configured part size.
Unlike the standard multipart download (MultipartDownloaderSubscriber), which reads totalParts from the x-amz-mp-parts-count response header (always ≤ 10,000 since S3 caps multipart uploads at 10,000 parts), the presigned URL multipart download calculates totalParts client-side from contentLength / partSize. This calculation has no S3-enforced upper bound — the only constraint is the configured minimumPartSizeInBytes.

The current implementation has two issues:
Issue 1: Floating-point precision loss Casting long → double loses precision for values > 2^53. With small partSize, the division loses precision and the result can be off.
Issue 2: Silent integer saturation When contentLength / partSize > Integer.MAX_VALUE, the (int) cast on the Math.ceil result silently saturates to Integer.MAX_VALUE rather than failing. The downloader uses this incorrect totalParts, the completion gate (completedParts == totalParts) fires before all data is downloaded, and the customer receives a truncated download reported as success

When the bug is reachable: The SDK does not enforce a minimum on MultipartConfiguration.minimumPartSizeInBytes(), so a customer can set it as low as 1L. With sub-KB part sizes against multi-GB objects, contentLength / partSize exceeds Integer.MAX_VALUE and the bug manifests.

Modifications

Replaced the floating-point ceiling division with exact integer arithmetic and
Changed calculateTotalParts return type from int to long
Changed totalParts field from Integer to Long in both subscribers

Testing

  • Unit tests (MultipartDownloadUtilsTest):
    Existing cases continue to pass (last-part smaller, exact boundary, contentLength == partSize, contentLength == 0, contentLength < partSize).
    Added 5 GiB / 1 byte → returns 5_368_709_120 (exceeds Integer.MAX_VALUE, previously saturated).
    Added Long.MAX_VALUE - 1 with partSize = 1 → returns Long.MAX_VALUE - 1 (verifies no overflow at the upper bound).
  • Updated tests (PresignedUrlDownloadHelperTest):

All validatePartResponse call sites updated from 2 to 2L to match the new Long parameter type.

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

@jencymaryjoseph jencymaryjoseph requested a review from a team as a code owner June 10, 2026 17:08
@jencymaryjoseph jencymaryjoseph force-pushed the jencyjos/pre-signed-url-getobject/fix-calculate-total-parts-overflow branch from 0a3ec57 to 1acedb8 Compare June 10, 2026 20:43
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