From a135699397e82504dd3ed330e5105051ba38220a Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 7 Feb 2020 22:14:50 +0100 Subject: [PATCH 1/2] Close file if file type has changed after initial stat --- internal/archiver/archiver.go | 1 + internal/archiver/archiver_test.go | 57 ++++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 16dd76254..bc5a31483 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -377,6 +377,7 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous // make sure it's still a file if !fs.IsRegularFile(fi) { err = errors.Errorf("file %v changed type, refusing to archive") + _ = file.Close() err = arch.error(abstarget, fi, err) if err != nil { return FutureNode{}, false, err diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 3cdee7c67..34a0074ac 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1985,12 +1985,15 @@ func chmod(t testing.TB, filename string, mode os.FileMode) { type StatFS struct { fs.FS - OverrideLstat map[string]os.FileInfo + OverrideLstat map[string]os.FileInfo + OnlyOverrideStat bool } func (fs *StatFS) Lstat(name string) (os.FileInfo, error) { - if fi, ok := fs.OverrideLstat[name]; ok { - return fi, nil + if !fs.OnlyOverrideStat { + if fi, ok := fs.OverrideLstat[name]; ok { + return fi, nil + } } return fs.FS.Lstat(name) @@ -2102,3 +2105,51 @@ func TestMetadataChanged(t *testing.T) { checker.TestCheckRepo(t, repo) } + +func TestRacyFileSwap(t *testing.T) { + files := TestDir{ + "file": TestFile{ + Content: "foo bar test file", + }, + } + + tempdir, repo, cleanup := prepareTempdirRepoSrc(t, files) + defer cleanup() + + back := fs.TestChdir(t, tempdir) + defer back() + + // get metadata of current folder + fi := lstat(t, ".") + tempfile := filepath.Join(tempdir, "file") + + statfs := &StatFS{ + FS: fs.Local{}, + OverrideLstat: map[string]os.FileInfo{ + tempfile: fi, + }, + OnlyOverrideStat: true, + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + var tmb tomb.Tomb + + arch := New(repo, fs.Track{FS: statfs}, Options{}) + arch.Error = func(item string, fi os.FileInfo, err error) error { + t.Logf("archiver error as expected for %v: %v", item, err) + return err + } + arch.runWorkers(tmb.Context(ctx), &tmb) + + // fs.Track will panic if the file was not closed + _, excluded, err := arch.Save(ctx, "/", tempfile, nil) + if err == nil { + t.Errorf("Save() should have failed") + } + + if excluded { + t.Errorf("Save() excluded the node, that's unexpected") + } +} From 760863e7f99908cc34ebb758fd8584045fe3d381 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 10 Feb 2020 21:58:00 +0100 Subject: [PATCH 2/2] archiver: Fix TestRacyFileSwap on windows --- internal/archiver/archiver_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 34a0074ac..ce38f52e2 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1991,7 +1991,7 @@ type StatFS struct { func (fs *StatFS) Lstat(name string) (os.FileInfo, error) { if !fs.OnlyOverrideStat { - if fi, ok := fs.OverrideLstat[name]; ok { + if fi, ok := fs.OverrideLstat[fixpath(name)]; ok { return fi, nil } } @@ -2000,7 +2000,7 @@ func (fs *StatFS) Lstat(name string) (os.FileInfo, error) { } func (fs *StatFS) OpenFile(name string, flags int, perm os.FileMode) (fs.File, error) { - if fi, ok := fs.OverrideLstat[name]; ok { + if fi, ok := fs.OverrideLstat[fixpath(name)]; ok { f, err := fs.FS.OpenFile(name, flags, perm) if err != nil { return nil, err