From c2ff49ed436614cc0a6a0332bd1f3fa63b608a0d Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Sat, 11 Aug 2018 22:24:36 +0200 Subject: [PATCH] lib/fs: Evaluate root when watching not on fs creation (fixes #5043) (#5105) --- lib/fs/basicfs.go | 57 ++++++++++++------------------------ lib/fs/basicfs_test.go | 33 +++++++++++++++++++++ lib/fs/basicfs_watch.go | 30 ++++++++++++------- lib/fs/basicfs_watch_test.go | 12 ++++++-- lib/fs/basicfs_windows.go | 4 +-- 5 files changed, 83 insertions(+), 53 deletions(-) diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go index 82dfdf669..8990e36e9 100644 --- a/lib/fs/basicfs.go +++ b/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 +} diff --git a/lib/fs/basicfs_test.go b/lib/fs/basicfs_test.go index 0d68d5890..5a05fbb1f 100644 --- a/lib/fs/basicfs_test.go +++ b/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) + } +} diff --git a/lib/fs/basicfs_watch.go b/lib/fs/basicfs_watch.go index 19802b951..2563a71d2 100644 --- a/lib/fs/basicfs_watch.go +++ b/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) } diff --git a/lib/fs/basicfs_watch_test.go b/lib/fs/basicfs_watch_test.go index 41bbf10af..c5c6be495 100644 --- a/lib/fs/basicfs_watch_test.go +++ b/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) { diff --git a/lib/fs/basicfs_windows.go b/lib/fs/basicfs_windows.go index 44950ec0c..eb4ade0f9 100644 --- a/lib/fs/basicfs_windows.go +++ b/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 {