Browse Source

lib/fs: Fix watcher panic due to casing on windows (fixes #4877) (#4878)

Simon Frei 7 years ago
parent
commit
01aef75c96
3 changed files with 48 additions and 5 deletions
  1. 15 3
      lib/fs/basicfs.go
  2. 3 2
      lib/fs/basicfs_watch.go
  3. 30 0
      lib/fs/basicfs_watch_test.go

+ 15 - 3
lib/fs/basicfs.go

@@ -88,8 +88,20 @@ func adjustRoot(root string) string {
 // directory, this returns an error, to prevent accessing files that are not in the
 // shared directory.
 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 f.root == "" {
+	if root == "" {
 		return "", ErrInvalidFilename
 	}
 
@@ -97,7 +109,7 @@ func (f *BasicFilesystem) rooted(rel string) (string, error) {
 
 	// The expected prefix for the resulting path is the root, with a path
 	// separator at the end.
-	expectedPrefix := filepath.FromSlash(f.root)
+	expectedPrefix := filepath.FromSlash(root)
 	if !strings.HasSuffix(expectedPrefix, pathSep) {
 		expectedPrefix += pathSep
 	}
@@ -111,7 +123,7 @@ func (f *BasicFilesystem) rooted(rel string) (string, error) {
 	// The supposedly correct path is the one filepath.Join will return, as
 	// it does cleaning and so on. Check that one first to make sure no
 	// obvious escape attempts have been made.
-	joined := filepath.Join(f.root, rel)
+	joined := filepath.Join(root, rel)
 	if rel == "." && !strings.HasSuffix(joined, pathSep) {
 		joined += pathSep
 	}

+ 3 - 2
lib/fs/basicfs_watch.go

@@ -11,6 +11,7 @@ package fs
 import (
 	"context"
 	"errors"
+	"fmt"
 	"path/filepath"
 	"strings"
 
@@ -23,7 +24,7 @@ import (
 var backendBuffer = 500
 
 func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context, ignorePerms bool) (<-chan Event, error) {
-	absName, err := f.rooted(name)
+	absName, err := f.rootedSymlinkEvaluated(name)
 	if err != nil {
 		return nil, err
 	}
@@ -110,7 +111,7 @@ func (f *BasicFilesystem) unrootedChecked(absPath string) string {
 		return "."
 	}
 	if !strings.HasPrefix(absPath, f.rootSymlinkEvaluated) {
-		panic("bug: Notify backend is processing a change outside of the watched path: " + absPath)
+		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))
 	}
 	return f.unrootedSymlinkEvaluated(absPath)
 }

+ 30 - 0
lib/fs/basicfs_watch_test.go

@@ -16,6 +16,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strconv"
+	"strings"
 	"syscall"
 	"testing"
 	"time"
@@ -256,6 +257,35 @@ func TestUnrootedChecked(t *testing.T) {
 	unrooted = fs.unrootedChecked("/random/other/path")
 }
 
+func TestWatchIssue4877(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("Windows specific test")
+	}
+
+	name := "Issue4877"
+
+	file := "file"
+
+	testCase := func() {
+		createTestFile(name, file)
+	}
+
+	expectedEvents := []Event{
+		{file, NonRemove},
+	}
+	allowedEvents := []Event{
+		{name, NonRemove},
+	}
+
+	origTestFs := testFs
+	testFs = NewFilesystem(FilesystemTypeBasic, strings.ToLower(testDirAbs[:1])+strings.ToUpper(testDirAbs[1:]))
+	defer func() {
+		testFs = origTestFs
+	}()
+
+	testScenario(t, name, testCase, expectedEvents, allowedEvents, "")
+}
+
 // path relative to folder root, also creates parent dirs if necessary
 func createTestFile(name string, file string) string {
 	joined := filepath.Join(name, file)