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 <[email protected]>

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.

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)

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)
 }

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 {