From 674fca386807ce23087d3053716e5d771dfd6ea8 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Mon, 7 Sep 2020 20:18:25 +0200 Subject: [PATCH] lib/db, lib/protocol: Never need empty-version entries (fixes #6961) (#6962) Avoid havoc when discovering locally-deleted-upgraded files during repair / need calculation... Co-authored-by: Simon Frei --- lib/db/transactions.go | 8 ++-- lib/model/model_test.go | 71 ++++++++++++++++++++++++++++++++++ lib/protocol/bep_extensions.go | 4 -- lib/protocol/vector.go | 5 +++ 4 files changed, 81 insertions(+), 7 deletions(-) diff --git a/lib/db/transactions.go b/lib/db/transactions.go index c51b35b21..f83f3f7db 100644 --- a/lib/db/transactions.go +++ b/lib/db/transactions.go @@ -474,7 +474,7 @@ func (t *readOnlyTransaction) withNeedIteratingGlobal(folder, device []byte, tru if shouldDebug() { if globalDev, ok := globalFV.FirstDevice(); ok { globalID, _ := protocol.DeviceIDFromBytes(globalDev) - l.Debugf("need folder=%q device=%v name=%q have=%v invalid=%v haveV=%v globalV=%v globalDev=%v", folder, devID, name, have, haveFV.IsInvalid(), haveFV.Version, gf.FileVersion(), globalID) + l.Debugf("need folder=%q device=%v name=%q have=%v invalid=%v haveV=%v haveDeleted=%v globalV=%v globalDeleted=%v globalDev=%v", folder, devID, name, have, haveFV.IsInvalid(), haveFV.Version, haveFV.Deleted, gf.FileVersion(), globalFV.Deleted, globalID) } } if !fn(gf) { @@ -759,8 +759,10 @@ func (t readWriteTransaction) updateLocalNeed(keyBuf, folder, name []byte, add b } func Need(global FileVersion, haveLocal bool, localVersion protocol.Vector) bool { - // We never need an invalid file. - if global.IsInvalid() { + // We never need an invalid file or a file without a valid version (just + // another way of expressing "invalid", really, until we fix that + // part...). + if global.IsInvalid() || global.Version.IsEmpty() { return false } // We don't need a deleted file if we don't have it. diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 3bac43cf1..005e0e879 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -4017,6 +4017,77 @@ func testConfigChangeClosesConnections(t *testing.T, expectFirstClosed, expectSe } } +// The end result of the tested scenario is that the global version entry has an +// empty version vector and is not deleted, while everything is actually deleted. +// That then causes these files to be considered as needed, while they are not. +// https://github.com/syncthing/syncthing/issues/6961 +func TestIssue6961(t *testing.T) { + wcfg, fcfg := tmpDefaultWrapper() + tfs := fcfg.Filesystem() + wcfg.SetDevice(config.NewDeviceConfiguration(device2, "device2")) + fcfg.Type = config.FolderTypeReceiveOnly + fcfg.Devices = append(fcfg.Devices, config.FolderDeviceConfiguration{DeviceID: device2}) + wcfg.SetFolder(fcfg) + m := setupModel(wcfg) + // defer cleanupModelAndRemoveDir(m, tfs.URI()) + defer cleanupModel(m) + + name := "foo" + version := protocol.Vector{}.Update(device1.Short()) + + // Remote, valid and existing file + m.Index(device1, fcfg.ID, []protocol.FileInfo{{Name: name, Version: version, Sequence: 1}}) + // Remote, invalid (receive-only) and existing file + m.Index(device2, fcfg.ID, []protocol.FileInfo{{Name: name, RawInvalid: true, Sequence: 1}}) + // Create a local file + if fd, err := tfs.OpenFile(name, fs.OptCreate, 0666); err != nil { + t.Fatal(err) + } else { + fd.Close() + } + if info, err := tfs.Lstat(name); err != nil { + t.Fatal(err) + } else { + l.Infoln("intest", info.Mode) + } + m.ScanFolders() + + // Get rid of valid global + waiter, err := wcfg.RemoveDevice(device1) + if err != nil { + t.Fatal(err) + } + waiter.Wait() + + // Delete the local file + must(t, tfs.Remove(name)) + m.ScanFolders() + + // Drop ther remote index, add some other file. + m.Index(device2, fcfg.ID, []protocol.FileInfo{{Name: "bar", RawInvalid: true, Sequence: 1}}) + + // Recalculate everything + fcfg.Paused = true + waiter, err = wcfg.SetFolder(fcfg) + if err != nil { + t.Fatal(err) + } + waiter.Wait() + m.db.CheckRepair() + fcfg.Paused = false + waiter, err = wcfg.SetFolder(fcfg) + if err != nil { + t.Fatal(err) + } + waiter.Wait() + + if comp := m.Completion(device2, fcfg.ID); comp.NeedDeletes != 0 { + t.Error("Expected 0 needed deletes, got", comp.NeedDeletes) + } else { + t.Log(comp) + } +} + func TestCompletionEmptyGlobal(t *testing.T) { wcfg, fcfg := tmpDefaultWrapper() m := setupModel(wcfg) diff --git a/lib/protocol/bep_extensions.go b/lib/protocol/bep_extensions.go index faf64db01..f5914d164 100644 --- a/lib/protocol/bep_extensions.go +++ b/lib/protocol/bep_extensions.go @@ -189,10 +189,6 @@ func WinsConflict(f, other FileIntf) bool { return f.FileVersion().Compare(other.FileVersion()) == ConcurrentGreater } -func (f FileInfo) IsEmpty() bool { - return f.Version.Counters == nil -} - func (f FileInfo) IsEquivalent(other FileInfo, modTimeWindow time.Duration) bool { return f.isEquivalent(other, modTimeWindow, false, false, 0) } diff --git a/lib/protocol/vector.go b/lib/protocol/vector.go index f4d8662dd..74a8bd42a 100644 --- a/lib/protocol/vector.go +++ b/lib/protocol/vector.go @@ -127,6 +127,11 @@ func (v Vector) Counter(id ShortID) uint64 { return 0 } +// IsEmpty returns true when there are no counters. +func (v Vector) IsEmpty() bool { + return len(v.Counters) == 0 +} + // DropOthers removes all counters, keeping only the one with given id. If there // is no such counter, an empty Vector is returned. func (v Vector) DropOthers(id ShortID) Vector {