Ensure that the file being processed has had its etag properly sanitised, log etag more

Signed-off-by: Claudio Cambra <claudio.cambra@gmail.com>
This commit is contained in:
Claudio Cambra 2022-09-14 12:56:59 +02:00 committed by Matthieu Gallien
parent c4b6eae299
commit 0a2186f8d9

View file

@ -181,6 +181,13 @@ void ProcessDirectoryJob::process()
processBlacklisted(path, e.localEntry, e.dbEntry); processBlacklisted(path, e.localEntry, e.dbEntry);
continue; 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); processFile(std::move(path), e.localEntry, e.serverEntry, e.dbEntry);
} }
QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs); QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs);
@ -536,6 +543,11 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
item->_instruction = CSYNC_INSTRUCTION_NEW; item->_instruction = CSYNC_INSTRUCTION_NEW;
} else { } else {
item->_instruction = CSYNC_INSTRUCTION_SYNC; 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) { } else if (dbEntry._modtime != serverEntry.modtime && localEntry.size == serverEntry.size && dbEntry._fileSize == serverEntry.size && dbEntry._etag == serverEntry.etag) {
item->_direction = SyncFileItem::Down; item->_direction = SyncFileItem::Down;
@ -813,6 +825,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
bool serverModified = item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC bool serverModified = item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC
|| item->_instruction == CSYNC_INSTRUCTION_RENAME || item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE; || 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 // Decay server modifications to UPDATE_METADATA if the local virtual exists
bool hasLocalVirtual = localEntry.isVirtualFile || (_queryLocal == ParentNotChanged && dbEntry.isVirtualFile()); bool hasLocalVirtual = localEntry.isVirtualFile || (_queryLocal == ParentNotChanged && dbEntry.isVirtualFile());
@ -1005,6 +1020,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
item->_modtime = dbEntry._modtime; item->_modtime = dbEntry._modtime;
item->_previousModtime = dbEntry._modtime; item->_previousModtime = dbEntry._modtime;
item->_type = localEntry.isDirectory ? ItemTypeDirectory : ItemTypeFile; 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; _childModified = true;
} else { } else {
// Local file was changed // Local file was changed
@ -1018,6 +1036,13 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
item->_size = localEntry.size; item->_size = localEntry.size;
item->_modtime = localEntry.modtime; item->_modtime = localEntry.modtime;
_childModified = true; _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, // Checksum comparison at this stage is only enabled for .eml files,
// check #4754 #4755 // check #4754 #4755
@ -1355,8 +1380,21 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
// whatever reason. // whatever reason.
item->_instruction = isConflict ? CSYNC_INSTRUCTION_CONFLICT : CSYNC_INSTRUCTION_UPDATE_METADATA; item->_instruction = isConflict ? CSYNC_INSTRUCTION_CONFLICT : CSYNC_INSTRUCTION_UPDATE_METADATA;
item->_direction = isConflict ? SyncFileItem::None : SyncFileItem::Down; 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; 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? // Do we have an UploadInfo for this?
// Maybe the Upload was completed, but the connection was broken just before // 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 item->_instruction = up._modtime == localEntry.modtime && up._size == localEntry.size
? CSYNC_INSTRUCTION_NONE : CSYNC_INSTRUCTION_SYNC; ? CSYNC_INSTRUCTION_NONE : CSYNC_INSTRUCTION_SYNC;
item->_direction = SyncFileItem::Up; 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 // Update the etag and other server metadata in the journal already
// (We can't use a typical CSYNC_INSTRUCTION_UPDATE_METADATA because // (We can't use a typical CSYNC_INSTRUCTION_UPDATE_METADATA because
@ -1385,6 +1427,13 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
} }
return; 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 // Rely on content hash comparisons to optimize away non-conflicts inside the job
item->_instruction = CSYNC_INSTRUCTION_CONFLICT; item->_instruction = CSYNC_INSTRUCTION_CONFLICT;