lib/fs: Properly handle case insensitive systems (fixes #1787, fixes #2739, fixes #5708)

With this change we emulate a case sensitive filesystem on top of
insensitive filesystems. This means we correctly pick up case-only renames
and throw a case conflict error when there would be multiple files differing
only in case.

This safety check has a small performance hit (about 20% more filesystem
operations when scanning for changes). The new advanced folder option
`caseSensitiveFS` can be used to disable the safety checks, retaining the
previous behavior on systems known to be fully case sensitive.

Co-authored-by: Jakob Borg <jakob@kastelo.net>

lib/model/folder_sendrecv.go

@@ -129,9 +129,9 @@ type sendReceiveFolder struct {
 	blockPullReorderer blockPullReorderer
 	writeLimiter       *byteSemaphore
 
-	pullErrors    map[string]string // errors for most recent/current iteration
-	oldPullErrors map[string]string // errors from previous iterations for log filtering only
-	pullErrorsMut sync.Mutex
+	pullErrors     map[string]string // actual exposed pull errors
+	tempPullErrors map[string]string // pull errors that might be just transient
+	pullErrorsMut  sync.Mutex
 }
 
 func newSendReceiveFolder(model *model, fset *db.FileSet, ignores *ignore.Matcher, cfg config.FolderConfiguration, ver versioner.Versioner, fs fs.Filesystem, evLogger events.Logger, ioLimiter *byteSemaphore) service {
@@ -192,6 +192,10 @@ func (f *sendReceiveFolder) pull() bool {
 
 	changed := 0
 
+	f.pullErrorsMut.Lock()
+	f.pullErrors = nil
+	f.pullErrorsMut.Unlock()
+
 	for tries := 0; tries < maxPullerIterations; tries++ {
 		select {
 		case <-f.ctx.Done():
@@ -216,8 +220,14 @@ func (f *sendReceiveFolder) pull() bool {
 	}
 
 	f.pullErrorsMut.Lock()
+	f.pullErrors = f.tempPullErrors
+	f.tempPullErrors = nil
+	for path, err := range f.pullErrors {
+		l.Infof("Puller (folder %s, item %q): %v", f.Description(), path, err)
+	}
 	pullErrNum := len(f.pullErrors)
 	f.pullErrorsMut.Unlock()
+
 	if pullErrNum > 0 {
 		l.Infof("%v: Failed to sync %v items", f.Description(), pullErrNum)
 		f.evLogger.Log(events.FolderErrors, map[string]interface{}{
@@ -235,8 +245,7 @@ func (f *sendReceiveFolder) pull() bool {
 // flagged as needed in the folder.
 func (f *sendReceiveFolder) pullerIteration(scanChan chan<- string) int {
 	f.pullErrorsMut.Lock()
-	f.oldPullErrors = f.pullErrors
-	f.pullErrors = make(map[string]string)
+	f.tempPullErrors = make(map[string]string)
 	f.pullErrorsMut.Unlock()
 
 	snap := f.fset.Snapshot()
@@ -306,10 +315,6 @@ func (f *sendReceiveFolder) pullerIteration(scanChan chan<- string) int {
 	close(dbUpdateChan)
 	updateWg.Wait()
 
-	f.pullErrorsMut.Lock()
-	f.oldPullErrors = nil
-	f.pullErrorsMut.Unlock()
-
 	f.queue.Reset()
 
 	return changed
@@ -739,7 +744,11 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, snap *db.Snaps
 	}
 
 	// There is already something under that name, we need to handle that.
-	if info, err := f.fs.Lstat(file.Name); err == nil {
+	switch info, err := f.fs.Lstat(file.Name); {
+	case err != nil && !fs.IsNotExist(err):
+		f.newPullError(file.Name, errors.Wrap(err, "checking for existing symlink"))
+		return
+	case err == nil:
 		// Check that it is what we have in the database.
 		curFile, hasCurFile := f.model.CurrentFolderFile(f.folderID, file.Name)
 		if err := f.scanIfItemChanged(file.Name, info, curFile, hasCurFile, scanChan); err != nil {
@@ -1783,7 +1792,7 @@ func (f *sendReceiveFolder) newPullError(path string, err error) {
 	// We might get more than one error report for a file (i.e. error on
 	// Write() followed by Close()); we keep the first error as that is
 	// probably closer to the root cause.
-	if _, ok := f.pullErrors[path]; ok {
+	if _, ok := f.tempPullErrors[path]; ok {
 		return
 	}
 
@@ -1791,15 +1800,9 @@ func (f *sendReceiveFolder) newPullError(path string, err error) {
 	// Use "syncing" as opposed to "pulling" as the latter might be used
 	// for errors occurring specificly in the puller routine.
 	errStr := fmt.Sprintln("syncing:", err)
-	f.pullErrors[path] = errStr
+	f.tempPullErrors[path] = errStr
 
-	if oldErr, ok := f.oldPullErrors[path]; ok && oldErr == errStr {
-		l.Debugf("Repeat error on puller (folder %s, item %q): %v", f.Description(), path, err)
-		delete(f.oldPullErrors, path) // Potential repeats are now caught by f.pullErrors itself
-		return
-	}
-
-	l.Infof("Puller (folder %s, item %q): %v", f.Description(), path, err)
+	l.Debugf("%v new error for %v: %v", f, path, err)
 }
 
 func (f *sendReceiveFolder) Errors() []FileError {

lib/model/folder_sendrecv_test.go

@@ -10,11 +10,13 @@ import (
 	"bytes"
 	"context"
 	"crypto/rand"
+	"errors"
 	"io"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 	"time"
 
@@ -95,6 +97,7 @@ func setupSendReceiveFolder(files ...protocol.FileInfo) (*model, *sendReceiveFol
 	model.Supervisor.Stop()
 	f := model.folderRunners[fcfg.ID].(*sendReceiveFolder)
 	f.pullErrors = make(map[string]string)
+	f.tempPullErrors = make(map[string]string)
 	f.ctx = context.Background()
 
 	// Update index
@@ -983,7 +986,7 @@ func TestDeleteBehindSymlink(t *testing.T) {
 	must(t, osutil.RenameOrCopy(fs.CopyRangeMethodStandard, ffs, destFs, file, "file"))
 	must(t, ffs.RemoveAll(link))
 
-	if err := osutil.DebugSymlinkForTestsOnly(destFs.URI(), filepath.Join(ffs.URI(), link)); err != nil {
+	if err := fs.DebugSymlinkForTestsOnly(destFs, ffs, "", link); err != nil {
 		if runtime.GOOS == "windows" {
 			// Probably we require permissions we don't have.
 			t.Skip("Need admin permissions or developer mode to run symlink test on Windows: " + err.Error())
@@ -1087,6 +1090,122 @@ func TestPullDeleteUnscannedDir(t *testing.T) {
 	}
 }
 
+func TestPullCaseOnlyPerformFinish(t *testing.T) {
+	m, f := setupSendReceiveFolder()
+	defer cleanupSRFolder(f, m)
+	ffs := f.Filesystem()
+
+	name := "foo"
+	contents := []byte("contents")
+	must(t, writeFile(ffs, name, contents, 0644))
+	must(t, f.scanSubdirs(nil))
+
+	var cur protocol.FileInfo
+	hasCur := false
+	snap := dbSnapshot(t, m, f.ID)
+	defer snap.Release()
+	snap.WithHave(protocol.LocalDeviceID, func(i protocol.FileIntf) bool {
+		if hasCur {
+			t.Fatal("got more than one file")
+		}
+		cur = i.(protocol.FileInfo)
+		hasCur = true
+		return true
+	})
+	if !hasCur {
+		t.Fatal("file is missing")
+	}
+
+	remote := *(&cur)
+	remote.Version = protocol.Vector{}.Update(device1.Short())
+	remote.Name = strings.ToUpper(cur.Name)
+	temp := fs.TempName(remote.Name)
+	must(t, writeFile(ffs, temp, contents, 0644))
+	scanChan := make(chan string, 1)
+	dbUpdateChan := make(chan dbUpdateJob, 1)
+
+	err := f.performFinish(remote, cur, hasCur, temp, snap, dbUpdateChan, scanChan)
+
+	select {
+	case <-dbUpdateChan: // boring case sensitive filesystem
+		return
+	case <-scanChan:
+		t.Error("no need to scan anything here")
+	default:
+	}
+
+	var caseErr *fs.ErrCaseConflict
+	if !errors.As(err, &caseErr) {
+		t.Error("Expected case conflict error, got", err)
+	}
+}
+
+func TestPullCaseOnlyDir(t *testing.T) {
+	testPullCaseOnlyDirOrSymlink(t, true)
+}
+
+func TestPullCaseOnlySymlink(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("symlinks not supported on windows")
+	}
+	testPullCaseOnlyDirOrSymlink(t, false)
+}
+
+func testPullCaseOnlyDirOrSymlink(t *testing.T, dir bool) {
+	m, f := setupSendReceiveFolder()
+	defer cleanupSRFolder(f, m)
+	ffs := f.Filesystem()
+
+	name := "foo"
+	if dir {
+		must(t, ffs.Mkdir(name, 0777))
+	} else {
+		must(t, ffs.CreateSymlink("target", name))
+	}
+
+	must(t, f.scanSubdirs(nil))
+	var cur protocol.FileInfo
+	hasCur := false
+	snap := dbSnapshot(t, m, f.ID)
+	defer snap.Release()
+	snap.WithHave(protocol.LocalDeviceID, func(i protocol.FileIntf) bool {
+		if hasCur {
+			t.Fatal("got more than one file")
+		}
+		cur = i.(protocol.FileInfo)
+		hasCur = true
+		return true
+	})
+	if !hasCur {
+		t.Fatal("file is missing")
+	}
+
+	scanChan := make(chan string, 1)
+	dbUpdateChan := make(chan dbUpdateJob, 1)
+	remote := *(&cur)
+	remote.Version = protocol.Vector{}.Update(device1.Short())
+	remote.Name = strings.ToUpper(cur.Name)
+
+	if dir {
+		f.handleDir(remote, snap, dbUpdateChan, scanChan)
+	} else {
+		f.handleSymlink(remote, snap, dbUpdateChan, scanChan)
+	}
+
+	select {
+	case <-dbUpdateChan: // boring case sensitive filesystem
+		return
+	case <-scanChan:
+		t.Error("no need to scan anything here")
+	default:
+	}
+	if errStr, ok := f.tempPullErrors[remote.Name]; !ok {
+		t.Error("missing error for", remote.Name)
+	} else if !strings.Contains(errStr, "differs from name") {
+		t.Error("unexpected error", errStr, "for", remote.Name)
+	}
+}
+
 func cleanupSharedPullerState(s *sharedPullerState) {
 	s.mut.Lock()
 	defer s.mut.Unlock()

lib/model/folder_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	"github.com/d4l3k/messagediff"
+
 	"github.com/syncthing/syncthing/lib/config"
 )
 

lib/model/model_test.go

@@ -270,17 +270,15 @@ func BenchmarkRequestOut(b *testing.B) {
 }
 
 func BenchmarkRequestInSingleFile(b *testing.B) {
-	testOs := &fatalOs{b}
-
 	m := setupModel(defaultCfgWrapper)
 	defer cleanupModel(m)
 
 	buf := make([]byte, 128<<10)
 	rand.Read(buf)
-	testOs.RemoveAll("testdata/request")
-	defer testOs.RemoveAll("testdata/request")
-	testOs.MkdirAll("testdata/request/for/a/file/in/a/couple/of/dirs", 0755)
-	ioutil.WriteFile("testdata/request/for/a/file/in/a/couple/of/dirs/128k", buf, 0644)
+	mustRemove(b, defaultFs.RemoveAll("request"))
+	defer func() { mustRemove(b, defaultFs.RemoveAll("request")) }()
+	must(b, defaultFs.MkdirAll("request/for/a/file/in/a/couple/of/dirs", 0755))
+	writeFile(defaultFs, "request/for/a/file/in/a/couple/of/dirs/128k", buf, 0644)
 
 	b.ResetTimer()
 
@@ -294,13 +292,11 @@ func BenchmarkRequestInSingleFile(b *testing.B) {
 }
 
 func TestDeviceRename(t *testing.T) {
-	testOs := &fatalOs{t}
-
 	hello := protocol.HelloResult{
 		ClientName:    "syncthing",
 		ClientVersion: "v0.9.4",
 	}
-	defer testOs.Remove("testdata/tmpconfig.xml")
+	defer func() { mustRemove(t, defaultFs.Remove("tmpconfig.xml")) }()
 
 	rawCfg := config.New(device1)
 	rawCfg.Devices = []config.DeviceConfiguration{
@@ -1447,12 +1443,10 @@ func changeIgnores(t *testing.T, m *model, expected []string) {
 }
 
 func TestIgnores(t *testing.T) {
-	testOs := &fatalOs{t}
-
 	// Assure a clean start state
-	testOs.RemoveAll(filepath.Join("testdata", config.DefaultMarkerName))
-	testOs.MkdirAll(filepath.Join("testdata", config.DefaultMarkerName), 0644)
-	ioutil.WriteFile("testdata/.stignore", []byte(".*\nquux\n"), 0644)
+	mustRemove(t, defaultFs.RemoveAll(config.DefaultMarkerName))
+	mustRemove(t, defaultFs.MkdirAll(config.DefaultMarkerName, 0644))
+	writeFile(defaultFs, ".stignore", []byte(".*\nquux\n"), 0644)
 
 	m := setupModel(defaultCfgWrapper)
 	defer cleanupModel(m)
@@ -1504,18 +1498,16 @@ func TestIgnores(t *testing.T) {
 
 	// Make sure no .stignore file is considered valid
 	defer func() {
-		testOs.Rename("testdata/.stignore.bak", "testdata/.stignore")
+		must(t, defaultFs.Rename(".stignore.bak", ".stignore"))
 	}()
-	testOs.Rename("testdata/.stignore", "testdata/.stignore.bak")
+	must(t, defaultFs.Rename(".stignore", ".stignore.bak"))
 	changeIgnores(t, m, []string{})
 }
 
 func TestEmptyIgnores(t *testing.T) {
-	testOs := &fatalOs{t}
-
 	// Assure a clean start state
-	testOs.RemoveAll(filepath.Join("testdata", config.DefaultMarkerName))
-	testOs.MkdirAll(filepath.Join("testdata", config.DefaultMarkerName), 0644)
+	mustRemove(t, defaultFs.RemoveAll(config.DefaultMarkerName))
+	must(t, defaultFs.MkdirAll(config.DefaultMarkerName, 0644))
 
 	m := setupModel(defaultCfgWrapper)
 	defer cleanupModel(m)
@@ -2117,14 +2109,14 @@ func benchmarkTree(b *testing.B, n1, n2 int) {
 }
 
 func TestIssue3028(t *testing.T) {
-	testOs := &fatalOs{t}
-
 	// Create two files that we'll delete, one with a name that is a prefix of the other.
 
-	must(t, ioutil.WriteFile("testdata/testrm", []byte("Hello"), 0644))
-	defer testOs.Remove("testdata/testrm")
-	must(t, ioutil.WriteFile("testdata/testrm2", []byte("Hello"), 0644))
-	defer testOs.Remove("testdata/testrm2")
+	must(t, writeFile(defaultFs, "testrm", []byte("Hello"), 0644))
+	must(t, writeFile(defaultFs, "testrm2", []byte("Hello"), 0644))
+	defer func() {
+		mustRemove(t, defaultFs.Remove("testrm"))
+		mustRemove(t, defaultFs.Remove("testrm2"))
+	}()
 
 	// Create a model and default folder
 
@@ -2138,8 +2130,8 @@ func TestIssue3028(t *testing.T) {
 
 	// Delete and rescan specifically these two
 
-	testOs.Remove("testdata/testrm")
-	testOs.Remove("testdata/testrm2")
+	must(t, defaultFs.Remove("testrm"))
+	must(t, defaultFs.Remove("testrm2"))
 	m.ScanFolderSubdirs("default", []string{"testrm", "testrm2"})
 
 	// Verify that the number of files decreased by two and the number of
@@ -2601,7 +2593,7 @@ func TestIssue2571(t *testing.T) {
 
 	must(t, testFs.RemoveAll("toLink"))
 
-	must(t, osutil.DebugSymlinkForTestsOnly(filepath.Join(testFs.URI(), "linkTarget"), filepath.Join(testFs.URI(), "toLink")))
+	must(t, fs.DebugSymlinkForTestsOnly(testFs, testFs, "linkTarget", "toLink"))
 
 	m.ScanFolder("default")
 
@@ -2718,13 +2710,10 @@ func TestCustomMarkerName(t *testing.T) {
 		{Name: "dummyfile"},
 	})
 
-	fcfg := config.FolderConfiguration{
-		ID:              "default",
-		Path:            "rwtestfolder",
-		Type:            config.FolderTypeSendReceive,
-		RescanIntervalS: 1,
-		MarkerName:      "myfile",
-	}
+	fcfg := testFolderConfigTmp()
+	fcfg.ID = "default"
+	fcfg.RescanIntervalS = 1
+	fcfg.MarkerName = "myfile"
 	cfg := createTmpWrapper(config.Configuration{
 		Folders: []config.FolderConfiguration{fcfg},
 		Devices: []config.DeviceConfiguration{
@@ -2735,13 +2724,12 @@ func TestCustomMarkerName(t *testing.T) {
 	})
 
 	testOs.RemoveAll(fcfg.Path)
-	defer testOs.RemoveAll(fcfg.Path)
 
 	m := newModel(cfg, myID, "syncthing", "dev", ldb, nil)
 	sub := m.evLogger.Subscribe(events.StateChanged)
 	defer sub.Unsubscribe()
 	m.ServeBackground()
-	defer cleanupModel(m)
+	defer cleanupModelAndRemoveDir(m, fcfg.Path)
 
 	waitForState(t, sub, "default", "folder path missing")
 
@@ -3806,6 +3794,57 @@ func TestBlockListMap(t *testing.T) {
 	}
 }
 
+func TestScanRenameCaseOnly(t *testing.T) {
+	wcfg, fcfg := tmpDefaultWrapper()
+	m := setupModel(wcfg)
+	defer cleanupModel(m)
+
+	ffs := fcfg.Filesystem()
+	name := "foo"
+	must(t, writeFile(ffs, name, []byte("contents"), 0644))
+
+	m.ScanFolders()
+
+	snap := dbSnapshot(t, m, fcfg.ID)
+	defer snap.Release()
+	found := false
+	snap.WithHave(protocol.LocalDeviceID, func(i protocol.FileIntf) bool {
+		if found {
+			t.Fatal("got more than one file")
+		}
+		if i.FileName() != name {
+			t.Fatalf("got file %v, expected %v", i.FileName(), name)
+		}
+		found = true
+		return true
+	})
+	snap.Release()
+
+	upper := strings.ToUpper(name)
+	must(t, ffs.Rename(name, upper))
+	m.ScanFolders()
+
+	snap = dbSnapshot(t, m, fcfg.ID)
+	defer snap.Release()
+	found = false
+	snap.WithHave(protocol.LocalDeviceID, func(i protocol.FileIntf) bool {
+		if i.FileName() == name {
+			if i.IsDeleted() {
+				return true
+			}
+			t.Fatal("renamed file not deleted")
+		}
+		if i.FileName() != upper {
+			t.Fatalf("got file %v, expected %v", i.FileName(), upper)
+		}
+		if found {
+			t.Fatal("got more than the expected files")
+		}
+		found = true
+		return true
+	})
+}
+
 func TestConnectionTerminationOnFolderAdd(t *testing.T) {
 	testConfigChangeClosesConnections(t, false, true, nil, func(cfg config.Wrapper) {
 		fcfg := testFolderConfigTmp()

lib/model/requests_test.go

@@ -323,7 +323,7 @@ func pullInvalidIgnored(t *testing.T, ft config.FolderType) {
 	fc.deleteFile(invDel)
 	fc.addFile(ign, 0644, protocol.FileInfoTypeFile, contents)
 	fc.addFile(ignExisting, 0644, protocol.FileInfoTypeFile, contents)
-	if err := ioutil.WriteFile(filepath.Join(fss.URI(), ignExisting), otherContents, 0644); err != nil {
+	if err := writeFile(fss, ignExisting, otherContents, 0644); err != nil {
 		panic(err)
 	}
 
@@ -465,12 +465,12 @@ func TestIssue4841(t *testing.T) {
 
 func TestRescanIfHaveInvalidContent(t *testing.T) {
 	m, fc, fcfg := setupModelWithConnection()
-	tmpDir := fcfg.Filesystem().URI()
-	defer cleanupModelAndRemoveDir(m, tmpDir)
+	tfs := fcfg.Filesystem()
+	defer cleanupModelAndRemoveDir(m, tfs.URI())
 
 	payload := []byte("hello")
 
-	must(t, ioutil.WriteFile(filepath.Join(tmpDir, "foo"), payload, 0777))
+	must(t, writeFile(tfs, "foo", payload, 0777))
 
 	received := make(chan []protocol.FileInfo)
 	fc.mut.Lock()
@@ -511,7 +511,7 @@ func TestRescanIfHaveInvalidContent(t *testing.T) {
 	payload = []byte("bye")
 	buf = make([]byte, len(payload))
 
-	must(t, ioutil.WriteFile(filepath.Join(tmpDir, "foo"), payload, 0777))
+	must(t, writeFile(tfs, "foo", payload, 0777))
 
 	_, err = m.Request(device1, "default", "foo", int32(len(payload)), 0, f.Blocks[0].Hash, f.Blocks[0].WeakHash, false)
 	if err == nil {
@@ -1051,7 +1051,7 @@ func TestIgnoreDeleteUnignore(t *testing.T) {
 	}
 	fc.mut.Unlock()
 
-	if err := ioutil.WriteFile(filepath.Join(fss.URI(), file), contents, 0644); err != nil {
+	if err := writeFile(fss, file, contents, 0644); err != nil {
 		panic(err)
 	}
 	m.ScanFolders()

lib/model/testos_test.go

@@ -9,6 +9,8 @@ package model
 import (
 	"os"
 	"time"
+
+	"github.com/syncthing/syncthing/lib/fs"
 )
 
 // fatal is the required common interface between *testing.B and *testing.T
@@ -28,6 +30,13 @@ func must(f fatal, err error) {
 	}
 }
 
+func mustRemove(f fatal, err error) {
+	f.Helper()
+	if err != nil && !fs.IsNotExist(err) {
+		f.Fatal(err)
+	}
+}
+
 func (f *fatalOs) Chmod(name string, mode os.FileMode) {
 	f.Helper()
 	must(f, os.Chmod(name, mode))

lib/model/testutils_test.go

@@ -36,9 +36,8 @@ func init() {
 	device1, _ = protocol.DeviceIDFromString("AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR")
 	device2, _ = protocol.DeviceIDFromString("GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY")
 
-	defaultFs = fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata")
-
 	defaultFolderConfig = testFolderConfig("testdata")
+	defaultFs = defaultFolderConfig.Filesystem()
 
 	defaultCfgWrapper = createTmpWrapper(config.New(myID))
 	_, _ = defaultCfgWrapper.SetDevice(config.NewDeviceConfiguration(device1, "device1"))