From e302ccf4b4fa0c2fba77036be7f5114b5574412d Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Mon, 22 Apr 2019 11:12:32 +0200 Subject: [PATCH] lib/fs: Improve IsParent (#5658) --- lib/fs/filesystem_test.go | 64 +++++++++++++++++++++++++++++++++++++++ lib/fs/util.go | 20 ++++++++++-- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/lib/fs/filesystem_test.go b/lib/fs/filesystem_test.go index 6716d52db..9e378e5f3 100644 --- a/lib/fs/filesystem_test.go +++ b/lib/fs/filesystem_test.go @@ -8,6 +8,7 @@ package fs import ( "path/filepath" + "runtime" "testing" ) @@ -106,3 +107,66 @@ func TestFileModeString(t *testing.T) { t.Fatalf("Got %v, expected %v", fm.String(), exp) } } + +func TestIsParent(t *testing.T) { + test := func(path, parent string, expected bool) { + t.Helper() + path = filepath.FromSlash(path) + parent = filepath.FromSlash(parent) + if res := IsParent(path, parent); res != expected { + t.Errorf(`Unexpected result: IsParent("%v", "%v"): %v should be %v`, path, parent, res, expected) + } + } + testBoth := func(path, parent string, expected bool) { + t.Helper() + test(path, parent, expected) + if runtime.GOOS == "windows" { + test("C:/"+path, "C:/"+parent, expected) + } else { + test("/"+path, "/"+parent, expected) + } + } + + // rel - abs + for _, parent := range []string{"/", "/foo", "/foo/bar"} { + for _, path := range []string{"", ".", "foo", "foo/bar", "bas", "bas/baz"} { + if runtime.GOOS == "windows" { + parent = "C:/" + parent + } + test(parent, path, false) + test(path, parent, false) + } + } + + // equal + for i, path := range []string{"/", "/foo", "/foo/bar", "", ".", "foo", "foo/bar"} { + if i < 3 && runtime.GOOS == "windows" { + path = "C:" + path + } + test(path, path, false) + } + + test("", ".", false) + test(".", "", false) + for _, parent := range []string{"", "."} { + for _, path := range []string{"foo", "foo/bar"} { + test(path, parent, true) + test(parent, path, false) + } + } + for _, parent := range []string{"foo", "foo/bar"} { + for _, path := range []string{"bar", "bar/foo"} { + testBoth(path, parent, false) + testBoth(parent, path, false) + } + } + for _, parent := range []string{"foo", "foo/bar"} { + for _, path := range []string{"foo/bar/baz", "foo/bar/baz/bas"} { + testBoth(path, parent, true) + testBoth(parent, path, false) + if runtime.GOOS == "windows" { + test("C:/"+path, "D:/"+parent, false) + } + } + } +} diff --git a/lib/fs/util.go b/lib/fs/util.go index 3c44d3c9e..e1d9836ad 100644 --- a/lib/fs/util.go +++ b/lib/fs/util.go @@ -78,11 +78,25 @@ func WindowsInvalidFilename(name string) bool { return strings.ContainsAny(name, windowsDisallowedCharacters) } +// IsParent compares paths purely lexicographically, meaning it returns false +// if path and parent aren't both absolute or relative. func IsParent(path, parent string) bool { - if len(parent) == 0 { + if parent == path { + // Twice the same root on windows would not be caught at the end. + return false + } + if filepath.IsAbs(path) != filepath.IsAbs(parent) { + return false + } + if parent == "" || parent == "." { // The empty string is the parent of everything except the empty - // string. (Avoids panic in the next step.) - return len(path) > 0 + // string and ".". (Avoids panic in the last step.) + return path != "" && path != "." + } + if parent == "/" { + // The root is the parent of everything except itself, which would + // not be caught below. + return path != "/" } if parent[len(parent)-1] != PathSeparator { parent += string(PathSeparator)