Skip to content

Conversation

@icewind1991
Copy link
Member

For multi-part s3 uploads, validate that the chunks add up to the expected size before completing the upload.

This prevents partially uploaded files from being saved in s3.

To test:

  • Ensure that your setup has no request buffering
  • Start a non-chunked upload of a large file (such as with curl)
  • Cancel the upload before it can complete
    • Before: the partial uploaded file is saved.
    • After: No partial data is saved

This fix applies both on newly created file and overwritten files.

@icewind1991 icewind1991 added this to the Nextcloud 32 milestone Jul 28, 2025
@icewind1991 icewind1991 requested a review from a team as a code owner July 28, 2025 17:59
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Jul 28, 2025
@icewind1991 icewind1991 requested review from nfebe, provokateurin and yemkareems and removed request for a team July 28, 2025 17:59
@icewind1991
Copy link
Member Author

Wasn't able to figure out a good solution for single part uploads so far, but smaller files should be less likely to be canceled anyway 🙈

@icewind1991 icewind1991 force-pushed the s3-multipart-size-check branch from 0ecec88 to 2687b73 Compare July 28, 2025 18:06
@joshtrichards
Copy link
Member

@icewind1991
Copy link
Member Author

Wasn't able to figure out a good solution for single part uploads so far

Ok, I think setting the ContentLength will make S3 detect the aborted single-part uploads

@icewind1991 icewind1991 force-pushed the s3-multipart-size-check branch from 52e5c09 to c762e86 Compare July 29, 2025 16:29
@icewind1991 icewind1991 requested a review from a team as a code owner July 29, 2025 17:06
@icewind1991 icewind1991 requested review from skjnldsv and susnux and removed request for a team July 29, 2025 17:06
@provokateurin provokateurin enabled auto-merge July 30, 2025 09:19
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Aug 1, 2025
@icewind1991 icewind1991 force-pushed the s3-multipart-size-check branch 3 times, most recently from 8ab626b to 7d3ede0 Compare August 5, 2025 12:19
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the s3-multipart-size-check branch from 7d3ede0 to 97efc95 Compare August 5, 2025 17:18
@provokateurin provokateurin merged commit 51e5f7b into master Aug 5, 2025
222 of 232 checks passed
@provokateurin provokateurin deleted the s3-multipart-size-check branch August 5, 2025 18:57
@icewind1991
Copy link
Member Author

/backport to stable31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants