Skip to content

Commit

Permalink
Merge pull request #4940 from nextcloud/bugfix/debug-conflict
Browse files Browse the repository at this point in the history 
Ensure that the file being processed has had its etag properly sanitised, log etag more
  • Loading branch information
@mgallien
mgallien authored Sep 15, 2022
2 parents c4b6eae + 0a2186f commit 1c5320a
Showing 1 changed file with 49 additions and 0 deletions.
49 changes: 49 additions & 0 deletions src/libsync/discovery.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ void ProcessDirectoryJob::process()
processBlacklisted(path, e.localEntry, e.dbEntry);
continue;
}

// HACK: Sometimes the serverEntry.etag does not correctly have its quotation marks amputated in the string.
// We are once again making sure they are chopped off here, but we should really find the root cause for why
// exactly they are not being lobbed off at any of the prior points of processing.

e.serverEntry.etag = Utility::normalizeEtag(e.serverEntry.etag);

processFile(std::move(path), e.localEntry, e.serverEntry, e.dbEntry);
}
QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs);
Expand Down Expand Up @@ -536,6 +543,11 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
item->_instruction = CSYNC_INSTRUCTION_NEW;
} else {
item->_instruction = CSYNC_INSTRUCTION_SYNC;
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: File" << item->_file << "if (dbEntry._etag != serverEntry.etag)"
<< "dbEntry._etag:" << dbEntry._etag
<< "serverEntry.etag:" << serverEntry.etag
<< "serverEntry.isDirectory:" << serverEntry.isDirectory
<< "dbEntry.isDirectory:" << dbEntry.isDirectory();
}
} else if (dbEntry._modtime != serverEntry.modtime && localEntry.size == serverEntry.size && dbEntry._fileSize == serverEntry.size && dbEntry._etag == serverEntry.etag) {
item->_direction = SyncFileItem::Down;
Expand Down Expand Up @@ -813,6 +825,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(

bool serverModified = item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC
|| item->_instruction == CSYNC_INSTRUCTION_RENAME || item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE;

qCDebug(lcDisco) << "File" << item->_file << "- servermodified:" << serverModified
<< "noServerEntry:" << noServerEntry;

// Decay server modifications to UPDATE_METADATA if the local virtual exists
bool hasLocalVirtual = localEntry.isVirtualFile || (_queryLocal == ParentNotChanged && dbEntry.isVirtualFile());
Expand Down Expand Up @@ -1005,6 +1020,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
item->_modtime = dbEntry._modtime;
item->_previousModtime = dbEntry._modtime;
item->_type = localEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile;
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: File" << item->_file << "if (dbEntry._modtime > 0 && localEntry.modtime <= 0)"
<< "dbEntry._modtime:" << dbEntry._modtime
<< "localEntry.modtime:" << localEntry.modtime;
_childModified = true;
} else {
// Local file was changed
Expand All @@ -1018,6 +1036,13 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
item->_size = localEntry.size;
item->_modtime = localEntry.modtime;
_childModified = true;

qCDebug(lcDisco) << "Local file was changed: File" << item->_file
<< "item->_instruction:" << item->_instruction
<< "noServerEntry:" << noServerEntry
<< "item->_direction:" << item->_direction
<< "item->_size:" << item->_size
<< "item->_modtime:" << item->_modtime;

// Checksum comparison at this stage is only enabled for .eml files,
// check #4754 #4755
Expand Down Expand Up @@ -1355,8 +1380,21 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
// whatever reason.
item->_instruction = isConflict ? CSYNC_INSTRUCTION_CONFLICT : CSYNC_INSTRUCTION_UPDATE_METADATA;
item->_direction = isConflict ? SyncFileItem::None : SyncFileItem::Down;
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_CONFLICT: File" << item->_file << "if (serverEntry.checksumHeader.isEmpty())";
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_CONFLICT: serverEntry.size:" << serverEntry.size
<< "localEntry.size:" << localEntry.size
<< "serverEntry.modtime:" << serverEntry.modtime
<< "localEntry.modtime:" << localEntry.modtime;
return;
}

if (!serverEntry.checksumHeader.isEmpty()) {
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_CONFLICT: File" << item->_file << "if (!serverEntry.checksumHeader.isEmpty())";
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_CONFLICT: serverEntry.size:" << serverEntry.size
<< "localEntry.size:" << localEntry.size
<< "serverEntry.modtime:" << serverEntry.modtime
<< "localEntry.modtime:" << localEntry.modtime;
}

// Do we have an UploadInfo for this?
// Maybe the Upload was completed, but the connection was broken just before
Expand All @@ -1367,6 +1405,10 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
item->_instruction = up._modtime == localEntry.modtime && up._size == localEntry.size
? CSYNC_INSTRUCTION_NONE : CSYNC_INSTRUCTION_SYNC;
item->_direction = SyncFileItem::Up;
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: File" << item->_file << "if (up._valid && up._contentChecksum == serverEntry.checksumHeader)";
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: up._valid:" << up._valid
<< "up._contentChecksum:" << up._contentChecksum
<< "serverEntry.checksumHeader:" << serverEntry.checksumHeader;

// Update the etag and other server metadata in the journal already
// (We can't use a typical CSYNC_INSTRUCTION_UPDATE_METADATA because
Expand All @@ -1385,6 +1427,13 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
}
return;
}

if (!up._valid || up._contentChecksum != serverEntry.checksumHeader) {
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: File" << item->_file << "if (!up._valid && up._contentChecksum != serverEntry.checksumHeader)";
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: up._valid:" << up._valid
<< "up._contentChecksum:" << up._contentChecksum
<< "serverEntry.checksumHeader:" << serverEntry.checksumHeader;
}

// Rely on content hash comparisons to optimize away non-conflicts inside the job
item->_instruction = CSYNC_INSTRUCTION_CONFLICT;
Expand Down

0 comments on commit 1c5320a

Please sign in to comment.