Browse Source

lib/fs: Case insensitive conversion to rel path on windows (fixes #5183) (#5176)

Simon Frei 7 years ago
parent
commit
50ba0fd079

+ 0 - 4
lib/fs/basicfs.go

@@ -90,10 +90,6 @@ func (f *BasicFilesystem) unrooted(path string) string {
 	return rel(path, f.root)
 }
 
-func rel(path, prefix string) string {
-	return strings.TrimPrefix(strings.TrimPrefix(path, prefix), string(PathSeparator))
-}
-
 func (f *BasicFilesystem) Chmod(name string, mode FileMode) error {
 	name, err := f.rooted(name)
 	if err != nil {

+ 26 - 0
lib/fs/basicfs_test.go

@@ -490,3 +490,29 @@ func TestNewBasicFilesystem(t *testing.T) {
 		t.Errorf(`newBasicFilesystem("relative/path").root == %q, expected absolutification`, fs.root)
 	}
 }
+
+func TestRel(t *testing.T) {
+	testCases := []struct {
+		root        string
+		abs         string
+		expectedRel string
+	}{
+		{"/", "/", ""},
+		{"/", "/test", "test"},
+		{"/", "/Test", "Test"},
+		{"/Test", "/Test/test", "test"},
+	}
+	if runtime.GOOS == "windows" {
+		for i := range testCases {
+			testCases[i].root = filepath.FromSlash(testCases[i].root)
+			testCases[i].abs = filepath.FromSlash(testCases[i].abs)
+			testCases[i].expectedRel = filepath.FromSlash(testCases[i].expectedRel)
+		}
+	}
+
+	for _, tc := range testCases {
+		if res := rel(tc.abs, tc.root); res != tc.expectedRel {
+			t.Errorf(`rel("%v", "%v") == "%v", expected "%v"`, tc.abs, tc.root, res, tc.expectedRel)
+		}
+	}
+}

+ 21 - 3
lib/fs/basicfs_unix.go

@@ -8,7 +8,11 @@
 
 package fs
 
-import "os"
+import (
+	"fmt"
+	"os"
+	"strings"
+)
 
 func (BasicFilesystem) SymlinksSupported() bool {
 	return true
@@ -52,6 +56,20 @@ func (f *BasicFilesystem) Roots() ([]string, error) {
 	return []string{"/"}, nil
 }
 
-func (f *BasicFilesystem) resolveWin83(absPath string) string {
-	return absPath
+// unrootedChecked returns the path relative to the folder root (same as
+// 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, root string) string {
+	if absPath+string(PathSeparator) == root {
+		return "."
+	}
+	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 rel(absPath, root)
+}
+
+func rel(path, prefix string) string {
+	return strings.TrimPrefix(strings.TrimPrefix(path, prefix), string(PathSeparator))
 }

+ 0 - 17
lib/fs/basicfs_watch.go

@@ -11,10 +11,8 @@ package fs
 import (
 	"context"
 	"errors"
-	"fmt"
 	"path/filepath"
 	"runtime"
-	"strings"
 
 	"github.com/syncthing/notify"
 )
@@ -114,18 +112,3 @@ func (f *BasicFilesystem) eventType(notifyType notify.Event) EventType {
 	}
 	return NonRemove
 }
-
-// unrootedChecked returns the path relative to the folder root (same as
-// 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, root string) string {
-	absPath = f.resolveWin83(absPath)
-	if absPath+string(PathSeparator) == root {
-		return "."
-	}
-	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 rel(absPath, root)
-}

+ 5 - 1
lib/fs/basicfs_watch_test.go

@@ -320,8 +320,12 @@ func TestWatchIssue4877(t *testing.T) {
 		{name, NonRemove},
 	}
 
+	volName := filepath.VolumeName(testDirAbs)
+	if volName == "" {
+		t.Fatalf("Failed to get volume name for path %v", testDirAbs)
+	}
 	origTestFs := testFs
-	testFs = NewFilesystem(FilesystemTypeBasic, strings.ToLower(testDirAbs[:1])+strings.ToUpper(testDirAbs[1:]))
+	testFs = NewFilesystem(FilesystemTypeBasic, strings.ToLower(volName)+strings.ToUpper(testDirAbs[len(volName):]))
 	defer func() {
 		testFs = origTestFs
 	}()

+ 24 - 1
lib/fs/basicfs_windows.go

@@ -152,6 +152,28 @@ func (f *BasicFilesystem) Roots() ([]string, error) {
 	return drives, nil
 }
 
+// unrootedChecked returns the path relative to the folder root (same as
+// 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, root string) string {
+	absPath = f.resolveWin83(absPath)
+	lowerAbsPath := UnicodeLowercase(absPath)
+	lowerRoot := UnicodeLowercase(root)
+	if lowerAbsPath+string(PathSeparator) == lowerRoot {
+		return "."
+	}
+	if !strings.HasPrefix(lowerAbsPath, lowerRoot) {
+		panic(fmt.Sprintf("bug: Notify backend is processing a change outside of the filesystem root: f.root==%v, root==%v (lower), path==%v (lower)", f.root, lowerRoot, lowerAbsPath))
+	}
+	return rel(absPath, root)
+}
+
+func rel(path, prefix string) string {
+	lowerRel := strings.TrimPrefix(strings.TrimPrefix(UnicodeLowercase(path), UnicodeLowercase(prefix)), string(PathSeparator))
+	return path[len(path)-len(lowerRel):]
+}
+
 func (f *BasicFilesystem) resolveWin83(absPath string) string {
 	if !isMaybeWin83(absPath) {
 		return absPath
@@ -170,7 +192,8 @@ 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.root); absPath = filepath.Dir(absPath) {
+	lowerRoot := UnicodeLowercase(f.root)
+	for absPath = filepath.Dir(absPath); strings.HasPrefix(UnicodeLowercase(absPath), lowerRoot); absPath = filepath.Dir(absPath) {
 		if !isMaybeWin83(absPath) {
 			return absPath
 		}

+ 45 - 0
lib/fs/basicfs_windows_test.go

@@ -90,3 +90,48 @@ func TestIsWindows83(t *testing.T) {
 		}
 	}
 }
+
+func TestRelUnrootedCheckedWindows(t *testing.T) {
+	testCases := []struct {
+		root        string
+		abs         string
+		expectedRel string
+	}{
+		{`c:\`, `c:\foo`, `foo`},
+		{`C:\`, `c:\foo`, `foo`},
+		{`C:\`, `C:\foo`, `foo`},
+		{`c:\`, `C:\foo`, `foo`},
+		{`\\?c:\`, `\\?c:\foo`, `foo`},
+		{`\\?C:\`, `\\?c:\foo`, `foo`},
+		{`\\?C:\`, `\\?C:\foo`, `foo`},
+		{`\\?c:\`, `\\?C:\foo`, `foo`},
+		{`c:\foo`, `c:\foo\bar`, `bar`},
+		{`c:\foo`, `c:\foo\bAr`, `bAr`},
+		{`c:\foO`, `c:\Foo\bar`, `bar`},
+		{`c:\foO`, `c:\fOo\bAr`, `bAr`},
+		{`c:\foO`, `c:\fOo`, ``},
+		{`C:\foO`, `c:\fOo`, ``},
+	}
+
+	for _, tc := range testCases {
+		if res := rel(tc.abs, tc.root); res != tc.expectedRel {
+			t.Errorf(`rel("%v", "%v") == "%v", expected "%v"`, tc.abs, tc.root, res, tc.expectedRel)
+		}
+
+		// unrootedChecked really just wraps rel, and does not care about
+		// the actual root of that filesystem, but should not panic on these
+		// test cases.
+		fs := BasicFilesystem{root: tc.root}
+		if res := fs.unrootedChecked(tc.abs, tc.root); res != tc.expectedRel {
+			t.Errorf(`unrootedChecked("%v", "%v") == "%v", expected "%v"`, tc.abs, tc.root, res, tc.expectedRel)
+		}
+		fs = BasicFilesystem{root: strings.ToLower(tc.root)}
+		if res := fs.unrootedChecked(tc.abs, tc.root); res != tc.expectedRel {
+			t.Errorf(`unrootedChecked("%v", "%v") == "%v", expected "%v"`, tc.abs, tc.root, res, tc.expectedRel)
+		}
+		fs = BasicFilesystem{root: strings.ToUpper(tc.root)}
+		if res := fs.unrootedChecked(tc.abs, tc.root); res != tc.expectedRel {
+			t.Errorf(`unrootedChecked("%v", "%v") == "%v", expected "%v"`, tc.abs, tc.root, res, tc.expectedRel)
+		}
+	}
+}