Merge pull request #5356 from MichaelEischer/fix-backup-stdin-filename

backup: Fix `--stdin-filename` with directory

changelog/unreleased/issue-5324

@@ -0,0 +1,14 @@
+Bugfix: Correctly handle `backup --stdin-filename` with directories
+
+In restic 0.18.0, the `backup` command failed if a filename that includes
+a least a directory was passed to `--stdin-filename`. For example,
+`--stdin-filename /foo/bar` resulted in the following error:
+
+```
+Fatal: unable to save snapshot: open /foo: no such file or directory
+```
+
+This has been fixed now.
+
+https://github.com/restic/restic/issues/5324
+https://github.com/restic/restic/pull/5356

cmd/restic/cmd_backup.go

@@ -591,11 +591,12 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter
 				return err
 			}
 		}
-		targetFS = &fs.Reader{
-			ModTime:    timeStamp,
-			Name:       filename,
-			Mode:       0644,
-			ReadCloser: source,
+		targetFS, err = fs.NewReader(filename, source, fs.ReaderOptions{
+			ModTime: timeStamp,
+			Mode:    0644,
+		})
+		if err != nil {
+			return fmt.Errorf("failed to backup from stdin: %w", err)
 		}
 		targets = []string{filename}
 	}

cmd/restic/cmd_backup_integration_test.go

@@ -632,12 +632,15 @@ func TestStdinFromCommand(t *testing.T) {
 
 	testSetupBackupData(t, env)
 	opts := BackupOptions{
-		StdinCommand:  true,
-		StdinFilename: "stdin",
+		StdinCommand: true,
+		// test that subdirectories are handled correctly
+		StdinFilename: "stdin/subdir/file",
 	}
 
 	testRunBackup(t, filepath.Dir(env.testdata), []string{"python", "-c", "import sys; print('something'); sys.exit(0)"}, opts, env.gopts)
-	testListSnapshots(t, env.gopts, 1)
+	snapshots := testListSnapshots(t, env.gopts, 1)
+	files := testRunLs(t, env.gopts, snapshots[0].String())
+	rtest.Assert(t, includes(files, "/stdin/subdir/file"), "file %q missing from snapshot, got %v", "stdin/subdir/file", files)
 
 	testRunCheck(t, env.gopts)
 }

internal/archiver/archiver_test.go

@@ -174,12 +174,11 @@ func TestArchiverSaveFileReaderFS(t *testing.T) {
 
 			ts := time.Now()
 			filename := "xx"
-			readerFs := &fs.Reader{
-				ModTime:    ts,
-				Mode:       0123,
-				Name:       filename,
-				ReadCloser: io.NopCloser(strings.NewReader(test.Data)),
-			}
+			readerFs, err := fs.NewReader(filename, io.NopCloser(strings.NewReader(test.Data)), fs.ReaderOptions{
+				ModTime: ts,
+				Mode:    0123,
+			})
+			rtest.OK(t, err)
 
 			node, stats := saveFile(t, repo, filename, readerFs)
 
@@ -288,13 +287,11 @@ func TestArchiverSaveReaderFS(t *testing.T) {
 
 			ts := time.Now()
 			filename := "xx"
-			readerFs := &fs.Reader{
-				ModTime:    ts,
-				Mode:       0123,
-				Name:       filename,
-				ReadCloser: io.NopCloser(strings.NewReader(test.Data)),
-			}
-
+			readerFs, err := fs.NewReader(filename, io.NopCloser(strings.NewReader(test.Data)), fs.ReaderOptions{
+				ModTime: ts,
+				Mode:    0123,
+			})
+			rtest.OK(t, err)
 			arch := New(repo, readerFs, Options{})
 			arch.Error = func(item string, err error) error {
 				t.Errorf("archiver error for %v: %v", item, err)

internal/fs/fs_reader.go

@@ -19,97 +19,135 @@ import (
 // be opened once, all subsequent open calls return syscall.EIO. For Lstat(),
 // the provided FileInfo is returned.
 type Reader struct {
-	Name string
-	io.ReadCloser
+	items map[string]readerItem
+}
 
-	// for FileInfo
+type ReaderOptions struct {
 	Mode    os.FileMode
 	ModTime time.Time
 	Size    int64
 
 	AllowEmptyFile bool
+}
 
-	open sync.Once
+type readerItem struct {
+	open           *sync.Once
+	fi             *ExtendedFileInfo
+	rc             io.ReadCloser
+	allowEmptyFile bool
+
+	children []string
 }
 
 // statically ensure that Local implements FS.
 var _ FS = &Reader{}
 
+func NewReader(name string, r io.ReadCloser, opts ReaderOptions) (*Reader, error) {
+	items := make(map[string]readerItem)
+	name = readerCleanPath(name)
+	if name == "/" {
+		return nil, fmt.Errorf("invalid filename specified")
+	}
+
+	isFile := true
+	for {
+		if isFile {
+			fi := &ExtendedFileInfo{
+				Name:    path.Base(name),
+				Mode:    opts.Mode,
+				ModTime: opts.ModTime,
+				Size:    opts.Size,
+			}
+			items[name] = readerItem{
+				open:           &sync.Once{},
+				fi:             fi,
+				rc:             r,
+				allowEmptyFile: opts.AllowEmptyFile,
+			}
+			isFile = false
+		} else {
+			fi := &ExtendedFileInfo{
+				Name:    path.Base(name),
+				Mode:    os.ModeDir | 0755,
+				ModTime: opts.ModTime,
+				Size:    0,
+			}
+			items[name] = readerItem{
+				fi: fi,
+				// keep the children set during the previous iteration
+				children: items[name].children,
+			}
+		}
+
+		parent := path.Dir(name)
+		if parent == name {
+			break
+		}
+		// add the current file to the children of the parent directory
+		item := items[parent]
+		item.children = append(item.children, path.Base(name))
+		items[parent] = item
+
+		name = parent
+	}
+	return &Reader{
+		items: items,
+	}, nil
+}
+
+func readerCleanPath(name string) string {
+	return path.Clean("/" + name)
+}
+
 // VolumeName returns leading volume name, for the Reader file system it's
 // always the empty string.
 func (fs *Reader) VolumeName(_ string) string {
 	return ""
 }
 
-func (fs *Reader) fi() *ExtendedFileInfo {
-	return &ExtendedFileInfo{
-		Name:    fs.Name,
-		Mode:    fs.Mode,
-		ModTime: fs.ModTime,
-		Size:    fs.Size,
-	}
-}
-
 func (fs *Reader) OpenFile(name string, flag int, _ bool) (f File, err error) {
 	if flag & ^(O_RDONLY|O_NOFOLLOW) != 0 {
 		return nil, pathError("open", name,
 			fmt.Errorf("invalid combination of flags 0x%x", flag))
 	}
 
-	switch name {
-	case fs.Name:
-		fs.open.Do(func() {
-			f = newReaderFile(fs.ReadCloser, fs.fi(), fs.AllowEmptyFile)
+	name = readerCleanPath(name)
+	item, ok := fs.items[name]
+	if !ok {
+		return nil, pathError("open", name, syscall.ENOENT)
+	}
+
+	// Check if the path matches our target file
+	if item.rc != nil {
+		item.open.Do(func() {
+			f = newReaderFile(item.rc, item.fi, item.allowEmptyFile)
 		})
 
 		if f == nil {
 			return nil, pathError("open", name, syscall.EIO)
 		}
 
-		return f, nil
-	case "/", ".":
-		f = fakeDir{
-			entries: []string{fs.fi().Name},
-		}
 		return f, nil
 	}
 
-	return nil, pathError("open", name, syscall.ENOENT)
+	f = fakeDir{
+		fakeFile: fakeFile{
+			fi: item.fi,
+		},
+		entries: slices.Clone(item.children),
+	}
+	return f, nil
 }
 
 // Lstat returns the FileInfo structure describing the named file.
-// If the file is a symbolic link, the returned FileInfo
-// describes the symbolic link.  Lstat makes no attempt to follow the link.
 // If there is an error, it will be of type *os.PathError.
 func (fs *Reader) Lstat(name string) (*ExtendedFileInfo, error) {
-	getDirInfo := func(name string) *ExtendedFileInfo {
-		return &ExtendedFileInfo{
-			Name:    fs.Base(name),
-			Size:    0,
-			Mode:    os.ModeDir | 0755,
-			ModTime: time.Now(),
-		}
+	name = readerCleanPath(name)
+	item, ok := fs.items[name]
+	if !ok {
+		return nil, pathError("lstat", name, os.ErrNotExist)
 	}
-
-	switch name {
-	case fs.Name:
-		return fs.fi(), nil
-	case "/", ".":
-		return getDirInfo(name), nil
-	}
-
-	dir := fs.Dir(fs.Name)
-	for {
-		if dir == "/" || dir == "." {
-			break
-		}
-		if name == dir {
-			return getDirInfo(name), nil
-		}
-		dir = fs.Dir(dir)
-	}
-
-	return nil, pathError("lstat", name, os.ErrNotExist)
+	return item.fi, nil
 }
 
 // Join joins any number of path elements into a single path, adding a
@@ -137,7 +175,7 @@ func (fs *Reader) IsAbs(_ string) bool {
 //
 // For the Reader, all paths are absolute.
 func (fs *Reader) Abs(p string) (string, error) {
-	return path.Clean(p), nil
+	return readerCleanPath(p), nil
 }
 
 // Clean returns the cleaned path. For details, see filepath.Clean.

internal/fs/fs_reader_test.go

@@ -17,19 +17,11 @@ import (
 
 func verifyFileContentOpenFile(t testing.TB, fs FS, filename string, want []byte) {
 	f, err := fs.OpenFile(filename, O_RDONLY, false)
-	if err != nil {
-		t.Fatal(err)
-	}
+	test.OK(t, err)
 
 	buf, err := io.ReadAll(f)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = f.Close()
-	if err != nil {
-		t.Fatal(err)
-	}
+	test.OK(t, err)
+	test.OK(t, f.Close())
 
 	if !cmp.Equal(want, buf) {
 		t.Error(cmp.Diff(want, buf))
@@ -38,19 +30,11 @@ func verifyFileContentOpenFile(t testing.TB, fs FS, filename string, want []byte
 
 func verifyDirectoryContents(t testing.TB, fs FS, dir string, want []string) {
 	f, err := fs.OpenFile(dir, O_RDONLY, false)
-	if err != nil {
-		t.Fatal(err)
-	}
+	test.OK(t, err)
 
 	entries, err := f.Readdirnames(-1)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	err = f.Close()
-	if err != nil {
-		t.Fatal(err)
-	}
+	test.OK(t, err)
+	test.OK(t, f.Close())
 
 	sort.Strings(want)
 	sort.Strings(entries)
@@ -69,7 +53,7 @@ func checkFileInfo(t testing.TB, fi *ExtendedFileInfo, filename string, modtime
 		t.Errorf("Mode has wrong value, want 0%o, got 0%o", mode, fi.Mode)
 	}
 
-	if !modtime.Equal(time.Time{}) && !fi.ModTime.Equal(modtime) {
+	if !fi.ModTime.Equal(modtime) {
 		t.Errorf("ModTime has wrong value, want %v, got %v", modtime, fi.ModTime)
 	}
 
@@ -82,40 +66,48 @@ func checkFileInfo(t testing.TB, fi *ExtendedFileInfo, filename string, modtime
 	}
 }
 
-func TestFSReader(t *testing.T) {
-	data := test.Random(55, 1<<18+588)
-	now := time.Now()
-	filename := "foobar"
+type fsTest []struct {
+	name string
+	f    func(t *testing.T, fs FS)
+}
 
-	var tests = []struct {
-		name string
-		f    func(t *testing.T, fs FS)
-	}{
+func createReadDirTest(fpath, filename string) fsTest {
+	return fsTest{
 		{
-			name: "Readdirnames-slash",
+			name: "Readdirnames-slash-" + fpath,
 			f: func(t *testing.T, fs FS) {
-				verifyDirectoryContents(t, fs, "/", []string{filename})
+				verifyDirectoryContents(t, fs, "/"+fpath, []string{filename})
 			},
 		},
 		{
-			name: "Readdirnames-current",
+			name: "Readdirnames-current-" + fpath,
 			f: func(t *testing.T, fs FS) {
-				verifyDirectoryContents(t, fs, ".", []string{filename})
+				verifyDirectoryContents(t, fs, path.Clean(fpath), []string{filename})
 			},
 		},
+	}
+}
+
+func createFileTest(filename string, now time.Time, data []byte) fsTest {
+	return fsTest{
 		{
 			name: "file/OpenFile",
 			f: func(t *testing.T, fs FS) {
 				verifyFileContentOpenFile(t, fs, filename, data)
 			},
 		},
+		{
+			name: "file/Open-error-not-exist",
+			f: func(t *testing.T, fs FS) {
+				_, err := fs.OpenFile(filename+"/other", O_RDONLY, false)
+				test.Assert(t, errors.Is(err, os.ErrNotExist), "unexpected error, got %v, expected %v", err, os.ErrNotExist)
+			},
+		},
 		{
 			name: "file/Lstat",
 			f: func(t *testing.T, fs FS) {
 				fi, err := fs.Lstat(filename)
-				if err != nil {
-					t.Fatal(err)
-				}
+				test.OK(t, err)
 
 				checkFileInfo(t, fi, filename, now, 0644, false)
 			},
@@ -123,91 +115,113 @@ func TestFSReader(t *testing.T) {
 		{
 			name: "file/Stat",
 			f: func(t *testing.T, fs FS) {
-				f, err := fs.OpenFile(filename, O_RDONLY, true)
-				if err != nil {
-					t.Fatal(err)
-				}
-
-				fi, err := f.Stat()
-				if err != nil {
-					t.Fatal(err)
-				}
-
-				err = f.Close()
-				if err != nil {
-					t.Fatal(err)
-				}
-
+				fi := fsOpenAndStat(t, fs, filename, true)
 				checkFileInfo(t, fi, filename, now, 0644, false)
 			},
 		},
-		{
-			name: "dir/Lstat-slash",
-			f: func(t *testing.T, fs FS) {
-				fi, err := fs.Lstat("/")
-				if err != nil {
-					t.Fatal(err)
-				}
+	}
+}
 
-				checkFileInfo(t, fi, "/", time.Time{}, os.ModeDir|0755, true)
+func createDirTest(fpath string, now time.Time) fsTest {
+	return fsTest{
+		{
+			name: "dir/Lstat-slash-" + fpath,
+			f: func(t *testing.T, fs FS) {
+				fi, err := fs.Lstat("/" + fpath)
+				test.OK(t, err)
+
+				checkFileInfo(t, fi, "/"+fpath, now, os.ModeDir|0755, true)
 			},
 		},
 		{
-			name: "dir/Lstat-current",
+			name: "dir/Lstat-current-" + fpath,
 			f: func(t *testing.T, fs FS) {
-				fi, err := fs.Lstat(".")
-				if err != nil {
-					t.Fatal(err)
-				}
+				fi, err := fs.Lstat("./" + fpath)
+				test.OK(t, err)
 
-				checkFileInfo(t, fi, ".", time.Time{}, os.ModeDir|0755, true)
+				checkFileInfo(t, fi, "/"+fpath, now, os.ModeDir|0755, true)
 			},
 		},
 		{
-			name: "dir/Lstat-error-not-exist",
+			name: "dir/Lstat-error-not-exist-" + fpath,
 			f: func(t *testing.T, fs FS) {
-				_, err := fs.Lstat("other")
-				if !errors.Is(err, os.ErrNotExist) {
-					t.Fatal(err)
-				}
+				_, err := fs.Lstat(fpath + "/other")
+				test.Assert(t, errors.Is(err, os.ErrNotExist), "unexpected error, got %v, expected %v", err, os.ErrNotExist)
 			},
 		},
 		{
-			name: "dir/Open-slash",
+			name: "dir/Open-slash-" + fpath,
 			f: func(t *testing.T, fs FS) {
-				fi, err := fs.Lstat("/")
-				if err != nil {
-					t.Fatal(err)
-				}
-
-				checkFileInfo(t, fi, "/", time.Time{}, os.ModeDir|0755, true)
+				fi := fsOpenAndStat(t, fs, "/"+fpath, false)
+				checkFileInfo(t, fi, "/"+fpath, now, os.ModeDir|0755, true)
 			},
 		},
 		{
-			name: "dir/Open-current",
+			name: "dir/Open-current-" + fpath,
 			f: func(t *testing.T, fs FS) {
-				fi, err := fs.Lstat(".")
-				if err != nil {
-					t.Fatal(err)
-				}
-
-				checkFileInfo(t, fi, ".", time.Time{}, os.ModeDir|0755, true)
+				fi := fsOpenAndStat(t, fs, "./"+fpath, false)
+				checkFileInfo(t, fi, "/"+fpath, now, os.ModeDir|0755, true)
 			},
 		},
 	}
+}
 
-	for _, test := range tests {
-		fs := &Reader{
-			Name:       filename,
-			ReadCloser: io.NopCloser(bytes.NewReader(data)),
+func fsOpenAndStat(t *testing.T, fs FS, fpath string, metadataOnly bool) *ExtendedFileInfo {
+	f, err := fs.OpenFile(fpath, O_RDONLY, metadataOnly)
+	test.OK(t, err)
 
+	fi, err := f.Stat()
+	test.OK(t, err)
+	test.OK(t, f.Close())
+	return fi
+}
+
+func TestFSReader(t *testing.T) {
+	data := test.Random(55, 1<<18+588)
+	now := time.Now()
+	filename := "foobar"
+
+	tests := createReadDirTest("", filename)
+	tests = append(tests, createFileTest(filename, now, data)...)
+	tests = append(tests, createDirTest("", now)...)
+
+	for _, tst := range tests {
+		fs, err := NewReader(filename, io.NopCloser(bytes.NewReader(data)), ReaderOptions{
 			Mode:    0644,
 			Size:    int64(len(data)),
 			ModTime: now,
-		}
+		})
+		test.OK(t, err)
 
-		t.Run(test.name, func(t *testing.T) {
-			test.f(t, fs)
+		t.Run(tst.name, func(t *testing.T) {
+			tst.f(t, fs)
+		})
+	}
+}
+
+func TestFSReaderNested(t *testing.T) {
+	data := test.Random(55, 1<<18+588)
+	now := time.Now()
+	filename := "foo/sub/bar"
+
+	tests := createReadDirTest("", "foo")
+	tests = append(tests, createReadDirTest("foo", "sub")...)
+	tests = append(tests, createReadDirTest("foo/sub", "bar")...)
+	tests = append(tests, createFileTest(filename, now, data)...)
+	tests = append(tests, createDirTest("", now)...)
+	tests = append(tests, createDirTest("foo", now)...)
+	tests = append(tests, createDirTest("foo/sub", now)...)
+
+	for _, tst := range tests {
+		fs, err := NewReader(filename, io.NopCloser(bytes.NewReader(data)), ReaderOptions{
+			Mode:    0644,
+			Size:    int64(len(data)),
+			ModTime: now,
+		})
+		test.OK(t, err)
+
+		t.Run(tst.name, func(t *testing.T) {
+			tst.f(t, fs)
 		})
 	}
 }
@@ -230,29 +244,24 @@ func TestFSReaderDir(t *testing.T) {
 		},
 	}
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			fs := &Reader{
-				Name:       test.filename,
-				ReadCloser: io.NopCloser(bytes.NewReader(data)),
-
+	for _, tst := range tests {
+		t.Run(tst.name, func(t *testing.T) {
+			fs, err := NewReader(tst.filename, io.NopCloser(bytes.NewReader(data)), ReaderOptions{
 				Mode:    0644,
 				Size:    int64(len(data)),
 				ModTime: now,
-			}
-
-			dir := path.Dir(fs.Name)
+			})
+			test.OK(t, err)
+			dir := path.Dir(tst.filename)
 			for {
 				if dir == "/" || dir == "." {
 					break
 				}
 
 				fi, err := fs.Lstat(dir)
-				if err != nil {
-					t.Fatal(err)
-				}
+				test.OK(t, err)
 
-				checkFileInfo(t, fi, dir, time.Time{}, os.ModeDir|0755, true)
+				checkFileInfo(t, fi, dir, now, os.ModeDir|0755, true)
 
 				dir = path.Dir(dir)
 			}
@@ -285,40 +294,30 @@ func TestFSReaderMinFileSize(t *testing.T) {
 		},
 	}
 
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			fs := &Reader{
-				Name:           "testfile",
-				ReadCloser:     io.NopCloser(strings.NewReader(test.data)),
+	for _, tst := range tests {
+		t.Run(tst.name, func(t *testing.T) {
+			fs, err := NewReader("testfile", io.NopCloser(strings.NewReader(tst.data)), ReaderOptions{
 				Mode:           0644,
 				ModTime:        time.Now(),
-				AllowEmptyFile: test.allowEmpty,
-			}
-
+				AllowEmptyFile: tst.allowEmpty,
+			})
+			test.OK(t, err)
 			f, err := fs.OpenFile("testfile", O_RDONLY, false)
-			if err != nil {
-				t.Fatal(err)
-			}
+			test.OK(t, err)
 
 			buf, err := io.ReadAll(f)
-			if test.readMustErr {
+			if tst.readMustErr {
 				if err == nil {
 					t.Fatal("expected error not found, got nil")
 				}
 			} else {
-				if err != nil {
-					t.Fatal(err)
-				}
+				test.OK(t, err)
 			}
 
-			if string(buf) != test.data {
-				t.Fatalf("wrong data returned, want %q, got %q", test.data, string(buf))
-			}
-
-			err = f.Close()
-			if err != nil {
-				t.Fatal(err)
+			if string(buf) != tst.data {
+				t.Fatalf("wrong data returned, want %q, got %q", tst.data, string(buf))
 			}
+			test.OK(t, f.Close())
 		})
 	}
 }

internal/restorer/restorer_test.go

@@ -908,13 +908,12 @@ func TestRestorerSparseFiles(t *testing.T) {
 
 	var zeros [1<<20 + 13]byte
 
-	target := &fs.Reader{
-		Mode:       0600,
-		Name:       "/zeros",
-		ReadCloser: io.NopCloser(bytes.NewReader(zeros[:])),
-	}
+	target, err := fs.NewReader("/zeros", io.NopCloser(bytes.NewReader(zeros[:])), fs.ReaderOptions{
+		Mode: 0600,
+	})
+	rtest.OK(t, err)
 	sc := archiver.NewScanner(target)
-	err := sc.Scan(context.TODO(), []string{"/zeros"})
+	err = sc.Scan(context.TODO(), []string{"/zeros"})
 	rtest.OK(t, err)
 
 	arch := archiver.New(repo, target, archiver.Options{})