lib/fs: Evaluate root when watching not on fs creation (fixes #5043) (#5105)

lib/fs/basicfs.go

@@ -25,8 +25,7 @@ var (
 // The BasicFilesystem implements all aspects by delegating to package os.
 // All paths are relative to the root and cannot (should not) escape the root directory.
 type BasicFilesystem struct {
-	root                 string
-	rootSymlinkEvaluated string
+	root string
 }
 
 func newBasicFilesystem(root string) *BasicFilesystem {
@@ -36,7 +35,8 @@ func newBasicFilesystem(root string) *BasicFilesystem {
 	// C:\somedir\ ->  C:\somedir\\   ->  C:\somedir
 	// This way in the tests, we get away without OS specific separators
 	// in the test configs.
-	root = filepath.Dir(root + string(filepath.Separator))
+	sep := string(filepath.Separator)
+	root = filepath.Dir(root + sep)
 
 	// Attempt tilde expansion; leave unchanged in case of error
 	if path, err := ExpandTilde(root); err == nil {
@@ -53,34 +53,17 @@ func newBasicFilesystem(root string) *BasicFilesystem {
 		}
 	}
 
-	rootSymlinkEvaluated, err := filepath.EvalSymlinks(root)
-	if err != nil {
-		rootSymlinkEvaluated = root
-	}
-
-	return &BasicFilesystem{
-		root:                 adjustRoot(root),
-		rootSymlinkEvaluated: adjustRoot(rootSymlinkEvaluated),
-	}
-}
-
-func adjustRoot(root string) string {
 	// Attempt to enable long filename support on Windows. We may still not
 	// have an absolute path here if the previous steps failed.
 	if runtime.GOOS == "windows" {
-		if filepath.IsAbs(root) && !strings.HasPrefix(root, `\\`) {
-			root = `\\?\` + root
-		}
-		return root
+		root = longFilenameSupport(root)
+	} else if !strings.HasSuffix(root, sep) {
+		// If we're not on Windows, we want the path to end with a slash to
+		// penetrate symlinks. On Windows, paths must not end with a slash.
+		root = root + sep
 	}
 
-	// If we're not on Windows, we want the path to end with a slash to
-	// penetrate symlinks. On Windows, paths must not end with a slash.
-	if root[len(root)-1] != filepath.Separator {
-		root = root + string(filepath.Separator)
-	}
-
-	return root
+	return &BasicFilesystem{root}
 }
 
 // rooted expands the relative path to the full path that is then used with os
@@ -91,14 +74,6 @@ func (f *BasicFilesystem) rooted(rel string) (string, error) {
 	return rooted(rel, f.root)
 }
 
-// rootedSymlinkEvaluated does the same as rooted, but the returned path will not
-// contain any symlinks.  package. If the relative path somehow causes the final
-// path to escape the root directory, this returns an error, to prevent accessing
-// files that are not in the shared directory.
-func (f *BasicFilesystem) rootedSymlinkEvaluated(rel string) (string, error) {
-	return rooted(rel, f.rootSymlinkEvaluated)
-}
-
 func rooted(rel, root string) (string, error) {
 	// The root must not be empty.
 	if root == "" {
@@ -138,10 +113,6 @@ func (f *BasicFilesystem) unrooted(path string) string {
 	return rel(path, f.root)
 }
 
-func (f *BasicFilesystem) unrootedSymlinkEvaluated(path string) string {
-	return rel(path, f.rootSymlinkEvaluated)
-}
-
 func rel(path, prefix string) string {
 	return strings.TrimPrefix(strings.TrimPrefix(path, prefix), string(PathSeparator))
 }
@@ -372,3 +343,13 @@ func (e fsFileInfo) IsRegular() bool {
 	// Must use fsFileInfo.Mode() because it may apply magic.
 	return e.Mode()&ModeType == 0
 }
+
+// longFilenameSupport adds the necessary prefix to the path to enable long
+// filename support on windows if necessary.
+// This does NOT check the current system, i.e. will also take effect on unix paths.
+func longFilenameSupport(path string) string {
+	if filepath.IsAbs(path) && !strings.HasPrefix(path, `\\`) {
+		return `\\?\` + path
+	}
+	return path
+}

lib/fs/basicfs_test.go

@@ -12,6 +12,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"sort"
+	"strings"
 	"testing"
 	"time"
 )
@@ -449,3 +450,35 @@ func TestRooted(t *testing.T) {
 		}
 	}
 }
+
+func TestNewBasicFilesystem(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("non-windows root paths")
+	}
+
+	testCases := []struct {
+		input        string
+		expectedRoot string
+		expectedURI  string
+	}{
+		{"/foo/bar/baz", "/foo/bar/baz/", "/foo/bar/baz/"},
+		{"/foo/bar/baz/", "/foo/bar/baz/", "/foo/bar/baz/"},
+		{"", "/", "/"},
+		{"/", "/", "/"},
+	}
+
+	for _, testCase := range testCases {
+		fs := newBasicFilesystem(testCase.input)
+		if fs.root != testCase.expectedRoot {
+			t.Errorf("root %q != %q", fs.root, testCase.expectedRoot)
+		}
+		if fs.URI() != testCase.expectedURI {
+			t.Errorf("uri %q != %q", fs.URI(), testCase.expectedURI)
+		}
+	}
+
+	fs := newBasicFilesystem("relative/path")
+	if fs.root == "relative/path" || !strings.HasPrefix(fs.root, string(PathSeparator)) {
+		t.Errorf(`newBasicFilesystem("relative/path").root == %q, expected absolutification`, fs.root)
+	}
+}

lib/fs/basicfs_watch.go

@@ -13,6 +13,7 @@ import (
 	"errors"
 	"fmt"
 	"path/filepath"
+	"runtime"
 	"strings"
 
 	"github.com/syncthing/notify"
@@ -24,7 +25,15 @@ import (
 var backendBuffer = 500
 
 func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context, ignorePerms bool) (<-chan Event, error) {
-	absName, err := f.rootedSymlinkEvaluated(name)
+	evalRoot, err := filepath.EvalSymlinks(f.root)
+	if err != nil {
+		return nil, err
+	}
+	if runtime.GOOS == "windows" {
+		evalRoot = longFilenameSupport(evalRoot)
+	}
+
+	absName, err := rooted(name, evalRoot)
 	if err != nil {
 		return nil, err
 	}
@@ -39,7 +48,7 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context
 
 	if ignore.SkipIgnoredDirs() {
 		absShouldIgnore := func(absPath string) bool {
-			return ignore.ShouldIgnore(f.unrootedChecked(absPath))
+			return ignore.ShouldIgnore(f.unrootedChecked(absPath, evalRoot))
 		}
 		err = notify.WatchWithFilter(filepath.Join(absName, "..."), backendChan, absShouldIgnore, eventMask)
 	} else {
@@ -53,12 +62,12 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context
 		return nil, err
 	}
 
-	go f.watchLoop(name, backendChan, outChan, ignore, ctx)
+	go f.watchLoop(name, evalRoot, backendChan, outChan, ignore, ctx)
 
 	return outChan, nil
 }
 
-func (f *BasicFilesystem) watchLoop(name string, backendChan chan notify.EventInfo, outChan chan<- Event, ignore Matcher, ctx context.Context) {
+func (f *BasicFilesystem) watchLoop(name, evalRoot string, backendChan chan notify.EventInfo, outChan chan<- Event, ignore Matcher, ctx context.Context) {
 	for {
 		// Detect channel overflow
 		if len(backendChan) == backendBuffer {
@@ -77,7 +86,7 @@ func (f *BasicFilesystem) watchLoop(name string, backendChan chan notify.EventIn
 
 		select {
 		case ev := <-backendChan:
-			relPath := f.unrootedChecked(ev.Path())
+			relPath := f.unrootedChecked(ev.Path(), evalRoot)
 			if ignore.ShouldIgnore(relPath) {
 				l.Debugln(f.Type(), f.URI(), "Watch: Ignoring", relPath)
 				continue
@@ -110,12 +119,13 @@ func (f *BasicFilesystem) eventType(notifyType notify.Event) EventType {
 // unrooted). It panics if the given path is not a subpath and handles the
 // special case when the given path is the folder root without a trailing
 // pathseparator.
-func (f *BasicFilesystem) unrootedChecked(absPath string) string {
-	if absPath+string(PathSeparator) == f.rootSymlinkEvaluated {
+func (f *BasicFilesystem) unrootedChecked(absPath, root string) string {
+	absPath = f.resolveWin83(absPath)
+	if absPath+string(PathSeparator) == root {
 		return "."
 	}
-	if !strings.HasPrefix(absPath, f.rootSymlinkEvaluated) {
-		panic(fmt.Sprintf("bug: Notify backend is processing a change outside of the filesystem root: root==%v, rootSymEval==%v, path==%v", f.root, f.rootSymlinkEvaluated, absPath))
+	if !strings.HasPrefix(absPath, root) {
+		panic(fmt.Sprintf("bug: Notify backend is processing a change outside of the filesystem root: f.root==%v, root==%v, path==%v", f.root, root, absPath))
 	}
-	return f.unrootedSymlinkEvaluated(f.resolveWin83(absPath))
+	return rel(absPath, root)
 }

lib/fs/basicfs_watch_test.go

@@ -33,11 +33,17 @@ func TestMain(m *testing.M) {
 	if err != nil {
 		panic("Cannot get absolute path to working dir")
 	}
+
 	dir, err = filepath.EvalSymlinks(dir)
 	if err != nil {
 		panic("Cannot get real path to working dir")
 	}
+
 	testDirAbs = filepath.Join(dir, testDir)
+	if runtime.GOOS == "windows" {
+		testDirAbs = longFilenameSupport(testDirAbs)
+	}
+
 	testFs = NewFilesystem(FilesystemTypeBasic, testDirAbs)
 
 	backendBuffer = 10
@@ -154,7 +160,7 @@ func TestWatchOutside(t *testing.T) {
 			}
 			cancel()
 		}()
-		fs.watchLoop(".", backendChan, outChan, fakeMatcher{}, ctx)
+		fs.watchLoop(".", testDirAbs, backendChan, outChan, fakeMatcher{}, ctx)
 	}()
 
 	backendChan <- fakeEventInfo(filepath.Join(filepath.Dir(testDirAbs), "outside"))
@@ -177,7 +183,7 @@ func TestWatchSubpath(t *testing.T) {
 	fs := newBasicFilesystem(testDirAbs)
 
 	abs, _ := fs.rooted("sub")
-	go fs.watchLoop("sub", backendChan, outChan, fakeMatcher{}, ctx)
+	go fs.watchLoop("sub", testDirAbs, backendChan, outChan, fakeMatcher{}, ctx)
 
 	backendChan <- fakeEventInfo(filepath.Join(abs, "file"))
 
@@ -291,7 +297,7 @@ func TestUnrootedChecked(t *testing.T) {
 		}
 	}()
 	fs := newBasicFilesystem(testDirAbs)
-	unrooted = fs.unrootedChecked("/random/other/path")
+	unrooted = fs.unrootedChecked("/random/other/path", testDirAbs)
 }
 
 func TestWatchIssue4877(t *testing.T) {

lib/fs/basicfs_windows.go

@@ -170,12 +170,12 @@ func (f *BasicFilesystem) resolveWin83(absPath string) string {
 	}
 	// Failed getting the long path. Return the part of the path which is
 	// already a long path.
-	for absPath = filepath.Dir(absPath); strings.HasPrefix(absPath, f.rootSymlinkEvaluated); absPath = filepath.Dir(absPath) {
+	for absPath = filepath.Dir(absPath); strings.HasPrefix(absPath, f.root); absPath = filepath.Dir(absPath) {
 		if !isMaybeWin83(absPath) {
 			return absPath
 		}
 	}
-	return f.rootSymlinkEvaluated
+	return f.root
 }
 
 func isMaybeWin83(absPath string) bool {