Skip to content

Commit b9243b0

Browse files
committed
fix(libsync): set read-only permissions after rename, not before
setFileReadOnly() on Windows adds an ACCESS_DENIED_ACE for BUILTIN\Users to the file's DACL. Applying this to the temp file before renaming it to its final destination blocks the rename, because Windows requires Delete access on the source path to complete the operation. This caused a persistent Error 5 (Access Denied) retry loop for read-only shares and Talk attachments on Windows. Move the permission block to after uncheckedRenameReplace() succeeds, targeting the final filename instead of _tmpFile.fileName(). Fixes: #9885 Signed-off-by: Lenart Kos <koslenart@gmail.com>
1 parent f2f9849 commit b9243b0

1 file changed

Lines changed: 10 additions & 10 deletions

File tree

src/libsync/propagatedownload.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,16 +1207,6 @@ void PropagateDownloadFile::downloadFinished()
12071207
}
12081208
}
12091209

1210-
if (_item->_locked == SyncFileItem::LockStatus::LockedItem && (_item->_lockOwnerType != SyncFileItem::LockOwnerType::UserLock || _item->_lockOwnerId != propagator()->account()->davUser())) {
1211-
qCDebug(lcPropagateDownload()) << _tmpFile.fileName() << "file is locked: making it read only";
1212-
FileSystem::setFileReadOnly(_tmpFile.fileName(), true);
1213-
} else {
1214-
qCDebug(lcPropagateDownload()) << _tmpFile.fileName() << "file is not locked: making it"
1215-
<< ((!_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) ? "read only"
1216-
: "read write");
1217-
FileSystem::setFileReadOnlyWeak(_tmpFile.fileName(), (!_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanWrite)));
1218-
}
1219-
12201210
const auto isConflict = (_item->_instruction == CSYNC_INSTRUCTION_CONFLICT
12211211
&& (FileSystem::isDir(filename) || !FileSystem::fileEquals(filename, _tmpFile.fileName()))) ||
12221212
_item->_instruction == CSYNC_INSTRUCTION_CASE_CLASH_CONFLICT;
@@ -1281,6 +1271,16 @@ void PropagateDownloadFile::downloadFinished()
12811271

12821272
FileSystem::setFileHidden(filename, false);
12831273

1274+
if (_item->_locked == SyncFileItem::LockStatus::LockedItem && (_item->_lockOwnerType != SyncFileItem::LockOwnerType::UserLock || _item->_lockOwnerId != propagator()->account()->davUser())) {
1275+
qCDebug(lcPropagateDownload()) << filename << "file is locked: making it read only";
1276+
FileSystem::setFileReadOnly(filename, true);
1277+
} else {
1278+
qCDebug(lcPropagateDownload()) << filename << "file is not locked: making it"
1279+
<< ((!_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) ? "read only"
1280+
: "read write");
1281+
FileSystem::setFileReadOnlyWeak(filename, (!_item->_remotePerm.isNull() && !_item->_remotePerm.hasPermission(RemotePermissions::CanWrite)));
1282+
}
1283+
12841284
if (_needParentFolderRestorePermissions) {
12851285
FileSystem::setFolderPermissions(QString::fromStdWString(_parentPath.wstring()), FileSystem::FolderPermissions::ReadOnly);
12861286
emit propagator()->touchedFile(QString::fromStdWString(_parentPath.wstring()));

0 commit comments

Comments
 (0)