From a8ce2e45cc2534052304088e394fb8f94f3b9afa Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 12 Jun 2025 22:17:57 +0200 Subject: [PATCH 1/6] backup: do not crash if nodeFromFileInfo fails this could crash in two cases: - if a directory is deleted between restic stating it and trying to list its directory content. - when restic tries to list the parent directory of a backup target, but the parent directory has been deleted. return an error in this case instead. --- internal/archiver/archiver.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 4a6577d27..e509f9f85 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -264,6 +264,11 @@ func (arch *Archiver) trackItem(item string, previous, current *restic.Node, s I // nodeFromFileInfo returns the restic node from an os.FileInfo. func (arch *Archiver) nodeFromFileInfo(snPath, filename string, meta ToNoder, ignoreXattrListError bool) (*restic.Node, error) { node, err := meta.ToNode(ignoreXattrListError) + // node does not exist. This prevents all further processing for this file. + // If an error and a node are returned, then preserve as much data as possible (see below). + if err != nil && node == nil { + return nil, err + } if !arch.WithAtime { node.AccessTime = node.ModTime } From c8bb7bd312c8cad7e49c92e9c18a9979f2e9218f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 12 Jun 2025 22:23:40 +0200 Subject: [PATCH 2/6] backup: do not fail backup is some parent folder is inaccessible Handle errors for parent directories of backup directories in the same way as all other file access errors during a backup. --- internal/archiver/archiver.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index e509f9f85..981b1fecc 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -723,8 +723,14 @@ func (arch *Archiver) saveTree(ctx context.Context, snPath string, atree *tree, arch.trackItem(snItem, oldNode, n, is, time.Since(start)) }) if err != nil { + err = arch.error(join(snPath, name), err) + if err == nil { + // ignore error + continue + } return futureNode{}, 0, err } + nodes = append(nodes, fn) } From 484cdf12e5800cdb4d10c2e13f74b35d67e9c3c6 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 12 Jun 2025 22:37:57 +0200 Subject: [PATCH 3/6] backup: test that missing parent directory is correctly handled --- internal/archiver/archiver_test.go | 39 +++++++++++++++++++----------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 906c83011..9cbce7c76 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1870,12 +1870,13 @@ func TestArchiverErrorReporting(t *testing.T) { } var tests = []struct { - name string - src TestDir - want TestDir - prepare func(t testing.TB) - errFn ErrorFunc - mustError bool + name string + targets []string + src TestDir + want TestDir + prepare func(t testing.TB) + errFn ErrorFunc + errStr string }{ { name: "no-error", @@ -1888,8 +1889,8 @@ func TestArchiverErrorReporting(t *testing.T) { src: TestDir{ "targetfile": TestFile{Content: "foobar"}, }, - prepare: chmodUnreadable("targetfile"), - mustError: true, + prepare: chmodUnreadable("targetfile"), + errStr: "open targetfile: permission denied", }, { name: "file-unreadable-ignore-error", @@ -1910,8 +1911,8 @@ func TestArchiverErrorReporting(t *testing.T) { "targetfile": TestFile{Content: "foobar"}, }, }, - prepare: chmodUnreadable("subdir/targetfile"), - mustError: true, + prepare: chmodUnreadable("subdir/targetfile"), + errStr: "open subdir/targetfile: permission denied", }, { name: "file-subdir-unreadable-ignore-error", @@ -1929,6 +1930,12 @@ func TestArchiverErrorReporting(t *testing.T) { prepare: chmodUnreadable("subdir/targetfile"), errFn: ignoreErrorForBasename("targetfile"), }, + { + name: "parent-dir-missing", + targets: []string{"subdir/missing"}, + src: TestDir{}, + errStr: "stat subdir: no such file or directory", + }, } for _, test := range tests { @@ -1948,14 +1955,18 @@ func TestArchiverErrorReporting(t *testing.T) { arch := New(repo, fs.Track{FS: fs.Local{}}, Options{}) arch.Error = test.errFn - _, snapshotID, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) - if test.mustError { - if err != nil { + target := test.targets + if len(target) == 0 { + target = []string{"."} + } + _, snapshotID, _, err := arch.Snapshot(ctx, target, SnapshotOptions{Time: time.Now()}) + if test.errStr != "" { + if strings.Contains(err.Error(), test.errStr) { t.Logf("found expected error (%v), skipping further checks", err) return } - t.Fatalf("expected error not returned by archiver") + t.Fatalf("expected error (%v) not returned by archiver, got (%v)", test.errStr, err) return } From 350f6452e741fffaa1fa678d1079da301173bb09 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 12 Jun 2025 22:43:45 +0200 Subject: [PATCH 4/6] backup: test that parent directory errors can be correctly filtered --- internal/archiver/archiver_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 9cbce7c76..cc858eeee 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1846,8 +1846,8 @@ func TestArchiverParent(t *testing.T) { func TestArchiverErrorReporting(t *testing.T) { ignoreErrorForBasename := func(basename string) ErrorFunc { return func(item string, err error) error { - if filepath.Base(item) == "targetfile" { - t.Logf("ignoring error for targetfile: %v", err) + if filepath.Base(item) == basename { + t.Logf("ignoring error for %v: %v", basename, err) return nil } @@ -1936,6 +1936,14 @@ func TestArchiverErrorReporting(t *testing.T) { src: TestDir{}, errStr: "stat subdir: no such file or directory", }, + { + name: "parent-dir-missing-filtered", + targets: []string{"targetfile", "subdir/missing"}, + src: TestDir{ + "targetfile": TestFile{Content: "foobar"}, + }, + errFn: ignoreErrorForBasename("subdir"), + }, } for _, test := range tests { From 484b706dd8a47bd9b23070cd71ed3e3389b95d96 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 21 Jul 2025 21:57:05 +0200 Subject: [PATCH 5/6] add changelog --- changelog/unreleased/pull-5421 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/pull-5421 diff --git a/changelog/unreleased/pull-5421 b/changelog/unreleased/pull-5421 new file mode 100644 index 000000000..3a76d4fcd --- /dev/null +++ b/changelog/unreleased/pull-5421 @@ -0,0 +1,8 @@ +Bugfix: Fix rare crash if directory is removed during backup + +In restic 0.18.0, the `backup` command could crash if a directory is removed +inbetween reading its metadata and listing its directory content. + +This has been fixed. + +https://github.com/restic/restic/pull/5421 From 01bc60e96fe67c5d87ab39070468e62d8873dd23 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Fri, 5 Sep 2025 19:35:25 +0200 Subject: [PATCH 6/6] backup: fix test on windows --- internal/archiver/archiver_test.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index cc858eeee..098e13395 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1876,7 +1876,7 @@ func TestArchiverErrorReporting(t *testing.T) { want TestDir prepare func(t testing.TB) errFn ErrorFunc - errStr string + errStr []string }{ { name: "no-error", @@ -1890,7 +1890,7 @@ func TestArchiverErrorReporting(t *testing.T) { "targetfile": TestFile{Content: "foobar"}, }, prepare: chmodUnreadable("targetfile"), - errStr: "open targetfile: permission denied", + errStr: []string{"open targetfile: permission denied"}, }, { name: "file-unreadable-ignore-error", @@ -1912,7 +1912,7 @@ func TestArchiverErrorReporting(t *testing.T) { }, }, prepare: chmodUnreadable("subdir/targetfile"), - errStr: "open subdir/targetfile: permission denied", + errStr: []string{"open subdir/targetfile: permission denied"}, }, { name: "file-subdir-unreadable-ignore-error", @@ -1934,7 +1934,7 @@ func TestArchiverErrorReporting(t *testing.T) { name: "parent-dir-missing", targets: []string{"subdir/missing"}, src: TestDir{}, - errStr: "stat subdir: no such file or directory", + errStr: []string{"stat subdir: no such file or directory", "CreateFile subdir: The system cannot find the file specified"}, }, { name: "parent-dir-missing-filtered", @@ -1968,10 +1968,13 @@ func TestArchiverErrorReporting(t *testing.T) { target = []string{"."} } _, snapshotID, _, err := arch.Snapshot(ctx, target, SnapshotOptions{Time: time.Now()}) - if test.errStr != "" { - if strings.Contains(err.Error(), test.errStr) { - t.Logf("found expected error (%v), skipping further checks", err) - return + if test.errStr != nil { + // check if any of the expected errors are contained in the error message + for _, errStr := range test.errStr { + if strings.Contains(err.Error(), errStr) { + t.Logf("found expected error (%v)", err) + return + } } t.Fatalf("expected error (%v) not returned by archiver, got (%v)", test.errStr, err)