From 26d87ec3bbd8ba9b18397e1117047e57f182296d Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Wed, 28 Mar 2018 23:01:25 +0200 Subject: [PATCH] lib/fs: Don't panic when watching a folder with symlinked root (#4846) --- lib/fs/basicfs.go | 38 ++++++++++++++++++++------ lib/fs/basicfs_watch.go | 12 ++++----- lib/fs/basicfs_watch_test.go | 52 ++++++++++++++++++++++++++++++++++-- 3 files changed, 86 insertions(+), 16 deletions(-) diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go index 92567b470..818bb6540 100644 --- a/lib/fs/basicfs.go +++ b/lib/fs/basicfs.go @@ -25,7 +25,8 @@ 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 + root string + rootSymlinkEvaluated string } func newBasicFilesystem(root string) *BasicFilesystem { @@ -52,21 +53,34 @@ 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 } - // 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. - } else if root[len(root)-1] != filepath.Separator { + return root + } + + // 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 &BasicFilesystem{ - root: root, - } + return root } // rooted expands the relative path to the full path that is then used with os @@ -109,7 +123,15 @@ func (f *BasicFilesystem) rooted(rel string) (string, error) { } func (f *BasicFilesystem) unrooted(path string) string { - return strings.TrimPrefix(strings.TrimPrefix(path, f.root), string(PathSeparator)) + 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)) } func (f *BasicFilesystem) Chmod(name string, mode FileMode) error { diff --git a/lib/fs/basicfs_watch.go b/lib/fs/basicfs_watch.go index a28d18842..81b20fdda 100644 --- a/lib/fs/basicfs_watch.go +++ b/lib/fs/basicfs_watch.go @@ -12,6 +12,7 @@ import ( "context" "errors" "path/filepath" + "strings" "github.com/syncthing/notify" ) @@ -47,12 +48,12 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context return nil, err } - go f.watchLoop(name, absName, backendChan, outChan, ignore, ctx) + go f.watchLoop(name, backendChan, outChan, ignore, ctx) return outChan, nil } -func (f *BasicFilesystem) watchLoop(name string, absName string, backendChan chan notify.EventInfo, outChan chan<- Event, ignore Matcher, ctx context.Context) { +func (f *BasicFilesystem) watchLoop(name string, backendChan chan notify.EventInfo, outChan chan<- Event, ignore Matcher, ctx context.Context) { for { // Detect channel overflow if len(backendChan) == backendBuffer { @@ -105,12 +106,11 @@ func (f *BasicFilesystem) eventType(notifyType notify.Event) EventType { // 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.root { + if absPath+string(PathSeparator) == f.rootSymlinkEvaluated { return "." } - relPath := f.unrooted(absPath) - if relPath == absPath { + if !strings.HasPrefix(absPath, f.rootSymlinkEvaluated) { panic("bug: Notify backend is processing a change outside of the watched path: " + absPath) } - return relPath + return f.unrootedSymlinkEvaluated(absPath) } diff --git a/lib/fs/basicfs_watch_test.go b/lib/fs/basicfs_watch_test.go index 5ff83ac90..ef6f90c18 100644 --- a/lib/fs/basicfs_watch_test.go +++ b/lib/fs/basicfs_watch_test.go @@ -123,7 +123,7 @@ func TestWatchOutside(t *testing.T) { } cancel() }() - fs.watchLoop(".", testDirAbs, backendChan, outChan, fakeMatcher{}, ctx) + fs.watchLoop(".", backendChan, outChan, fakeMatcher{}, ctx) }() backendChan <- fakeEventInfo(filepath.Join(filepath.Dir(testDirAbs), "outside")) @@ -139,7 +139,7 @@ func TestWatchSubpath(t *testing.T) { fs := newBasicFilesystem(testDirAbs) abs, _ := fs.rooted("sub") - go fs.watchLoop("sub", abs, backendChan, outChan, fakeMatcher{}, ctx) + go fs.watchLoop("sub", backendChan, outChan, fakeMatcher{}, ctx) backendChan <- fakeEventInfo(filepath.Join(abs, "file")) @@ -208,6 +208,54 @@ func TestWatchErrorLinuxInterpretation(t *testing.T) { } } +func TestWatchSymlinkedRoot(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Involves symlinks") + } + + name := "symlinkedRoot" + if err := testFs.MkdirAll(name, 0755); err != nil { + panic(fmt.Sprintf("Failed to create directory %s: %s", name, err)) + } + defer testFs.RemoveAll(name) + + root := filepath.Join(name, "root") + if err := testFs.MkdirAll(root, 0777); err != nil { + panic(err) + } + link := filepath.Join(name, "link") + + if err := testFs.CreateSymlink(filepath.Join(testFs.URI(), root), link); err != nil { + panic(err) + } + + linkedFs := NewFilesystem(FilesystemTypeBasic, filepath.Join(testFs.URI(), link)) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + if _, err := linkedFs.Watch(".", fakeMatcher{}, ctx, false); err != nil { + panic(err) + } + + if err := linkedFs.MkdirAll("foo", 0777); err != nil { + panic(err) + } + + // Give the panic some time to happen + sleepMs(100) +} + +func TestUnrootedChecked(t *testing.T) { + var unrooted string + defer func() { + if recover() == nil { + t.Fatal("unrootedChecked did not panic on outside path, but returned", unrooted) + } + }() + fs := newBasicFilesystem(testDirAbs) + unrooted = fs.unrootedChecked("/random/other/path") +} + // path relative to folder root, also creates parent dirs if necessary func createTestFile(name string, file string) string { joined := filepath.Join(name, file)