From 2f95ec70316c4bf41f4a4641a3237329deeb75be Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 10 Jan 2020 13:47:35 -0800 Subject: [PATCH] Fix reading absolute paths through OsFs OsFs may be asked to read absolute paths if buildDir is absolute. Check if the path is absolute before prepending srcDir to it. Bug: 146437378 Test: fs_test.go Change-Id: I2a67593e9d836ca3e11dc10b81f49a4fb49d2cdf --- pathtools/fs.go | 25 +++-- pathtools/fs_test.go | 223 ++++++++++++------------------------------- 2 files changed, 76 insertions(+), 172 deletions(-) diff --git a/pathtools/fs.go b/pathtools/fs.go index b45c4a5..21754d0 100644 --- a/pathtools/fs.go +++ b/pathtools/fs.go @@ -131,6 +131,13 @@ func NewOsFs(path string) FileSystem { return &osFs{srcDir: path} } +func (fs *osFs) toAbs(path string) string { + if filepath.IsAbs(path) { + return path + } + return filepath.Join(fs.srcDir, path) +} + func (fs *osFs) removeSrcDirPrefix(path string) string { if fs.srcDir == "" { return path @@ -157,11 +164,11 @@ func (fs *osFs) removeSrcDirPrefixes(paths []string) []string { } func (fs *osFs) Open(name string) (ReaderAtSeekerCloser, error) { - return os.Open(filepath.Join(fs.srcDir, name)) + return os.Open(fs.toAbs(name)) } func (fs *osFs) Exists(name string) (bool, bool, error) { - stat, err := os.Stat(filepath.Join(fs.srcDir, name)) + stat, err := os.Stat(fs.toAbs(name)) if err == nil { return true, stat.IsDir(), nil } else if os.IsNotExist(err) { @@ -172,7 +179,7 @@ func (fs *osFs) Exists(name string) (bool, bool, error) { } func (fs *osFs) IsDir(name string) (bool, error) { - info, err := os.Stat(filepath.Join(fs.srcDir, name)) + info, err := os.Stat(fs.toAbs(name)) if err != nil { return false, err } @@ -180,7 +187,7 @@ func (fs *osFs) IsDir(name string) (bool, error) { } func (fs *osFs) IsSymlink(name string) (bool, error) { - if info, err := os.Lstat(filepath.Join(fs.srcDir, name)); err != nil { + if info, err := os.Lstat(fs.toAbs(name)); err != nil { return false, err } else { return info.Mode()&os.ModeSymlink != 0, nil @@ -192,17 +199,17 @@ func (fs *osFs) Glob(pattern string, excludes []string, follow ShouldFollowSymli } func (fs *osFs) glob(pattern string) ([]string, error) { - paths, err := filepath.Glob(filepath.Join(fs.srcDir, pattern)) + paths, err := filepath.Glob(fs.toAbs(pattern)) fs.removeSrcDirPrefixes(paths) return paths, err } func (fs *osFs) Lstat(path string) (stats os.FileInfo, err error) { - return os.Lstat(filepath.Join(fs.srcDir, path)) + return os.Lstat(fs.toAbs(path)) } func (fs *osFs) Stat(path string) (stats os.FileInfo, err error) { - return os.Stat(filepath.Join(fs.srcDir, path)) + return os.Stat(fs.toAbs(path)) } // Returns a list of all directories under dir @@ -211,7 +218,7 @@ func (fs *osFs) ListDirsRecursive(name string, follow ShouldFollowSymlinks) (dir } func (fs *osFs) ReadDirNames(name string) ([]string, error) { - dir, err := os.Open(filepath.Join(fs.srcDir, name)) + dir, err := os.Open(fs.toAbs(name)) if err != nil { return nil, err } @@ -227,7 +234,7 @@ func (fs *osFs) ReadDirNames(name string) ([]string, error) { } func (fs *osFs) Readlink(name string) (string, error) { - return os.Readlink(filepath.Join(fs.srcDir, name)) + return os.Readlink(fs.toAbs(name)) } type mockFs struct { diff --git a/pathtools/fs_test.go b/pathtools/fs_test.go index a9a7060..3b4d4d0 100644 --- a/pathtools/fs_test.go +++ b/pathtools/fs_test.go @@ -103,6 +103,42 @@ func TestMockFs_followSymlinks(t *testing.T) { } } +func runTestFs(t *testing.T, f func(t *testing.T, fs FileSystem, dir string)) { + mock := symlinkMockFs() + wd, _ := os.Getwd() + absTestDataDir := filepath.Join(wd, testdataDir) + + run := func(t *testing.T, fs FileSystem) { + t.Run("relpath", func(t *testing.T) { + f(t, fs, "") + }) + t.Run("abspath", func(t *testing.T) { + f(t, fs, absTestDataDir) + }) + } + + t.Run("mock", func(t *testing.T) { + f(t, mock, "") + }) + + t.Run("os", func(t *testing.T) { + os.Chdir(absTestDataDir) + defer os.Chdir(wd) + run(t, OsFs) + }) + + t.Run("os relative srcDir", func(t *testing.T) { + run(t, NewOsFs(testdataDir)) + }) + + t.Run("os absolute srcDir", func(t *testing.T) { + os.Chdir("/") + defer os.Chdir(wd) + run(t, NewOsFs(filepath.Join(wd, testdataDir))) + }) + +} + func TestFs_IsDir(t *testing.T) { testCases := []struct { name string @@ -150,37 +186,16 @@ func TestFs_IsDir(t *testing.T) { {"c/f/missing", false, syscall.ENOTDIR}, } - mock := symlinkMockFs() - - run := func(t *testing.T, fs FileSystem) { + runTestFs(t, func(t *testing.T, fs FileSystem, dir string) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - got, err := fs.IsDir(test.name) + got, err := fs.IsDir(filepath.Join(dir, test.name)) checkErr(t, test.err, err) if got != test.isDir { t.Errorf("want: %v, got %v", test.isDir, got) } }) } - } - - t.Run("mock", func(t *testing.T) { - run(t, mock) - }) - - t.Run("os", func(t *testing.T) { - os.Chdir("testdata/dangling") - defer os.Chdir("../..") - run(t, OsFs) - }) - - t.Run("os relative srcDir", func(t *testing.T) { - run(t, NewOsFs(testdataDir)) - }) - - t.Run("os absolute srcDir", func(t *testing.T) { - wd, _ := os.Getwd() - run(t, NewOsFs(filepath.Join(wd, testdataDir))) }) } @@ -213,37 +228,20 @@ func TestFs_ListDirsRecursiveFollowSymlinks(t *testing.T) { {"missing", nil, os.ErrNotExist}, } - mock := symlinkMockFs() - - run := func(t *testing.T, fs FileSystem) { + runTestFs(t, func(t *testing.T, fs FileSystem, dir string) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - got, err := fs.ListDirsRecursive(test.name, FollowSymlinks) + got, err := fs.ListDirsRecursive(filepath.Join(dir, test.name), FollowSymlinks) checkErr(t, test.err, err) - if !reflect.DeepEqual(got, test.dirs) { - t.Errorf("want: %v, got %v", test.dirs, got) + want := append([]string(nil), test.dirs...) + for i := range want { + want[i] = filepath.Join(dir, want[i]) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("want: %v, got %v", want, got) } }) } - } - - t.Run("mock", func(t *testing.T) { - run(t, mock) - }) - - t.Run("os", func(t *testing.T) { - os.Chdir("testdata/dangling") - defer os.Chdir("../..") - run(t, OsFs) - }) - - t.Run("os relative srcDir", func(t *testing.T) { - run(t, NewOsFs(testdataDir)) - }) - - t.Run("os absolute srcDir", func(t *testing.T) { - wd, _ := os.Getwd() - run(t, NewOsFs(filepath.Join(wd, testdataDir))) }) } @@ -276,37 +274,20 @@ func TestFs_ListDirsRecursiveDontFollowSymlinks(t *testing.T) { {"missing", nil, os.ErrNotExist}, } - mock := symlinkMockFs() - - run := func(t *testing.T, fs FileSystem) { + runTestFs(t, func(t *testing.T, fs FileSystem, dir string) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - got, err := fs.ListDirsRecursive(test.name, DontFollowSymlinks) + got, err := fs.ListDirsRecursive(filepath.Join(dir, test.name), DontFollowSymlinks) checkErr(t, test.err, err) - if !reflect.DeepEqual(got, test.dirs) { - t.Errorf("want: %v, got %v", test.dirs, got) + want := append([]string(nil), test.dirs...) + for i := range want { + want[i] = filepath.Join(dir, want[i]) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("want: %v, got %v", want, got) } }) } - } - - t.Run("mock", func(t *testing.T) { - run(t, mock) - }) - - t.Run("os", func(t *testing.T) { - os.Chdir("testdata/dangling") - defer os.Chdir("../..") - run(t, OsFs) - }) - - t.Run("os relative srcDir", func(t *testing.T) { - run(t, NewOsFs(testdataDir)) - }) - - t.Run("os absolute srcDir", func(t *testing.T) { - wd, _ := os.Getwd() - run(t, NewOsFs(filepath.Join(wd, testdataDir))) }) } @@ -356,9 +337,7 @@ func TestFs_Readlink(t *testing.T) { {"dangling/missing/missing", "", os.ErrNotExist}, } - mock := symlinkMockFs() - - run := func(t *testing.T, fs FileSystem) { + runTestFs(t, func(t *testing.T, fs FileSystem, dir string) { for _, test := range testCases { t.Run(test.from, func(t *testing.T) { got, err := fs.Readlink(test.from) @@ -368,25 +347,6 @@ func TestFs_Readlink(t *testing.T) { } }) } - } - - t.Run("mock", func(t *testing.T) { - run(t, mock) - }) - - t.Run("os", func(t *testing.T) { - os.Chdir("testdata/dangling") - defer os.Chdir("../..") - run(t, OsFs) - }) - - t.Run("os relative srcDir", func(t *testing.T) { - run(t, NewOsFs(testdataDir)) - }) - - t.Run("os absolute srcDir", func(t *testing.T) { - wd, _ := os.Getwd() - run(t, NewOsFs(filepath.Join(wd, testdataDir))) }) } @@ -438,12 +398,10 @@ func TestFs_Lstat(t *testing.T) { {"dangling/missing/missing", 0, 0, os.ErrNotExist}, } - mock := symlinkMockFs() - - run := func(t *testing.T, fs FileSystem) { + runTestFs(t, func(t *testing.T, fs FileSystem, dir string) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - got, err := fs.Lstat(test.name) + got, err := fs.Lstat(filepath.Join(dir, test.name)) checkErr(t, test.err, err) if err != nil { return @@ -457,25 +415,6 @@ func TestFs_Lstat(t *testing.T) { } }) } - } - - t.Run("mock", func(t *testing.T) { - run(t, mock) - }) - - t.Run("os", func(t *testing.T) { - os.Chdir("testdata/dangling") - defer os.Chdir("../..") - run(t, OsFs) - }) - - t.Run("os relative srcDir", func(t *testing.T) { - run(t, NewOsFs(testdataDir)) - }) - - t.Run("os absolute srcDir", func(t *testing.T) { - wd, _ := os.Getwd() - run(t, NewOsFs(filepath.Join(wd, testdataDir))) }) } @@ -527,12 +466,10 @@ func TestFs_Stat(t *testing.T) { {"dangling/missing/missing", 0, 0, os.ErrNotExist}, } - mock := symlinkMockFs() - - run := func(t *testing.T, fs FileSystem) { + runTestFs(t, func(t *testing.T, fs FileSystem, dir string) { for _, test := range testCases { t.Run(test.name, func(t *testing.T) { - got, err := fs.Stat(test.name) + got, err := fs.Stat(filepath.Join(dir, test.name)) checkErr(t, test.err, err) if err != nil { return @@ -546,25 +483,6 @@ func TestFs_Stat(t *testing.T) { } }) } - } - - t.Run("mock", func(t *testing.T) { - run(t, mock) - }) - - t.Run("os", func(t *testing.T) { - os.Chdir("testdata/dangling") - defer os.Chdir("../..") - run(t, OsFs) - }) - - t.Run("os relative srcDir", func(t *testing.T) { - run(t, NewOsFs(testdataDir)) - }) - - t.Run("os absolute srcDir", func(t *testing.T) { - wd, _ := os.Getwd() - run(t, NewOsFs(filepath.Join(wd, testdataDir))) }) } @@ -606,9 +524,7 @@ func TestMockFs_glob(t *testing.T) { {"missing", nil}, } - mock := symlinkMockFs() - - run := func(t *testing.T, fs FileSystem) { + runTestFs(t, func(t *testing.T, fs FileSystem, dir string) { for _, test := range testCases { t.Run(test.pattern, func(t *testing.T) { got, err := fs.glob(test.pattern) @@ -620,25 +536,6 @@ func TestMockFs_glob(t *testing.T) { } }) } - } - - t.Run("mock", func(t *testing.T) { - run(t, mock) - }) - - t.Run("os", func(t *testing.T) { - os.Chdir("testdata/dangling") - defer os.Chdir("../..") - run(t, OsFs) - }) - - t.Run("os relative srcDir", func(t *testing.T) { - run(t, NewOsFs(testdataDir)) - }) - - t.Run("os absolute srcDir", func(t *testing.T) { - wd, _ := os.Getwd() - run(t, NewOsFs(filepath.Join(wd, testdataDir))) }) }