From 0a2186f8d9bc1d513669eb00b0a8a6af50caec4a Mon Sep 17 00:00:00 2001 From: Claudio Cambra Date: Wed, 14 Sep 2022 12:56:59 +0200 Subject: [PATCH] Ensure that the file being processed has had its etag properly sanitised, log etag more Signed-off-by: Claudio Cambra --- src/libsync/discovery.cpp | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index 714fb09bda88d..a437dc48d30e1 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -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); @@ -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; @@ -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()); @@ -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 @@ -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 @@ -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 @@ -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 @@ -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;