From 0f88088bbc504558227ce14c5595c9c21b4b670b Mon Sep 17 00:00:00 2001 From: Ibrahim AshShohail Date: Fri, 4 Aug 2017 14:39:34 +0300 Subject: [PATCH] internal/fs: fix isCaseSensitiveFilesystem bug isCaseSensitiveFilesystem returns true when os.Stat fails on the dir passed. This caused HasFilepathPrefix to always treat *nix systems as case-sensitive, since it passed a relative path (missing the root) to isCaseSensitiveFilesystem. This commit updates isCaseSensitiveFilesystem to return an error in addtion to the boolean check. Also, HasFilepathPrefix now passes absolute paths to isCaseSensitiveFilesystem. Signed-off-by: Ibrahim AshShohail --- internal/fs/fs.go | 72 ++++++++++++++++++++++++++---------------- internal/fs/fs_test.go | 40 +++++++++++++---------- 2 files changed, 69 insertions(+), 43 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index d0b63f989c..e1b00d1c52 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -45,13 +45,11 @@ func HasFilepathPrefix(path, prefix string) bool { return false } - // because filepath.Join("c:","dir") returns "c:dir", we have to manually add path separator to drive letters - if strings.HasSuffix(vnPath, ":") { - vnPath += string(os.PathSeparator) - } - if strings.HasSuffix(vnPrefix, ":") { - vnPrefix += string(os.PathSeparator) - } + // Because filepath.Join("c:","dir") returns "c:dir", we have to manually + // add path separator to drive letters. Also, we need to set the path root + // on *nix systems, since filepath.Join("", "dir") returns a relative path. + vnPath += string(os.PathSeparator) + vnPrefix += string(os.PathSeparator) var dn string @@ -74,7 +72,7 @@ func HasFilepathPrefix(path, prefix string) bool { return false } - // d,p are initialized with "" on *nix and volume name on Windows + // d,p are initialized with "/" on *nix and volume name on Windows d := vnPath p := vnPrefix @@ -84,7 +82,11 @@ func HasFilepathPrefix(path, prefix string) bool { // something like ext4 filesystem mounted on FAT // mountpoint, mounted on ext4 filesystem, i.e. the // problematic filesystem is not the last one. - if isCaseSensitiveFilesystem(filepath.Join(d, dirs[i])) { + caseSensitive, err := isCaseSensitiveFilesystem(filepath.Join(d, dirs[i])) + if err != nil { + return false + } + if caseSensitive { d = filepath.Join(d, dirs[i]) p = filepath.Join(p, prefixes[i]) } else { @@ -106,28 +108,45 @@ func EqualPaths(p1, p2 string) (bool, error) { p1 = filepath.Clean(p1) p2 = filepath.Clean(p2) - if isDir, err := IsDir(p2); err != nil && !strings.HasSuffix(err.Error(), "is not a directory") { - return false, err - } else if !isDir { - // If p2 is not a directory, compare the bases of the paths to ensure - // they are equivalent. - var p1Base, p2Base string + fi1, err := os.Stat(p1) + if err != nil { + return false, errors.Wrapf(err, "could not check for path equivalence") + } + fi2, err := os.Stat(p2) + if err != nil { + return false, errors.Wrapf(err, "could not check for path equivalence") + } - p1, p1Base = filepath.Split(p1) - p2, p2Base = filepath.Split(p2) + p1Filename, p2Filename := "", "" - if isCaseSensitiveFilesystem(p1) { - if p1Base != p2Base { + if !fi1.IsDir() { + p1, p1Filename = filepath.Split(p1) + } + if !fi2.IsDir() { + p2, p2Filename = filepath.Split(p2) + } + + if !HasFilepathPrefix(p1, p2) || !HasFilepathPrefix(p2, p1) { + return false, nil + } + + if p1Filename != "" || p2Filename != "" { + caseSensitive, err := isCaseSensitiveFilesystem(filepath.Join(p1, p1Filename)) + if err != nil { + return false, errors.Wrap(err, "could not check for filesystem case-sensitivity") + } + if caseSensitive { + if p1Filename != p2Filename { return false, nil } } else { - if strings.ToLower(p1Base) != strings.ToLower(p2Base) { + if strings.ToLower(p1Filename) != strings.ToLower(p2Filename) { return false, nil } } } - return len(p1) == len(p2) && HasFilepathPrefix(p1, p2), nil + return true, nil } // RenameWithFallback attempts to rename a file or directory, but falls back to @@ -189,21 +208,20 @@ func renameByCopy(src, dst string) error { // If the input directory is such that the last component is composed // exclusively of case-less codepoints (e.g. numbers), this function will // return false. -func isCaseSensitiveFilesystem(dir string) bool { - alt := filepath.Join(filepath.Dir(dir), - genTestFilename(filepath.Base(dir))) +func isCaseSensitiveFilesystem(dir string) (bool, error) { + alt := filepath.Join(filepath.Dir(dir), genTestFilename(filepath.Base(dir))) dInfo, err := os.Stat(dir) if err != nil { - return true + return false, errors.Wrap(err, "could not determine the case-sensitivity of the filesystem") } aInfo, err := os.Stat(alt) if err != nil { - return true + return false, errors.Wrap(err, "could not determine the case-sensitivity of the filesystem") } - return !os.SameFile(dInfo, aInfo) + return !os.SameFile(dInfo, aInfo), nil } // genTestFilename returns a string with at most one rune case-flipped. diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index f5625eddbf..3731f24a27 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -145,22 +145,24 @@ func TestEqualPaths(t *testing.T) { h.TempDir("DIR") h.TempFile("FILE", "") - // caseInsensitive flag ensures that these cases are only equivalent on a - // case-insensitive filesystem. - caseInsensitive := isCaseSensitiveFilesystem(h.Path("dir")) - testcases := []struct { - p1, p2 string - want bool - err bool + p1, p2 string + caseSensitiveEquivalent bool + caseInensitiveEquivalent bool + err bool }{ - {h.Path("dir"), h.Path("dir"), true, false}, - {h.Path("file"), h.Path("file"), true, false}, - {h.Path("dir"), h.Path("dir2"), false, false}, - {h.Path("file"), h.Path("file2"), false, false}, - {h.Path("dir"), h.Path("file"), false, false}, - {h.Path("dir"), h.Path("DIR"), caseInsensitive, false}, - {strings.ToLower(h.Path("dir")), strings.ToUpper(h.Path("dir")), caseInsensitive, true}, + {h.Path("dir"), h.Path("dir"), true, true, false}, + {h.Path("file"), h.Path("file"), true, true, false}, + {h.Path("dir"), h.Path("dir2"), false, false, false}, + {h.Path("file"), h.Path("file2"), false, false, false}, + {h.Path("dir"), h.Path("file"), false, false, false}, + {h.Path("dir"), h.Path("DIR"), false, true, false}, + {strings.ToLower(h.Path("dir")), strings.ToUpper(h.Path("dir")), false, true, true}, + } + + caseSensitive, err := isCaseSensitiveFilesystem(h.Path("dir")) + if err != nil { + t.Fatal("unexpcted error:", err) } for _, tc := range testcases { @@ -168,8 +170,14 @@ func TestEqualPaths(t *testing.T) { if err != nil && !tc.err { t.Error("unexpected error:", err) } - if got != tc.want { - t.Errorf("expected EqualPaths(%q, %q) to be %t, got %t", tc.p1, tc.p2, tc.want, got) + if caseSensitive { + if tc.caseSensitiveEquivalent != got { + t.Errorf("expected EqualPaths(%q, %q) to be %t on case-sensitive filesystem, got %t", tc.p1, tc.p2, tc.caseSensitiveEquivalent, got) + } + } else { + if tc.caseInensitiveEquivalent != got { + t.Errorf("expected EqualPaths(%q, %q) to be %t on case-insensitive filesystem, got %t", tc.p1, tc.p2, tc.caseInensitiveEquivalent, got) + } } } }