Skip to content

Prevent writing invalid mtime#30114

Merged
skjnldsv merged 1 commit intomasterfrom
fix/sanitizeMtime
Dec 7, 2021
Merged

Prevent writing invalid mtime#30114
skjnldsv merged 1 commit intomasterfrom
fix/sanitizeMtime

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Dec 6, 2021

Signed-off-by: John Molakvoæ skjnldsv@protonmail.com

@skjnldsv skjnldsv requested review from a team, ArtificialOwl, PVince81, come-nc, mgallien and nickvergessen and removed request for a team December 6, 2021 14:17
@skjnldsv skjnldsv self-assigned this Dec 6, 2021
@skjnldsv skjnldsv added 3. to review Waiting for reviews bug high labels Dec 6, 2021
@skjnldsv skjnldsv added this to the Nextcloud 24 milestone Dec 6, 2021
mgallien
mgallien reviewed Dec 6, 2021
View reviewed changes
Copy link
Contributor

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

but is it really enough to prevent files from being stored with old invalid mtime ?
is it possible to instead return an error to the client ?

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 6, 2021

but is it really enough to prevent files from being stored with old invalid mtime ? is it possible to instead return an error to the client ?

done

@skjnldsv skjnldsv requested a review from mgallien December 6, 2021 14:33
@nickvergessen
Copy link
Member

nickvergessen commented Dec 6, 2021

/backport to stable23

come-nc
come-nc reviewed Dec 6, 2021
View reviewed changes
@skjnldsv
Prevent writing invalid mtime
5223b0b 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 7, 2021

Tests fixed

@skjnldsv skjnldsv requested a review from come-nc December 7, 2021 08:40
@skjnldsv skjnldsv merged commit ee1fe51 into master Dec 7, 2021
@skjnldsv skjnldsv deleted the fix/sanitizeMtime branch December 7, 2021 11:12
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Dec 7, 2021
Merged
@mgallien
Copy link
Contributor

mgallien commented Dec 8, 2021

@skjnldsv
as files got truncated on Monday after a version of this got applied I decided to test against it and a desktop client sending invalid modified time
the fact is that with what was merged in stable23 I got a 500 internal server error reported and the files is still written on storage

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 4, 2022

/backport to stable22

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Feb 4, 2022
Merged
@PVince81
Copy link
Member

PVince81 commented Feb 18, 2022

/backport to stable21

@PVince81
Copy link
Member

PVince81 commented Feb 18, 2022

/backport to stable20

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Feb 18, 2022
Merged
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Feb 18, 2022
Merged
@blizzz
Copy link
Member

blizzz commented Feb 18, 2022

/backport to stable19

@blizzz
Copy link
Member

blizzz commented Feb 18, 2022

/backport to stable18

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Feb 18, 2022
Merged
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Feb 18, 2022
Merged
@joshtrichards joshtrichards mentioned this pull request Jul 23, 2024
Closed
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessen nickvergessen nickvergessen approved these changes

@come-nc come-nc come-nc approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl

@mgallien mgallien Awaiting requested review from mgallien

Assignees

@skjnldsv skjnldsv

Labels

3. to review Waiting for reviews bug high

Projects

None yet

Milestone

Nextcloud 24

Development

Successfully merging this pull request may close these issues.

6 participants

@skjnldsv @nickvergessen @mgallien @PVince81 @blizzz @come-nc