From cfec9e38e6519e80e1e3ac3de224ffd33997bcea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Mur=C3=A9?= Date: Sat, 14 Jan 2023 12:52:56 +0100 Subject: [PATCH 1/3] dotgit: fix a filesystem race in Refs/walkReferencesTree In walkReferencesTree(), the filesystem is browsed recursively. After a folder is listed, each child element (directory, file) are inspected. What can happen is that by the time we get to operate on the child element, it might have been deleted from the filesystem and we would get a PathError. In the case of directories there was already a case to avoid bubbling up the error (we consider that there is no ref there, which is correct), but that was missing for files. This commit just apply the same logic. --- storage/filesystem/dotgit/dotgit.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index 6c386f799..2be2bae3e 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -943,6 +943,7 @@ func (d *DotGit) walkReferencesTree(refs *[]*plumbing.Reference, relPath []strin files, err := d.fs.ReadDir(d.fs.Join(relPath...)) if err != nil { if os.IsNotExist(err) { + // a race happened, and our directory is gone now return nil } @@ -960,6 +961,10 @@ func (d *DotGit) walkReferencesTree(refs *[]*plumbing.Reference, relPath []strin } ref, err := d.readReferenceFile(".", strings.Join(newRelPath, "/")) + if os.IsNotExist(err) { + // a race happened, and our file is gone now + continue + } if err != nil { return err } From 16cc29310be7089bbc8d54d79d608834d2753adc Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Sat, 4 Mar 2023 19:12:51 +0100 Subject: [PATCH 2/3] dotgit: test skip deleted references Checks that reading references it correctly skips deleted directories and files. Signed-off-by: Javi Fontan --- storage/filesystem/dotgit/dotgit_test.go | 67 ++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index a8f0eb754..098b559ab 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -864,3 +864,70 @@ func (s *SuiteDotGit) TestIncBytes(c *C) { c.Assert(overflow, Equals, test.overflow) } } + +// this filesystem wrapper returns os.ErrNotExist if the file matches +// the provided paths list +type notExistsFS struct { + billy.Filesystem + + paths []string +} + +func (f *notExistsFS) matches(filename string) bool { + for _, n := range f.paths { + if filename == n { + return true + } + } + return false +} + +func (f *notExistsFS) Open(filename string) (billy.File, error) { + if f.matches(filename) { + return nil, os.ErrNotExist + } + + return f.Filesystem.Open(filename) +} + +func (f *notExistsFS) ReadDir(path string) ([]os.FileInfo, error) { + if f.matches(path) { + return nil, os.ErrNotExist + } + + return f.Filesystem.ReadDir(path) +} + +func (s *SuiteDotGit) TestDeletedRefs(c *C) { + fs, clean := s.TemporalFilesystem() + defer clean() + + dir := New(¬ExistsFS{ + Filesystem: fs, + paths: []string{ + "refs/heads/bar", + "refs/heads/baz", + }, + }) + + err := dir.SetRef(plumbing.NewReferenceFromStrings( + "refs/heads/foo", + "e8d3ffab552895c19b9fcf7aa264d277cde33881", + ), nil) + c.Assert(err, IsNil) + err = dir.SetRef(plumbing.NewReferenceFromStrings( + "refs/heads/bar", + "a8d3ffab552895c19b9fcf7aa264d277cde33881", + ), nil) + c.Assert(err, IsNil) + err = dir.SetRef(plumbing.NewReferenceFromStrings( + "refs/heads/baz/baz", + "a8d3ffab552895c19b9fcf7aa264d277cde33881", + ), nil) + c.Assert(err, IsNil) + + refs, err := dir.Refs() + c.Assert(err, IsNil) + c.Assert(refs, HasLen, 1) + c.Assert(refs[0].Name(), Equals, plumbing.ReferenceName("refs/heads/foo")) +} From 660071d48cfe9728953e4a84960d40deb7078811 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Sat, 4 Mar 2023 20:12:17 +0100 Subject: [PATCH 3/3] dotgit: fix deleted references test in windows Signed-off-by: Javi Fontan --- storage/filesystem/dotgit/dotgit_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index 098b559ab..63c9eb015 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -873,9 +873,10 @@ type notExistsFS struct { paths []string } -func (f *notExistsFS) matches(filename string) bool { +func (f *notExistsFS) matches(path string) bool { + p := filepath.ToSlash(path) for _, n := range f.paths { - if filename == n { + if p == n { return true } }