Merge pull request #5421 from MichaelEischer/fix-backup-metadata-crash

backup: fix possible crash if directory disappears before metadata collection

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

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
 	}
@@ -718,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)
 	}
 

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
 			}
 
@@ -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:  []string{"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:  []string{"open subdir/targetfile: permission denied"},
 		},
 		{
 			name: "file-subdir-unreadable-ignore-error",
@@ -1929,6 +1930,20 @@ func TestArchiverErrorReporting(t *testing.T) {
 			prepare: chmodUnreadable("subdir/targetfile"),
 			errFn:   ignoreErrorForBasename("targetfile"),
 		},
+		{
+			name:    "parent-dir-missing",
+			targets: []string{"subdir/missing"},
+			src:     TestDir{},
+			errStr:  []string{"stat subdir: no such file or directory", "CreateFile subdir: The system cannot find the file specified"},
+		},
+		{
+			name:    "parent-dir-missing-filtered",
+			targets: []string{"targetfile", "subdir/missing"},
+			src: TestDir{
+				"targetfile": TestFile{Content: "foobar"},
+			},
+			errFn: ignoreErrorForBasename("subdir"),
+		},
 	}
 
 	for _, test := range tests {
@@ -1948,14 +1963,21 @@ 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 {
-					t.Logf("found expected error (%v), skipping further checks", err)
-					return
+			target := test.targets
+			if len(target) == 0 {
+				target = []string{"."}
+			}
+			_, snapshotID, _, err := arch.Snapshot(ctx, target, SnapshotOptions{Time: time.Now()})
+			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 not returned by archiver")
+				t.Fatalf("expected error (%v) not returned by archiver, got (%v)", test.errStr, err)
 				return
 			}