Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Sep 15, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 49 additions & 0 deletions src/libsync/discovery.cpp
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