lib/fs: Check events against both the user and eval root (#6013)

lib/fs/basicfs.go

@@ -345,6 +345,6 @@ func (e *ErrWatchEventOutsideRoot) Error() string {
 	return e.msg
 }
 
-func (f *BasicFilesystem) newErrWatchEventOutsideRoot(absPath, root string) *ErrWatchEventOutsideRoot {
+func (f *BasicFilesystem) newErrWatchEventOutsideRoot(absPath string, roots []string) *ErrWatchEventOutsideRoot {
-	return &ErrWatchEventOutsideRoot{fmt.Sprintf("Watching for changes encountered an event outside of the filesystem root: f.root==%v, root==%v, path==%v. This should never happen, please report this message to forum.syncthing.net.", f.root, root, absPath)}
+	return &ErrWatchEventOutsideRoot{fmt.Sprintf("Watching for changes encountered an event outside of the filesystem root: f.root==%v, roots==%v, path==%v. This should never happen, please report this message to forum.syncthing.net.", f.root, roots, absPath)}
 }

lib/fs/basicfs_unix.go

@@ -60,14 +60,16 @@ func (f *BasicFilesystem) Roots() ([]string, error) {
 // unrooted) or an error 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, root string) (string, *ErrWatchEventOutsideRoot) {
+func (f *BasicFilesystem) unrootedChecked(absPath string, roots []string) (string, *ErrWatchEventOutsideRoot) {
+	for _, root := range roots {
 		if absPath+string(PathSeparator) == root {
 			return ".", nil
 		}
-	if !strings.HasPrefix(absPath, root) {
+		if strings.HasPrefix(absPath, root) {
-		return "", f.newErrWatchEventOutsideRoot(absPath, root)
-	}
 			return rel(absPath, root), nil
+		}
+	}
+	return "", f.newErrWatchEventOutsideRoot(absPath, roots)
 }
 
 func rel(path, prefix string) string {
@@ -78,16 +80,16 @@ var evalSymlinks = filepath.EvalSymlinks
 
 // watchPaths adjust the folder root for use with the notify backend and the
 // corresponding absolute path to be passed to notify to watch name.
-func (f *BasicFilesystem) watchPaths(name string) (string, string, error) {
+func (f *BasicFilesystem) watchPaths(name string) (string, []string, error) {
 	root, err := evalSymlinks(f.root)
 	if err != nil {
-		return "", "", err
+		return "", nil, err
 	}
 
 	absName, err := rooted(name, root)
 	if err != nil {
-		return "", "", err
+		return "", nil, err
 	}
 
-	return filepath.Join(absName, "..."), root, nil
+	return filepath.Join(absName, "..."), []string{root}, nil
 }

lib/fs/basicfs_watch.go

@@ -21,7 +21,7 @@ import (
 var backendBuffer = 500
 
 func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context, ignorePerms bool) (<-chan Event, <-chan error, error) {
-	watchPath, root, err := f.watchPaths(name)
+	watchPath, roots, err := f.watchPaths(name)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -36,7 +36,7 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context
 
 	if ignore.SkipIgnoredDirs() {
 		absShouldIgnore := func(absPath string) bool {
-			rel, err := f.unrootedChecked(absPath, root)
+			rel, err := f.unrootedChecked(absPath, roots)
 			if err != nil {
 				return true
 			}
@@ -55,12 +55,12 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context
 	}
 
 	errChan := make(chan error)
-	go f.watchLoop(name, root, backendChan, outChan, errChan, ignore, ctx)
+	go f.watchLoop(name, roots, backendChan, outChan, errChan, ignore, ctx)
 
 	return outChan, errChan, nil
 }
 
-func (f *BasicFilesystem) watchLoop(name, evalRoot string, backendChan chan notify.EventInfo, outChan chan<- Event, errChan chan<- error, ignore Matcher, ctx context.Context) {
+func (f *BasicFilesystem) watchLoop(name string, roots []string, backendChan chan notify.EventInfo, outChan chan<- Event, errChan chan<- error, ignore Matcher, ctx context.Context) {
 	for {
 		// Detect channel overflow
 		if len(backendChan) == backendBuffer {
@@ -79,7 +79,7 @@ func (f *BasicFilesystem) watchLoop(name, evalRoot string, backendChan chan noti
 
 		select {
 		case ev := <-backendChan:
-			relPath, err := f.unrootedChecked(ev.Path(), evalRoot)
+			relPath, err := f.unrootedChecked(ev.Path(), roots)
 			if err != nil {
 				select {
 				case errChan <- err:

lib/fs/basicfs_watch_test.go

@@ -166,7 +166,7 @@ func TestWatchWinRoot(t *testing.T) {
 	// testFs is Filesystem, but we need BasicFilesystem here
 	root := `D:\`
 	fs := newBasicFilesystem(root)
-	watch, root, err := fs.watchPaths(".")
+	watch, roots, err := fs.watchPaths(".")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -178,7 +178,7 @@ func TestWatchWinRoot(t *testing.T) {
 			}
 			cancel()
 		}()
-		fs.watchLoop(".", root, backendChan, outChan, errChan, fakeMatcher{}, ctx)
+		fs.watchLoop(".", roots, backendChan, outChan, errChan, fakeMatcher{}, ctx)
 	}()
 
 	// filepath.Dir as watch has a /... suffix
@@ -210,7 +210,7 @@ func TestWatchOutside(t *testing.T) {
 	// testFs is Filesystem, but we need BasicFilesystem here
 	fs := newBasicFilesystem(testDirAbs)
 
-	go fs.watchLoop(".", testDirAbs, backendChan, outChan, errChan, fakeMatcher{}, ctx)
+	go fs.watchLoop(".", []string{testDirAbs}, backendChan, outChan, errChan, fakeMatcher{}, ctx)
 
 	backendChan <- fakeEventInfo(filepath.Join(filepath.Dir(testDirAbs), "outside"))
 
@@ -234,7 +234,7 @@ func TestWatchSubpath(t *testing.T) {
 	fs := newBasicFilesystem(testDirAbs)
 
 	abs, _ := fs.rooted("sub")
-	go fs.watchLoop("sub", testDirAbs, backendChan, outChan, errChan, fakeMatcher{}, ctx)
+	go fs.watchLoop("sub", []string{testDirAbs}, backendChan, outChan, errChan, fakeMatcher{}, ctx)
 
 	backendChan <- fakeEventInfo(filepath.Join(abs, "file"))
 
@@ -347,7 +347,7 @@ func TestWatchSymlinkedRoot(t *testing.T) {
 
 func TestUnrootedChecked(t *testing.T) {
 	fs := newBasicFilesystem(testDirAbs)
-	if unrooted, err := fs.unrootedChecked("/random/other/path", testDirAbs); err == nil {
+	if unrooted, err := fs.unrootedChecked("/random/other/path", []string{testDirAbs}); err == nil {
 		t.Error("unrootedChecked did not return an error on outside path, but returned", unrooted)
 	}
 }

lib/fs/basicfs_windows.go

@@ -156,17 +156,19 @@ func (f *BasicFilesystem) Roots() ([]string, error) {
 // unrooted) or an error 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, root string) (string, error) {
+func (f *BasicFilesystem) unrootedChecked(absPath string, roots []string) (string, error) {
 	absPath = f.resolveWin83(absPath)
 	lowerAbsPath := UnicodeLowercase(absPath)
+	for _, root := range roots {
 		lowerRoot := UnicodeLowercase(root)
 		if lowerAbsPath+string(PathSeparator) == lowerRoot {
 			return ".", nil
 		}
-	if !strings.HasPrefix(lowerAbsPath, lowerRoot) {
+		if strings.HasPrefix(lowerAbsPath, lowerRoot) {
-		return "", f.newErrWatchEventOutsideRoot(lowerAbsPath, lowerRoot)
-	}
 			return rel(absPath, root), nil
+		}
+	}
+	return "", f.newErrWatchEventOutsideRoot(lowerAbsPath, roots)
 }
 
 func rel(path, prefix string) string {
@@ -294,10 +296,10 @@ func evalSymlinks(in string) (string, error) {
 
 // watchPaths adjust the folder root for use with the notify backend and the
 // corresponding absolute path to be passed to notify to watch name.
-func (f *BasicFilesystem) watchPaths(name string) (string, string, error) {
+func (f *BasicFilesystem) watchPaths(name string) (string, []string, error) {
 	root, err := evalSymlinks(f.root)
 	if err != nil {
-		return "", "", err
+		return "", nil, err
 	}
 
 	// Remove `\\?\` prefix if the path is just a drive letter as a dirty
@@ -308,11 +310,17 @@ func (f *BasicFilesystem) watchPaths(name string) (string, string, error) {
 
 	absName, err := rooted(name, root)
 	if err != nil {
-		return "", "", err
+		return "", nil, err
 	}
 
-	root = f.resolveWin83(root)
+	roots := []string{f.resolveWin83(root)}
 	absName = f.resolveWin83(absName)
 
-	return filepath.Join(absName, "..."), root, nil
+	// Events returned from fs watching are all over the place, so allow
+	// both the user's input and the result of "canonicalizing" the path.
+	if roots[0] != f.root {
+		roots = append(roots, f.root)
+	}
+
+	return filepath.Join(absName, "..."), roots, nil
 }

lib/fs/basicfs_windows_test.go

@@ -131,7 +131,7 @@ func TestRelUnrootedCheckedWindows(t *testing.T) {
 		// on these test cases.
 		for _, root := range []string{tc.root, strings.ToLower(tc.root), strings.ToUpper(tc.root)} {
 			fs := BasicFilesystem{root: root}
-			if res, err := fs.unrootedChecked(tc.abs, tc.root); err != nil {
+			if res, err := fs.unrootedChecked(tc.abs, []string{tc.root}); err != nil {
 				t.Errorf(`Unexpected error from unrootedChecked("%v", "%v"): %v (fs.root: %v)`, tc.abs, tc.root, err, root)
 			} else if res != tc.expectedRel {
 				t.Errorf(`unrootedChecked("%v", "%v") == "%v", expected "%v" (fs.root: %v)`, tc.abs, tc.root, res, tc.expectedRel, root)
@@ -140,6 +140,21 @@ func TestRelUnrootedCheckedWindows(t *testing.T) {
 	}
 }
 
+// TestMultipleRoot checks that fs.unrootedChecked returns the correct path
+// when given more than one possible root path.
+func TestMultipleRoot(t *testing.T) {
+	root := `c:\foO`
+	roots := []string{root, `d:\`}
+	rel := `bar`
+	path := filepath.Join(root, rel)
+	fs := BasicFilesystem{root: root}
+	if res, err := fs.unrootedChecked(path, roots); err != nil {
+		t.Errorf(`Unexpected error from unrootedChecked("%v", "%v"): %v (fs.root: %v)`, path, roots, err, root)
+	} else if res != rel {
+		t.Errorf(`unrootedChecked("%v", "%v") == "%v", expected "%v" (fs.root: %v)`, path, roots, res, rel, root)
+	}
+}
+
 func TestGetFinalPath(t *testing.T) {
 	testCases := []struct {
 		input         string