Set folders writable if remote perms require it, recurse into subdirs if etag and remoteperms differ #9637
Set folders writable if remote perms require it, recurse into subdirs if etag and remoteperms differ #9637
Conversation
nilsding
requested review from
Aiiaiiio,
camilasan,
i2h3 and
mgallien
as code owners
|
/backport to stable-33.0 |
|
/backport to stable-4.0 |
mgallien
left a comment
mgallien
left a comment
There was a problem hiding this comment.
not a fan of recursing inside folders if the only change is a permission change without etag changes
unsure what are the side effects of that
that said, if no automated tests complain, you still get the approval
previous behavior should have been covered by tests
…ssions Some of these should not mark the folder as read-only. Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
…odifiable permissions present Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
subfolders should also be updated Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
I'm still not sure if `serverEntry.remotePerm != dbEntry._remotePerm` was intentional, or if it's even necessary to check for that as well. The etag should change if permissions were changed, but this is not always the case (e.g. due to a fixed bug in a storage app). Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>
nilsding
force-pushed
the
bugfix/noid/perms
branch
from
6752ce8 to
2648a47
Compare
|
Artifact containing the AppImage: nextcloud-appimage-pr-9637.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
|
2 participants
Certain remote permissions (CanWrite/CanDelete/CanRename/CanMove) allow contents of a certain folder to be changed. Setting those folders read-only just because the CanAddSubDirectories/CanAddFile permission seems to be the wrong thing to do.
I'm still not sure if
serverEntry.remotePerm != dbEntry._remotePermwas intentional, or if it's even necessary to check for that as well; the etag should change if permissions were changed, but this seems to not always be the case (e.g. due to a fixed bug in a storage app).