lib/ignore: Add central check for internal files, used in scanning, pulling and requests

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

lib/ignore/ignore.go

@@ -401,3 +401,20 @@ func parseIgnoreFile(fd io.Reader, currentFile string, modtimes map[string]time.
 
 	return patterns, nil
 }
+
+// IsInternal returns true if the file, as a path relative to the folder
+// root, represents an internal file that should always be ignored. The file
+// path must be clean (i.e., in canonical shortest form).
+func IsInternal(file string) bool {
+	internals := []string{".stfolder", ".stignore", ".stversions"}
+	pathSep := string(os.PathSeparator)
+	for _, internal := range internals {
+		if file == internal {
+			return true
+		}
+		if strings.HasPrefix(file, internal+pathSep) {
+			return true
+		}
+	}
+	return false
+}

lib/ignore/ignore_test.go

@@ -812,3 +812,34 @@ func TestGobwasGlobIssue18(t *testing.T) {
 		}
 	}
 }
+
+func TestIsInternal(t *testing.T) {
+	cases := []struct {
+		file     string
+		internal bool
+	}{
+		{".stfolder", true},
+		{".stignore", true},
+		{".stversions", true},
+		{".stfolder/foo", true},
+		{".stignore/foo", true},
+		{".stversions/foo", true},
+
+		{".stfolderfoo", false},
+		{".stignorefoo", false},
+		{".stversionsfoo", false},
+		{"foo.stfolder", false},
+		{"foo.stignore", false},
+		{"foo.stversions", false},
+		{"foo/.stfolder", false},
+		{"foo/.stignore", false},
+		{"foo/.stversions", false},
+	}
+
+	for _, tc := range cases {
+		res := IsInternal(filepath.FromSlash(tc.file))
+		if res != tc.internal {
+			t.Errorf("Unexpected result: IsInteral(%q): %v should be %v", tc.file, res, tc.internal)
+		}
+	}
+}

lib/model/model.go

@@ -1099,16 +1099,18 @@ func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset
 		return protocol.ErrInvalid
 	}
 
-	if folderIgnores != nil {
-		// "rn" becomes the relative name of the file within the folder. This is
-		// different than the original "name" parameter in that it's been
-		// cleaned from any possible funny business.
-		if rn, err := filepath.Rel(folderPath, fn); err != nil {
-			return err
-		} else if folderIgnores.Match(rn).IsIgnored() {
-			l.Debugf("%v REQ(in) for ignored file: %s: %q / %q o=%d s=%d", m, deviceID, folder, name, offset, len(buf))
+	// Having passed the rootedJoinedPath check above, we know "name" is
+	// acceptable relative to "folderPath" and in canonical form, so we can
+	// trust it.
+
+	if ignore.IsInternal(name) {
+		l.Debugf("%v REQ(in) for internal file: %s: %q / %q o=%d s=%d", m, deviceID, folder, name, offset, len(buf))
 		return protocol.ErrNoSuchFile
 	}
+
+	if folderIgnores.Match(name).IsIgnored() {
+		l.Debugf("%v REQ(in) for ignored file: %s: %q / %q o=%d s=%d", m, deviceID, folder, name, offset, len(buf))
+		return protocol.ErrNoSuchFile
 	}
 
 	if info, err := osutil.Lstat(fn); err == nil && info.Mode()&os.ModeSymlink != 0 {
@@ -2483,7 +2485,7 @@ func unifySubs(dirs []string, exists func(dir string) bool) []string {
 func trimUntilParentKnown(dirs []string, exists func(dir string) bool) []string {
 	var subs []string
 	for _, sub := range dirs {
-		for sub != "" && sub != ".stfolder" && sub != ".stignore" {
+		for sub != "" && !ignore.IsInternal(sub) {
 			sub = filepath.Clean(sub)
 			parent := filepath.Dir(sub)
 			if parent == "." || exists(parent) {
@@ -2545,6 +2547,9 @@ func shouldIgnore(file db.FileIntf, matcher *ignore.Matcher, ignoreDelete bool)
 		// deleted files.
 		return true
 
+	case ignore.IsInternal(file.FileName()):
+		return true
+
 	case matcher.Match(file.FileName()).IsIgnored():
 		return true
 	}

lib/scanner/walk.go

@@ -242,13 +242,12 @@ func (w *walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 		}
 
 		info, err = w.Lstater.Lstat(absPath)
-		// An error here would be weird as we've already gotten to this point, but act on it ninetheless
+		// An error here would be weird as we've already gotten to this point, but act on it nonetheless
 		if err != nil {
 			return skip
 		}
 
 		if w.TempNamer.IsTemporary(relPath) {
-			// A temporary file
 			l.Debugln("temporary:", relPath)
 			if info.Mode().IsRegular() && info.ModTime().Add(w.TempLifetime).Before(now) {
 				os.Remove(absPath)
@@ -257,10 +256,13 @@ func (w *walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 			return nil
 		}
 
-		if sn := filepath.Base(relPath); sn == ".stignore" || sn == ".stfolder" ||
-			strings.HasPrefix(relPath, ".stversions") || (w.Matcher != nil && w.Matcher.Match(relPath).IsIgnored()) {
-			// An ignored file
-			l.Debugln("ignored:", relPath)
+		if ignore.IsInternal(relPath) {
+			l.Debugln("ignored (internal):", relPath)
+			return skip
+		}
+
+		if w.Matcher.Match(relPath).IsIgnored() {
+			l.Debugln("ignored (patterns):", relPath)
 			return skip
 		}