Skip to content

fix(S3): Only append streams if non-seekable - #55654

Merged
artonge merged 1 commit into
masterfrom
artonge/fix/do_not_happen_streams
Oct 10, 2025
Merged

fix(S3): Only append streams if non-seekable#55654
artonge merged 1 commit into
masterfrom
artonge/fix/do_not_happen_streams

Conversation

@artonge

@artongeartonge commented Oct 9, 2025

Copy link
Copy Markdown
Collaborator

Later, when we rewind the stream in writeMultiPart during retry, both streams were rewinded, so the resulting stream was bigger than expected.

Inspired by https://github.com/aws/aws-sdk-php/blob/master/src/S3/ObjectUploader.php#L136-L146

@artonge
artonge requested a review from a team as a code ownerOctober 9, 2025 15:44
@artonge
artonge requested review from Altahrim, provokateurin and salmart-dev and removed request for a teamOctober 9, 2025 15:44
@artongeartonge self-assigned this Oct 9, 2025
@artongeartonge added bug 3. to review Waiting for reviews php Pull requests that update Php code feature: object storage labels Oct 9, 2025
@artongeartonge added this to the Nextcloud 33 milestone Oct 9, 2025
@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport to stable32

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport to stable31

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport to stable30

@artonge
artonge requested review from icewind1991 and removed request for salmart-devOctober 9, 2025 15:45

@icewind1991icewind1991 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should check that the stream is seekable.

If it isn't then the retry will fail anyway so the current logic would work for that

@artongeartonge changed the title fix(S3): Do not append streams for multipart uploadsfix(S3): Only append streams if non-seekableOct 9, 2025
@artonge
artongeforce-pushed the artonge/fix/do_not_happen_streams branch from 8f43415 to adc45bfCompareOctober 9, 2025 15:56
@artonge
artongeforce-pushed the artonge/fix/do_not_happen_streams branch from adc45bf to c02e47aCompareOctober 9, 2025 16:15
Comment threadlib/private/Files/ObjectStore/S3ObjectTrait.php Outdated
Later, when we rewind the stream in `writeMultiPart` during retry, both streams were rewinded, so the resulting stream was bigger than expected.
Inspired by https://github.com/aws/aws-sdk-php/blob/master/src/S3/ObjectUploader.php#L136-L146
Signed-off-by: Louis Chmn <louis@chmn.me>
@artonge
artongeforce-pushed the artonge/fix/do_not_happen_streams branch from c02e47a to 00ec57eCompareOctober 10, 2025 07:54
@artonge
artonge merged commit 9dfd6f1 into masterOct 10, 2025
188 checks passed
@artonge
artonge deleted the artonge/fix/do_not_happen_streams branch October 10, 2025 12:43
@backportbot

Copy link
Copy Markdown

The backport to stable30 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable30
git pull origin stable30
# Create the new backport branch
git checkout -b backport/55654/stable30
# Cherry pick the change from the commit sha1 of the change against the default branch# This might cause conflicts, resolve them
git cherry-pick 00ec57ef
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/55654/stable30

Error: Failed to check for changes with origin/stable30: No changes found in backport branch


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

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

Labels

3. to reviewWaiting for reviewsbackport-requestbugfeature: object storagephpPull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@artonge@icewind1991@leftybournes@provokateurin