lib/model: Fix reverting when version has only our own ID (fixes #7708) (#7714)

lib/model/folder_recvonly.go

@@ -100,9 +100,15 @@ func (f *receiveOnlyFolder) revert() error {
 		}
 
 		fi.LocalFlags &^= protocol.FlagLocalReceiveOnly
-		if len(fi.Version.Counters) == 1 && fi.Version.Counters[0].ID == f.shortID {
+
-			// We are the only device mentioned in the version vector so the
+		switch gf, ok := snap.GetGlobal(fi.Name); {
-			// file must originate here. A revert then means to delete it.
+		case !ok:
+			msg := "Unexpected global file that we have locally"
+			l.Debugf("%v revert: %v: %v", f, msg, fi.Name)
+			f.evLogger.Log(events.Failure, msg)
+			return true
+		case gf.IsReceiveOnlyChanged():
+			// The global file is our own. A revert then means to delete it.
 			// We'll delete files directly, directories get queued and
 			// handled below.
 
@@ -114,10 +120,12 @@ func (f *receiveOnlyFolder) revert() error {
 			if !handled {
 				return true // continue
 			}
-
 			fi.SetDeleted(f.shortID)
 			fi.Version = protocol.Vector{} // if this file ever resurfaces anywhere we want our delete to be strictly older
-		} else {
+		case gf.IsEquivalentOptional(fi, f.modTimeWindow, false, false, protocol.FlagLocalReceiveOnly):
+			// What we have locally is equivalent to the global file.
+			fi = gf
+		default:
 			// Revert means to throw away our local changes. We reset the
 			// version to the empty vector, which is strictly older than any
 			// other existing version. It is not in conflict with anything,

lib/model/folder_recvonly_test.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"github.com/syncthing/syncthing/lib/config"
+	"github.com/syncthing/syncthing/lib/events"
 	"github.com/syncthing/syncthing/lib/fs"
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syncthing/syncthing/lib/scanner"
@@ -415,6 +416,69 @@ func TestRecvOnlyRemoteUndoChanges(t *testing.T) {
 	}
 }
 
+func TestRecvOnlyRevertOwnID(t *testing.T) {
+	// If the folder was receive-only in the past, the global item might have
+	// only our id in the version vector and be valid. There was a bug based on
+	// the incorrect assumption that this can never happen.
+
+	// Get us a model up and running
+
+	m, f, wcfgCancel := setupROFolder(t)
+	defer wcfgCancel()
+	ffs := f.Filesystem()
+	defer cleanupModel(m)
+
+	// Create some test data
+
+	must(t, ffs.MkdirAll(".stfolder", 0755))
+	data := []byte("hello\n")
+	name := "foo"
+	must(t, writeFile(ffs, name, data, 0644))
+
+	// Make sure the file is scanned and locally changed
+	must(t, m.ScanFolder("ro"))
+	fi, ok := m.testCurrentFolderFile(f.ID, name)
+	if !ok {
+		t.Fatal("File missing")
+	} else if !fi.IsReceiveOnlyChanged() {
+		t.Fatal("File should be receiveonly changed")
+	}
+	fi.LocalFlags = 0
+	v := fi.Version.Counters[0].Value
+	fi.Version.Counters[0].Value = uint64(time.Unix(int64(v), 0).Add(-10 * time.Second).Unix())
+
+	// Monitor the outcome
+	sub := f.evLogger.Subscribe(events.LocalIndexUpdated)
+	defer sub.Unsubscribe()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go func() {
+		for {
+			select {
+			case <-ctx.Done():
+				return
+			case <-sub.C():
+				if file, _ := m.testCurrentFolderFile(f.ID, name); file.Deleted {
+					t.Error("local file was deleted")
+					cancel()
+				} else if file.IsEquivalent(fi, f.modTimeWindow) {
+					cancel() // That's what we are waiting for
+				}
+			}
+		}
+	}()
+
+	// Receive an index update with an older version, but valid and then revert
+	must(t, m.Index(device1, f.ID, []protocol.FileInfo{fi}))
+	f.Revert()
+
+	select {
+	case <-ctx.Done():
+	case <-time.After(10 * time.Second):
+		t.Fatal("timed out")
+	}
+}
+
 func setupKnownFiles(t *testing.T, ffs fs.Filesystem, data []byte) []protocol.FileInfo {
 	t.Helper()