all: Add invalid/ignored files to global list, announce to peers (fixes #623)

This lets us determine accurate completion status for remote peers when they
have ignored files.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4460

lib/model/model.go

@@ -595,7 +595,6 @@ type FolderCompletion struct {
 func (m *Model) Completion(device protocol.DeviceID, folder string) FolderCompletion {
 	m.fmut.RLock()
 	rf, ok := m.folderFiles[folder]
-	ignores := m.folderIgnores[folder]
 	m.fmut.RUnlock()
 	if !ok {
 		return FolderCompletion{} // Folder doesn't exist, so we hardly have any of it
@@ -615,10 +614,6 @@ func (m *Model) Completion(device protocol.DeviceID, folder string) FolderComple
 
 	var need, fileNeed, downloaded, deletes int64
 	rf.WithNeedTruncated(device, func(f db.FileIntf) bool {
-		if ignores.Match(f.FileName()).IsIgnored() {
-			return true
-		}
-
 		ft := f.(db.FileInfoTruncated)
 
 		// If the file is deleted, we account it only in the deleted column.
@@ -703,10 +698,9 @@ func (m *Model) NeedSize(folder string) db.Counts {
 
 	var result db.Counts
 	if rf, ok := m.folderFiles[folder]; ok {
-		ignores := m.folderIgnores[folder]
 		cfg := m.folderCfgs[folder]
 		rf.WithNeedTruncated(protocol.LocalDeviceID, func(f db.FileIntf) bool {
-			if shouldIgnore(f, ignores, cfg.IgnoreDelete) {
+			if cfg.IgnoreDelete && f.IsDeleted() {
 				return true
 			}
 
@@ -767,10 +761,9 @@ func (m *Model) NeedFolderFiles(folder string, page, perpage int) ([]db.FileInfo
 	}
 
 	rest = make([]db.FileInfoTruncated, 0, perpage)
-	ignores := m.folderIgnores[folder]
 	cfg := m.folderCfgs[folder]
 	rf.WithNeedTruncated(protocol.LocalDeviceID, func(f db.FileIntf) bool {
-		if shouldIgnore(f, ignores, cfg.IgnoreDelete) {
+		if cfg.IgnoreDelete && f.IsDeleted() {
 			return true
 		}
 
@@ -1721,6 +1714,13 @@ func (m *Model) diskChangeDetected(folderCfg config.FolderConfiguration, files [
 		objType := "file"
 		action := "modified"
 
+		switch {
+		case file.IsDeleted():
+			action = "deleted"
+
+		case file.Invalid:
+			action = "ignored" // invalidated seems not very user friendly
+
 		// If our local vector is version 1 AND it is the only version
 		// vector so far seen for this file then it is a new file.  Else if
 		// it is > 1 it's not new, and if it is 1 but another shortId
@@ -1728,16 +1728,13 @@ func (m *Model) diskChangeDetected(folderCfg config.FolderConfiguration, files [
 		// so the file is still not new but modified by us. Only if it is
 		// truly new do we change this to 'added', else we leave it as
 		// 'modified'.
-		if len(file.Version.Counters) == 1 && file.Version.Counters[0].Value == 1 {
+		case len(file.Version.Counters) == 1 && file.Version.Counters[0].Value == 1:
 			action = "added"
 		}
 
 		if file.IsDirectory() {
 			objType = "dir"
 		}
-		if file.IsDeleted() {
-			action = "deleted"
-		}
 
 		// Two different events can be fired here based on what EventType is passed into function
 		events.Default.Log(typeOfEvent, map[string]string{
@@ -1971,18 +1968,7 @@ func (m *Model) internalScanFolderSubdirs(ctx context.Context, folder string, su
 			case !f.IsInvalid() && ignores.Match(f.Name).IsIgnored():
 				// File was valid at last pass but has been ignored. Set invalid bit.
 				l.Debugln("setting invalid bit on ignored", f)
-				nf := protocol.FileInfo{
-					Name:          f.Name,
-					Type:          f.Type,
-					Size:          f.Size,
-					ModifiedS:     f.ModifiedS,
-					ModifiedNs:    f.ModifiedNs,
-					ModifiedBy:    m.id.Short(),
-					Permissions:   f.Permissions,
-					NoPermissions: f.NoPermissions,
-					Invalid:       true,
-					Version:       f.Version, // The file is still the same, so don't bump version
-				}
+				nf := f.ConvertToInvalidFileInfo(m.id.Short())
 				batch = append(batch, nf)
 				batchSizeBytes += nf.ProtoSize()
 
@@ -2167,6 +2153,10 @@ func (m *Model) Override(folder string) {
 		}
 
 		have, ok := fs.Get(protocol.LocalDeviceID, need.Name)
+		// Don't override invalid (e.g. ignored) files
+		if ok && have.Invalid {
+			return true
+		}
 		if !ok || have.Name != need.Name {
 			// We are missing the file
 			need.Deleted = true

lib/model/requests_test.go

@@ -26,10 +26,9 @@ func TestRequestSimple(t *testing.T) {
 	// Verify that the model performs a request and creates a file based on
 	// an incoming index update.
 
-	defer os.RemoveAll("_tmpfolder")
-
-	m, fc := setupModelWithConnection()
+	m, fc, tmpFolder := setupModelWithConnection()
 	defer m.Stop()
+	defer os.RemoveAll(tmpFolder)
 
 	// We listen for incoming index updates and trigger when we see one for
 	// the expected test file.
@@ -52,7 +51,7 @@ func TestRequestSimple(t *testing.T) {
 	<-done
 
 	// Verify the contents
-	bs, err := ioutil.ReadFile("_tmpfolder/testfile")
+	bs, err := ioutil.ReadFile(filepath.Join(tmpFolder, "testfile"))
 	if err != nil {
 		t.Error("File did not sync correctly:", err)
 		return
@@ -70,10 +69,9 @@ func TestSymlinkTraversalRead(t *testing.T) {
 		return
 	}
 
-	defer os.RemoveAll("_tmpfolder")
-
-	m, fc := setupModelWithConnection()
+	m, fc, tmpFolder := setupModelWithConnection()
 	defer m.Stop()
+	defer os.RemoveAll(tmpFolder)
 
 	// We listen for incoming index updates and trigger when we see one for
 	// the expected test file.
@@ -111,10 +109,9 @@ func TestSymlinkTraversalWrite(t *testing.T) {
 		return
 	}
 
-	defer os.RemoveAll("_tmpfolder")
-
-	m, fc := setupModelWithConnection()
+	m, fc, tmpFolder := setupModelWithConnection()
 	defer m.Stop()
+	defer os.RemoveAll(tmpFolder)
 
 	// We listen for incoming index updates and trigger when we see one for
 	// the expected names.
@@ -170,22 +167,25 @@ func TestSymlinkTraversalWrite(t *testing.T) {
 }
 
 func TestRequestCreateTmpSymlink(t *testing.T) {
-	// Verify that the model performs a request and creates a file based on
-	// an incoming index update.
+	// Test that an update for a temporary file is invalidated
 
-	defer os.RemoveAll("_tmpfolder")
-
-	m, fc := setupModelWithConnection()
+	m, fc, tmpFolder := setupModelWithConnection()
 	defer m.Stop()
+	defer os.RemoveAll(tmpFolder)
 
 	// We listen for incoming index updates and trigger when we see one for
 	// the expected test file.
-	badIdx := make(chan string)
+	goodIdx := make(chan struct{})
+	name := fs.TempName("testlink")
 	fc.mut.Lock()
 	fc.indexFn = func(folder string, fs []protocol.FileInfo) {
 		for _, f := range fs {
-			if f.Name == ".syncthing.testlink.tmp" {
-				badIdx <- f.Name
+			if f.Name == name {
+				if f.Invalid {
+					goodIdx <- struct{}{}
+				} else {
+					t.Fatal("Received index with non-invalid temporary file")
+				}
 				return
 			}
 		}
@@ -193,16 +193,13 @@ func TestRequestCreateTmpSymlink(t *testing.T) {
 	fc.mut.Unlock()
 
 	// Send an update for the test file, wait for it to sync and be reported back.
-	fc.addFile(".syncthing.testlink.tmp", 0644, protocol.FileInfoTypeSymlink, []byte(".."))
+	fc.addFile(name, 0644, protocol.FileInfoTypeSymlink, []byte(".."))
 	fc.sendIndexUpdate()
 
 	select {
-	case name := <-badIdx:
-		t.Fatal("Should not have sent the index entry for", name)
+	case <-goodIdx:
 	case <-time.After(3 * time.Second):
-		// Unfortunately not much else to trigger on here. The puller sleep
-		// interval is 1s so if we didn't get any requests within two
-		// iterations we should be fine.
+		t.Fatal("Timed out without index entry being sent")
 	}
 }
 
@@ -214,8 +211,12 @@ func TestRequestVersioningSymlinkAttack(t *testing.T) {
 	// Sets up a folder with trashcan versioning and tries to use a
 	// deleted symlink to escape
 
+	tmpFolder, err := ioutil.TempDir(".", "_request-")
+	if err != nil {
+		panic("Failed to create temporary testing dir")
+	}
 	cfg := defaultConfig.RawCopy()
-	cfg.Folders[0] = config.NewFolderConfiguration("default", fs.FilesystemTypeBasic, "_tmpfolder")
+	cfg.Folders[0] = config.NewFolderConfiguration("default", fs.FilesystemTypeBasic, tmpFolder)
 	cfg.Folders[0].Devices = []config.FolderDeviceConfiguration{
 		{DeviceID: device1},
 		{DeviceID: device2},
@@ -232,7 +233,7 @@ func TestRequestVersioningSymlinkAttack(t *testing.T) {
 	m.StartFolder("default")
 	defer m.Stop()
 
-	defer os.RemoveAll("_tmpfolder")
+	defer os.RemoveAll(tmpFolder)
 
 	fc := addFakeConn(m, device2)
 	fc.folder = "default"
@@ -285,9 +286,13 @@ func TestRequestVersioningSymlinkAttack(t *testing.T) {
 	}
 }
 
-func setupModelWithConnection() (*Model, *fakeConnection) {
+func setupModelWithConnection() (*Model, *fakeConnection, string) {
+	tmpFolder, err := ioutil.TempDir(".", "_request-")
+	if err != nil {
+		panic("Failed to create temporary testing dir")
+	}
 	cfg := defaultConfig.RawCopy()
-	cfg.Folders[0] = config.NewFolderConfiguration("default", fs.FilesystemTypeBasic, "_tmpfolder")
+	cfg.Folders[0] = config.NewFolderConfiguration("default", fs.FilesystemTypeBasic, tmpFolder)
 	cfg.Folders[0].Devices = []config.FolderDeviceConfiguration{
 		{DeviceID: device1},
 		{DeviceID: device2},
@@ -303,5 +308,5 @@ func setupModelWithConnection() (*Model, *fakeConnection) {
 	fc := addFakeConn(m, device2)
 	fc.folder = "default"
 
-	return m, fc
+	return m, fc, tmpFolder
 }

lib/model/rwfolder.go

@@ -69,6 +69,7 @@ const (
 	dbUpdateDeleteFile
 	dbUpdateShortcutFile
 	dbUpdateHandleSymlink
+	dbUpdateInvalidate
 )
 
 const (
@@ -234,7 +235,9 @@ func (f *sendReceiveFolder) pull(prevSeq int64, prevIgnoreHash string) (curSeq i
 	f.model.fmut.RUnlock()
 
 	curSeq = prevSeq
-	if curIgnoreHash = curIgnores.Hash(); curIgnoreHash != prevIgnoreHash {
+	curIgnoreHash = curIgnores.Hash()
+	ignoresChanged := curIgnoreHash != prevIgnoreHash
+	if ignoresChanged {
 		// The ignore patterns have changed. We need to re-evaluate if
 		// there are files we need now that were ignored before.
 		l.Debugln(f, "ignore patterns have changed, resetting curSeq")
@@ -263,7 +266,7 @@ func (f *sendReceiveFolder) pull(prevSeq int64, prevIgnoreHash string) (curSeq i
 	for {
 		tries++
 
-		changed = f.pullerIteration(curIgnores)
+		changed := f.pullerIteration(curIgnores, ignoresChanged)
 		l.Debugln(f, "changed", changed)
 
 		if changed == 0 {
@@ -317,7 +320,7 @@ func (f *sendReceiveFolder) pull(prevSeq int64, prevIgnoreHash string) (curSeq i
 // returns the number items that should have been synced (even those that
 // might have failed). One puller iteration handles all files currently
 // flagged as needed in the folder.
-func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher) int {
+func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher, ignoresChanged bool) int {
 	pullChan := make(chan pullBlockState)
 	copyChan := make(chan copyBlocksState)
 	finisherChan := make(chan *sharedPullerState)
@@ -374,15 +377,21 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher) int {
 	// (directories, symlinks and deletes) goes into the "process directly"
 	// pile.
 
-	folderFiles.WithNeed(protocol.LocalDeviceID, func(intf db.FileIntf) bool {
-		if shouldIgnore(intf, ignores, f.IgnoreDelete) {
+	// Don't iterate over invalid/ignored files unless ignores have changed
+	iterate := folderFiles.WithNeed
+	if ignoresChanged {
+		iterate = folderFiles.WithNeedOrInvalid
+	}
+
+	iterate(protocol.LocalDeviceID, func(intf db.FileIntf) bool {
+		if f.IgnoreDelete && intf.IsDeleted() {
 			return true
 		}
 
-		if err := fileValid(intf); err != nil {
-			// The file isn't valid so we can't process it. Pretend that we
-			// tried and set the error for the file.
-			f.newError("need", intf.FileName(), err)
+		// If filename isn't valid, we can terminate early with an appropriate error.
+		// in case it is deleted, we don't care about the filename, so don't complain.
+		if !intf.IsDeleted() && runtime.GOOS == "windows" && fs.WindowsInvalidFilename(intf.FileName()) {
+			f.newError("need", intf.FileName(), fs.ErrInvalidFilename)
 			changed++
 			return true
 		}
@@ -390,6 +399,11 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher) int {
 		file := intf.(protocol.FileInfo)
 
 		switch {
+		case ignores.ShouldIgnore(file.Name):
+			file.Invalidate(f.model.id.Short())
+			l.Debugln(f, "Handling ignored file", file)
+			f.dbUpdates <- dbUpdateJob{file, dbUpdateInvalidate}
+
 		case file.IsDeleted():
 			processDirectly = append(processDirectly, file)
 			changed++
@@ -403,9 +417,15 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher) int {
 				if f.model.ConnectedTo(dev) {
 					f.queue.Push(file.Name, file.Size, file.ModTime())
 					changed++
-					break
+					return true
 				}
 			}
+			l.Debugln(f, "Needed file is unavailable", file)
+
+		case runtime.GOOS == "windows" && file.IsSymlink():
+			file.Invalidate(f.model.id.Short())
+			l.Debugln(f, "Invalidating symlink (unsupported)", file.Name)
+			f.dbUpdates <- dbUpdateJob{file, dbUpdateInvalidate}
 
 		default:
 			// Directories, symlinks
@@ -449,7 +469,7 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher) int {
 				// number, hence the deletion coming in again as part of
 				// WithNeed, furthermore, the file can simply be of the wrong
 				// type if we haven't yet managed to pull it.
-				if ok && !df.IsDeleted() && !df.IsSymlink() && !df.IsDirectory() {
+				if ok && !df.IsDeleted() && !df.IsSymlink() && !df.IsDirectory() && !df.IsInvalid() {
 					// Put files into buckets per first hash
 					key := string(df.Blocks[0].Hash)
 					buckets[key] = append(buckets[key], df)
@@ -457,11 +477,11 @@ func (f *sendReceiveFolder) pullerIteration(ignores *ignore.Matcher) int {
 			}
 
 		case fi.IsDirectory() && !fi.IsSymlink():
-			l.Debugln("Handling directory", fi.Name)
+			l.Debugln(f, "Handling directory", fi.Name)
 			f.handleDir(fi)
 
 		case fi.IsSymlink():
-			l.Debugln("Handling symlink", fi.Name)
+			l.Debugln(f, "Handling symlink", fi.Name)
 			f.handleSymlink(fi)
 
 		default:
@@ -566,13 +586,13 @@ nextFile:
 	doneWg.Wait()
 
 	for _, file := range fileDeletions {
-		l.Debugln("Deleting file", file.Name)
+		l.Debugln(f, "Deleting file", file.Name)
 		f.deleteFile(file)
 	}
 
 	for i := range dirDeletions {
 		dir := dirDeletions[len(dirDeletions)-i-1]
-		l.Debugln("Deleting dir", dir.Name)
+		l.Debugln(f, "Deleting dir", dir.Name)
 		f.deleteDir(dir, ignores)
 	}
 
@@ -1516,8 +1536,9 @@ func (f *sendReceiveFolder) dbUpdaterRoutine() {
 				changedDirs[filepath.Dir(job.file.Name)] = struct{}{}
 			case dbUpdateHandleDir:
 				changedDirs[job.file.Name] = struct{}{}
-			case dbUpdateHandleSymlink:
-				// fsyncing symlinks is only supported by MacOS, ignore
+			case dbUpdateHandleSymlink, dbUpdateInvalidate:
+				// fsyncing symlinks is only supported by MacOS
+				// and invalidated files are db only changes -> no sync
 			}
 
 			if job.file.IsInvalid() || (job.file.IsDirectory() && !job.file.IsSymlink()) {
@@ -1722,47 +1743,6 @@ func (l fileErrorList) Swap(a, b int) {
 	l[a], l[b] = l[b], l[a]
 }
 
-// fileValid returns nil when the file is valid for processing, or an error if it's not
-func fileValid(file db.FileIntf) error {
-	switch {
-	case file.IsDeleted():
-		// We don't care about file validity if we're not supposed to have it
-		return nil
-
-	case runtime.GOOS == "windows" && file.IsSymlink():
-		return errSymlinksUnsupported
-
-	case runtime.GOOS == "windows" && windowsInvalidFilename(file.FileName()):
-		return fs.ErrInvalidFilename
-	}
-
-	return nil
-}
-
-var windowsDisallowedCharacters = string([]rune{
-	'<', '>', ':', '"', '|', '?', '*',
-	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
-	11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
-	21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
-	31,
-})
-
-func windowsInvalidFilename(name string) bool {
-	// None of the path components should end in space
-	for _, part := range strings.Split(name, `\`) {
-		if len(part) == 0 {
-			continue
-		}
-		if part[len(part)-1] == ' ' {
-			// Names ending in space are not valid.
-			return true
-		}
-	}
-
-	// The path must not contain any disallowed characters
-	return strings.ContainsAny(name, windowsDisallowedCharacters)
-}
-
 // byComponentCount sorts by the number of path components in Name, that is
 // "x/y" sorts before "foo/bar/baz".
 type byComponentCount []protocol.FileInfo