Skip to content

Commit

Permalink
Only check for external storage permissions when it is a folder.
Browse files Browse the repository at this point in the history 
Signed-off-by: Camila Ayres <[email protected]>
  • Loading branch information
@camilasan
camilasan committed Jun 25, 2024
1 parent 98de577 commit 1525c11
Showing 1 changed file with 52 additions and 67 deletions.
119 changes: 52 additions & 67 deletions src/libsync/discovery.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1356,7 +1356,6 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
const auto originalPath = base.path();

// Function to gradually check conditions for accepting a move-candidate
const auto isExternalStorage = base._remotePerm.hasPermission(RemotePermissions::IsMounted);
auto moveCheck = [&]() {
if (!base.isValid()) {
qCInfo(lcDisco) << "Not a move, no item in db with inode" << localEntry.inode;
Expand Down Expand Up @@ -1405,19 +1404,6 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(

return true;
};

auto isExternalStorageRename = [&] {
OCC::SyncJournalFileRecord dbRecord;
const auto validRecord = _discoveryData->_statedb->getFileRecordByInode(localEntry.inode, &dbRecord);
qCInfo(lcDisco) << "File is saved in DB:" << validRecord;
qCInfo(lcDisco) << "File is in external storage:" << isExternalStorage;
if (validRecord && isExternalStorage) {
qCInfo(lcDisco) << "Try to process rename for path" << dbRecord._path << "and inode" << dbRecord._inode;
return true;
}

return false;
};
const auto isMove = moveCheck();
const auto isE2eeMove = isMove && (base.isE2eEncrypted() || isInsideEncryptedTree());
const auto isCfApiVfsMode = _discoveryData->_syncOptions._vfs && _discoveryData->_syncOptions._vfs->mode() == Vfs::WindowsCfApi;
Expand All @@ -1431,64 +1417,63 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
item->_errorString = tr("Moved to invalid target, restoring");
}

// If it's not a move/rename, it's just a local-NEW
if (!isExternalStorageRename()) {
if ((!isMove || (isE2eeMove && !isE2eeMoveOnlineOnlyItemWithCfApi))) {
if (base.isE2eEncrypted()) {
// renaming the encrypted folder is done via remove + re-upload hence we need to mark the newly created folder as encrypted
// base is a record in the SyncJournal database that contains the data about the being-renamed folder with it's old name and encryption information
item->_e2eEncryptionStatus = EncryptionStatusEnums::fromDbEncryptionStatus(base._e2eEncryptionStatus);
item->_e2eEncryptionServerCapability = EncryptionStatusEnums::fromEndToEndEncryptionApiVersion(_discoveryData->_account->capabilities().clientSideEncryptionVersion());
}
postProcessLocalNew();
finalize();
return;
// If it's not a move it's just a local-NEW
if ((!isMove || (isE2eeMove && !isE2eeMoveOnlineOnlyItemWithCfApi))) {
if (base.isE2eEncrypted()) {
// renaming the encrypted folder is done via remove + re-upload hence we need to mark the newly created folder as encrypted
// base is a record in the SyncJournal database that contains the data about the being-renamed folder with it's old name and encryption information
item->_e2eEncryptionStatus = EncryptionStatusEnums::fromDbEncryptionStatus(base._e2eEncryptionStatus);
item->_e2eEncryptionServerCapability = EncryptionStatusEnums::fromEndToEndEncryptionApiVersion(_discoveryData->_account->capabilities().clientSideEncryptionVersion());
}
postProcessLocalNew();
finalize();
return;
}

// Check local permission if we are allowed to put move the file here
// Technically we should use the permissions from the server, but we'll assume it is the same
const auto serverHasMountRootProperty = _discoveryData->_account->serverHasMountRootProperty();
const auto movePerms = checkMovePermissions(base._remotePerm, originalPath, item->isDirectory());
if (!movePerms.sourceOk || !movePerms.destinationOk || (serverHasMountRootProperty && isExternalStorage) || isE2eeMoveOnlineOnlyItemWithCfApi) {
qCInfo(lcDisco) << "Move without permission to rename base file, "
<< "source:" << movePerms.sourceOk
<< ", target:" << movePerms.destinationOk
<< ", targetNew:" << movePerms.destinationNewOk
<< ", isExternalStorage:" << isExternalStorage
<< ", serverHasMountRootProperty:" << serverHasMountRootProperty
<< ", base._remotePerm:" << base._remotePerm.toString()
<< ", base.path():" << base.path();

// If we can create the destination, do that.
// Permission errors on the destination will be handled by checkPermissions later.
postProcessLocalNew();
finalize();

// If the destination upload will work, we're fine with the source deletion.
// If the source deletion can't work, checkPermissions will error.
// In case of external storage mounted folders we are never allowed to move/delete them
if (movePerms.destinationNewOk && !isExternalStorage && !isE2eeMoveOnlineOnlyItemWithCfApi) {
return;
}
// Check local permission if we are allowed to put move the file here
// Technically we should use the permissions from the server, but we'll assume it is the same
const auto serverHasMountRootProperty = _discoveryData->_account->serverHasMountRootProperty();
const auto isExternalStorage = base._remotePerm.hasPermission(RemotePermissions::IsMounted) && base.isDirectory();
const auto movePerms = checkMovePermissions(base._remotePerm, originalPath, item->isDirectory());
if (!movePerms.sourceOk || !movePerms.destinationOk || (serverHasMountRootProperty && isExternalStorage) || isE2eeMoveOnlineOnlyItemWithCfApi) {
qCInfo(lcDisco) << "Move without permission to rename base file, "
<< "source:" << movePerms.sourceOk
<< ", target:" << movePerms.destinationOk
<< ", targetNew:" << movePerms.destinationNewOk
<< ", isExternalStorage:" << isExternalStorage
<< ", serverHasMountRootProperty:" << serverHasMountRootProperty
<< ", base._remotePerm:" << base._remotePerm.toString()
<< ", base.path():" << base.path();

// If we can create the destination, do that.
// Permission errors on the destination will be handled by checkPermissions later.
postProcessLocalNew();
finalize();

// Here we know the new location can't be uploaded: must prevent the source delete.
// Two cases: either the source item was already processed or not.
auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath);
if (wasDeletedOnClient.first) {
// More complicated. The REMOVE is canceled. Restore will happen next sync.
qCInfo(lcDisco) << "Undid remove instruction on source" << originalPath;
if (!_discoveryData->_statedb->deleteFileRecord(originalPath, true)) {
qCWarning(lcDisco) << "Failed to delete a file record from the local DB" << originalPath;
}
_discoveryData->_statedb->schedulePathForRemoteDiscovery(originalPath);
_discoveryData->_anotherSyncNeeded = true;
} else {
// Signal to future checkPermissions() to forbid the REMOVE and set to restore instead
qCInfo(lcDisco) << "Preventing future remove on source" << originalPath;
_discoveryData->_forbiddenDeletes[originalPath + '/'] = true;
}
// If the destination upload will work, we're fine with the source deletion.
// If the source deletion can't work, checkPermissions will error.
// In case of external storage mounted folders we are never allowed to move/delete them
if (movePerms.destinationNewOk && !isExternalStorage && !isE2eeMoveOnlineOnlyItemWithCfApi) {
return;
}

// Here we know the new location can't be uploaded: must prevent the source delete.
// Two cases: either the source item was already processed or not.
auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath);
if (wasDeletedOnClient.first) {
// More complicated. The REMOVE is canceled. Restore will happen next sync.
qCInfo(lcDisco) << "Undid remove instruction on source" << originalPath;
if (!_discoveryData->_statedb->deleteFileRecord(originalPath, true)) {
qCWarning(lcDisco) << "Failed to delete a file record from the local DB" << originalPath;
}
_discoveryData->_statedb->schedulePathForRemoteDiscovery(originalPath);
_discoveryData->_anotherSyncNeeded = true;
} else {
// Signal to future checkPermissions() to forbid the REMOVE and set to restore instead
qCInfo(lcDisco) << "Preventing future remove on source" << originalPath;
_discoveryData->_forbiddenDeletes[originalPath + '/'] = true;
}
return;
}

auto wasDeletedOnClient = _discoveryData->findAndCancelDeletedJob(originalPath);
Expand Down

0 comments on commit 1525c11

Please sign in to comment.