Skip to content

Reset checksum when writing files to object store #27586

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
blizzz merged 1 commit into from
Jul 1, 2021
Merged

Reset checksum when writing files to object store #27586

blizzz merged 1 commit into from
Jul 1, 2021

Conversation

@juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Jun 21, 2021 •
edited
Loading

With a local storage the checksum will be reset when scanning the file after writing to it:

server/lib/private/Files/Cache/Scanner.php

Line 222 in 215aef3

$newData['checksum'] = '';

This does not work for object store storage backends as those use a NoopScanner so the checksum was never reset for those.

Fixes #27563

@juliusknorr juliusknorr added this to the Nextcloud 22 milestone Jun 21, 2021
blizzz
blizzz approved these changes Jun 22, 2021
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

🐘

@blizzz
Copy link
Member

blizzz commented Jun 22, 2021

are backports necessary?

kesselb
kesselb reviewed Jun 22, 2021
View reviewed changes
Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

LGTM. Let's see what icewind thinks.

@blizzz blizzz mentioned this pull request Jun 23, 2021
Merged
39 tasks
@juliusknorr juliusknorr mentioned this pull request Jun 24, 2021
Closed
@blizzz blizzz modified the milestones: Nextcloud 22, Nextcloud 23 Jun 24, 2021
@juliusknorr juliusknorr added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 29, 2021
@juliusknorr
Copy link
Member Author

juliusknorr commented Jun 29, 2021

/backport to stable22

@juliusknorr
Copy link
Member Author

juliusknorr commented Jun 29, 2021

/backport to stable21

@juliusknorr
Copy link
Member Author

juliusknorr commented Jun 29, 2021

/backport to stable20

@ImaCrea
Copy link

ImaCrea commented Jul 1, 2021 •
edited
Loading

/backport to stable20

Hello, do you think it would be possible to backport it to 19? Or is it risky/not recommended? Thanks for your help :)

@blizzz
Copy link
Member

blizzz commented Jul 1, 2021

/backport to stable20

Hello, do you think it would be possible to backport it to 19? Or is it risky/not recommended? Thanks for your help :)

19 is having its last release today and goes EOL. So it would not get included.

@blizzz blizzz merged commit 62675eb into master Jul 1, 2021
@blizzz blizzz deleted the bugfi/noid/objectstore-checksum branch July 1, 2021 15:30
This was referenced Jul 1, 2021
Merged
Merged
Merged
@ImaCrea
Copy link

ImaCrea commented Jul 1, 2021

/backport to stable20

Hello, do you think it would be possible to backport it to 19? Or is it risky/not recommended? Thanks for your help :)

19 is having its last release today and goes EOL. So it would not get included.

Oh, didn't know that. Thanks for the info ! Gotta update quick I guess :)

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

Reviewers

@kesselb kesselb kesselb left review comments

@icewind1991 icewind1991 icewind1991 approved these changes

@blizzz blizzz blizzz approved these changes

Assignees

No one assigned

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: object storage

Projects

None yet

Milestone

Nextcloud 23

Development

Successfully merging this pull request may close these issues.

Synchronization checksum error on a document modified online (onlyoffice)

6 participants

@juliusknorr @blizzz @ImaCrea @icewind1991 @kesselb