diff --git a/lib/model/folder_recvonly.go b/lib/model/folder_recvonly.go index 72068764b..210308c5f 100644 --- a/lib/model/folder_recvonly.go +++ b/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 - // file must originate here. A revert then means to delete it. + + switch gf, ok := snap.GetGlobal(fi.Name); { + 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, diff --git a/lib/model/folder_recvonly_test.go b/lib/model/folder_recvonly_test.go index 094efe504..4adeac7d4 100644 --- a/lib/model/folder_recvonly_test.go +++ b/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()