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

Fix discovery crash when non-virtual but suffixed files exist on server or in db #6962

Merged
merged 1 commit into from
Jan 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 31 additions & 16 deletions src/libsync/discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,6 @@ void ProcessDirectoryJob::process()
}
_serverNormalQueryEntries.clear();

for (auto &e : _localNormalQueryEntries) {
// Remove the virtual file suffix
auto name = e.name;
if (e.isVirtualFile && isVfsWithSuffix()) {
chopVirtualFileSuffix(name);
auto &entry = entries[name];
// If there is both a virtual file and a real file, we must keep the real file
if (!entry.localEntry.isValid())
entry.localEntry = std::move(e);
} else {
entries[name].localEntry = std::move(e);
}
}
_localNormalQueryEntries.clear();

// fetch all the name from the DB
auto pathU8 = _currentFolder._original.toUtf8();
if (!_discoveryData->_statedb->listFilesInPath(pathU8, [&](const SyncJournalFileRecord &rec) {
Expand All @@ -109,6 +94,21 @@ void ProcessDirectoryJob::process()
return;
}

for (auto &e : _localNormalQueryEntries) {
// Normally for vfs-suffix files the local entries need the suffix removed.
// However, don't do it if "foo.owncloud" exists on the server or in the db
// (as a non-virtual file): we don't want to create two entries.
auto name = e.name;
if (e.isVirtualFile && isVfsWithSuffix() && entries.find(name) == entries.end()) {
chopVirtualFileSuffix(name);
// If there is both a virtual file and a real file, we must keep the real file
if (entries[name].localEntry.isValid())
continue;
}
entries[name].localEntry = std::move(e);
}
_localNormalQueryEntries.clear();

//
// Iterate over entries and process them
//
Expand All @@ -123,7 +123,7 @@ void ProcessDirectoryJob::process()
if (e.dbEntry.isVirtualFile()) {
ASSERT(hasVirtualFileSuffix(e.dbEntry._path));
addVirtualFileSuffix(path._original);
} else if (e.localEntry.isVirtualFile) {
} else if (e.localEntry.isVirtualFile && !e.dbEntry.isValid()) {
// We don't have a db entry - but it should be at this path
addVirtualFileSuffix(path._original);
}
Expand Down Expand Up @@ -304,6 +304,18 @@ void ProcessDirectoryJob::processFile(PathTuple path,
if (item->_type == ItemTypeVirtualFileDehydration)
item->_type = ItemTypeFile;

// VFS suffixed files on the server are ignored
if (isVfsWithSuffix()) {
if (hasVirtualFileSuffix(serverEntry.name)
|| (localEntry.isVirtualFile && !dbEntry.isVirtualFile() && hasVirtualFileSuffix(dbEntry._path))) {
item->_instruction = CSYNC_INSTRUCTION_IGNORE;
item->_errorString = tr("File has extension reserved for virtual files.");
_childIgnored = true;
emit _discoveryData->itemDiscovered(item);
return;
}
}

if (serverEntry.isValid()) {
processFileAnalyzeRemoteInfo(item, path, localEntry, serverEntry, dbEntry);
return;
Expand Down Expand Up @@ -673,6 +685,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
item->_direction = SyncFileItem::Down;
item->_instruction = CSYNC_INSTRUCTION_NEW;
item->_type = ItemTypeVirtualFile;
} else {
qCInfo(lcDisco) << "Virtual file with non-virtual db entry, ignoring:" << item->_file;
item->_instruction = CSYNC_INSTRUCTION_IGNORE;
}
}
} else if (!typeChange && ((dbEntry._modtime == localEntry.modtime && dbEntry._fileSize == localEntry.size) || localEntry.isDirectory)) {
Expand Down
47 changes: 45 additions & 2 deletions test/testsyncvirtualfiles.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,7 @@ private slots:
void testNewVirtuals()
{
FakeFolder fakeFolder{ FileInfo() };
SyncOptions syncOptions = vfsSyncOptions(fakeFolder);
fakeFolder.syncEngine().setSyncOptions(syncOptions);
fakeFolder.syncEngine().setSyncOptions(vfsSyncOptions(fakeFolder));
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());

auto setPin = [&] (const QByteArray &path, PinState state) {
Expand Down Expand Up @@ -791,6 +790,50 @@ private slots:
QVERIFY(fakeFolder.currentLocalState().find("local/file1"));
QVERIFY(fakeFolder.currentLocalState().find("unspec/file1.owncloud"));
}

// Check what happens if vfs-suffixed files exist on the server or in the db
void testSuffixOnServerOrDb()
{
FakeFolder fakeFolder{ FileInfo() };

QSignalSpy completeSpy(&fakeFolder.syncEngine(), SIGNAL(itemCompleted(const SyncFileItemPtr &)));
auto cleanup = [&]() {
completeSpy.clear();
};
cleanup();

// file1.owncloud is happily synced with Vfs::Off
fakeFolder.remoteModifier().mkdir("A");
fakeFolder.remoteModifier().insert("A/file1.owncloud");
QVERIFY(fakeFolder.syncOnce());
QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
cleanup();

// Enable suffix vfs
fakeFolder.syncEngine().setSyncOptions(vfsSyncOptions(fakeFolder));

// Local changes of suffixed file do nothing
fakeFolder.localModifier().appendByte("A/file1.owncloud");
QVERIFY(fakeFolder.syncOnce());
QVERIFY(itemInstruction(completeSpy, "A/file1.owncloud", CSYNC_INSTRUCTION_IGNORE));
cleanup();

// Remote don't do anything either
fakeFolder.remoteModifier().appendByte("A/file1.owncloud");
QVERIFY(fakeFolder.syncOnce());
QVERIFY(itemInstruction(completeSpy, "A/file1.owncloud", CSYNC_INSTRUCTION_IGNORE));
cleanup();

// New files with a suffix aren't propagated downwards in the first place
fakeFolder.remoteModifier().insert("A/file2.owncloud");
QVERIFY(fakeFolder.syncOnce());
QVERIFY(itemInstruction(completeSpy, "A/file2.owncloud", CSYNC_INSTRUCTION_IGNORE));
QVERIFY(fakeFolder.currentRemoteState().find("A/file2.owncloud"));
QVERIFY(!fakeFolder.currentLocalState().find("A/file2"));
QVERIFY(!fakeFolder.currentLocalState().find("A/file2.owncloud"));
QVERIFY(!fakeFolder.currentLocalState().find("A/file2.owncloud.owncloud"));
cleanup();
}
};

QTEST_GUILESS_MAIN(TestSyncVirtualFiles)
Expand Down