From 8eff4e0e5cf4c63ffe5e6bf9d5bd2ecf71dd3efb Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 29 Aug 2024 16:32:15 +0200 Subject: [PATCH 1/3] cache: correctly ignore files whose filename is no ID this can for example be the case for temporary files created by the backend implementation. --- internal/backend/cache/backend.go | 5 ++--- internal/backend/cache/backend_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/internal/backend/cache/backend.go b/internal/backend/cache/backend.go index 58b03dd38..3754266ba 100644 --- a/internal/backend/cache/backend.go +++ b/internal/backend/cache/backend.go @@ -231,9 +231,8 @@ func (b *Backend) List(ctx context.Context, t backend.FileType, fn func(f backen wrapFn := func(f backend.FileInfo) error { id, err := restic.ParseID(f.Name) if err != nil { - // returning error here since, if we cannot parse the ID, the file - // is invalid and the list must exit. - return err + // ignore files with invalid name + return nil } ids.Insert(id) diff --git a/internal/backend/cache/backend_test.go b/internal/backend/cache/backend_test.go index dca51c2bf..7f83e40cb 100644 --- a/internal/backend/cache/backend_test.go +++ b/internal/backend/cache/backend_test.go @@ -296,3 +296,20 @@ func TestAutomaticCacheClear(t *testing.T) { t.Errorf("cache doesn't have file2 after list") } } + +func TestAutomaticCacheClearInvalidFilename(t *testing.T) { + be := mem.New() + c := TestNewCache(t) + + data := test.Random(rand.Int(), 42) + h := backend.Handle{ + Type: backend.IndexFile, + Name: "tmp12345", + } + save(t, be, h, data) + + wbe := c.Wrap(be) + + // list all files in the backend + list(t, wbe, func(_ backend.FileInfo) error { return nil }) +} From d19f706d505bec2de16a8319ea627f77e5507a90 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 29 Aug 2024 16:33:18 +0200 Subject: [PATCH 2/3] Add temporary files repositories in integration tests This is intended to catch problems with temporary files stored in the backend, even if the responsible component forgets to test for those. --- cmd/restic/cmd_init_integration_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmd/restic/cmd_init_integration_test.go b/cmd/restic/cmd_init_integration_test.go index 9b5eed6e0..4795d5510 100644 --- a/cmd/restic/cmd_init_integration_test.go +++ b/cmd/restic/cmd_init_integration_test.go @@ -2,6 +2,8 @@ package main import ( "context" + "os" + "path/filepath" "testing" "github.com/restic/restic/internal/repository" @@ -16,6 +18,11 @@ func testRunInit(t testing.TB, opts GlobalOptions) { rtest.OK(t, runInit(context.TODO(), InitOptions{}, opts, nil)) t.Logf("repository initialized at %v", opts.Repo) + + // create temporary junk files to verify that restic does not trip over them + for _, path := range []string{"index", "snapshots", "keys", "locks", filepath.Join("data", "00")} { + rtest.OK(t, os.WriteFile(filepath.Join(opts.Repo, path, "tmp12345"), []byte("junk file"), 0o600)) + } } func TestInitCopyChunkerParams(t *testing.T) { From dd90e1926b785fba2601e1df2752094ad419506c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 29 Aug 2024 16:35:48 +0200 Subject: [PATCH 3/3] use OrderedListOnceBackend where possible --- cmd/restic/cmd_prune_integration_test.go | 5 ++--- cmd/restic/integration_test.go | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/cmd/restic/cmd_prune_integration_test.go b/cmd/restic/cmd_prune_integration_test.go index 746eb5cc9..536ec40d8 100644 --- a/cmd/restic/cmd_prune_integration_test.go +++ b/cmd/restic/cmd_prune_integration_test.go @@ -146,10 +146,9 @@ func TestPruneWithDamagedRepository(t *testing.T) { env.gopts.backendTestHook = oldHook }() // prune should fail - rtest.Assert(t, withTermStatus(env.gopts, func(ctx context.Context, term *termstatus.Terminal) error { + rtest.Equals(t, repository.ErrPacksMissing, withTermStatus(env.gopts, func(ctx context.Context, term *termstatus.Terminal) error { return runPrune(context.TODO(), pruneDefaultOptions, env.gopts, term) - }) == repository.ErrPacksMissing, - "prune should have reported index not complete error") + }), "prune should have reported index not complete error") } // Test repos for edge cases diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index 4cecec6bc..df95031dc 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -80,7 +80,7 @@ func TestListOnce(t *testing.T) { defer cleanup() env.gopts.backendTestHook = func(r backend.Backend) (backend.Backend, error) { - return newListOnceBackend(r), nil + return newOrderedListOnceBackend(r), nil } pruneOpts := PruneOptions{MaxUnused: "0"} checkOpts := CheckOptions{ReadData: true, CheckUnused: true} @@ -148,7 +148,7 @@ func TestFindListOnce(t *testing.T) { defer cleanup() env.gopts.backendTestHook = func(r backend.Backend) (backend.Backend, error) { - return newListOnceBackend(r), nil + return newOrderedListOnceBackend(r), nil } testSetupBackupData(t, env)