diff --git a/changelog/unreleased/issue-2135 b/changelog/unreleased/issue-2135 new file mode 100644 index 000000000..b689b54f7 --- /dev/null +++ b/changelog/unreleased/issue-2135 @@ -0,0 +1,10 @@ +Bugfix: Return error when no bytes could be read from stdin + +We assume that users reading backup data from stdin want to know when no data +could be read, so now restic returns an error when `backup --stdin` is called +but no bytes could be read. Usually, this means that an earlier command in a +pipe has failed. The documentation was amended and now recommends setting the +`pipefail` option (`set -o pipefail`). + +https://github.com/restic/restic/pull/2135 +https://github.com/restic/restic/pull/2139 diff --git a/doc/040_backup.rst b/doc/040_backup.rst index cb51d5c04..fbffaaa40 100644 --- a/doc/040_backup.rst +++ b/doc/040_backup.rst @@ -289,6 +289,7 @@ this mode of operation, just supply the option ``--stdin`` to the .. code-block:: console + $ set -o pipefail $ mysqldump [...] | restic -r /srv/restic-repo backup --stdin This creates a new snapshot of the output of ``mysqldump``. You can then @@ -302,6 +303,13 @@ specified with ``--stdin-filename``, e.g. like this: $ mysqldump [...] | restic -r /srv/restic-repo backup --stdin --stdin-filename production.sql +The option ``pipefail`` is highly recommended so that a non-zero exit code from +one of the programs in the pipe (e.g. ``mysqldump`` here) makes the whole chain +return a non-zero exit code. Refer to the `Use the Unofficial Bash Strict Mode +`__ for more +details on this. + + Tags for backup *************** diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index e7cda551a..8749dcc81 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -160,7 +160,6 @@ func TestArchiverSaveFileReaderFS(t *testing.T) { var tests = []struct { Data string }{ - {Data: ""}, {Data: "foo"}, {Data: string(restictest.Random(23, 12*1024*1024+1287898))}, } @@ -271,7 +270,6 @@ func TestArchiverSaveReaderFS(t *testing.T) { var tests = []struct { Data string }{ - {Data: ""}, {Data: "foo"}, {Data: string(restictest.Random(23, 12*1024*1024+1287898))}, } diff --git a/internal/fs/fs_reader.go b/internal/fs/fs_reader.go index 385c8f92b..27fc46eb6 100644 --- a/internal/fs/fs_reader.go +++ b/internal/fs/fs_reader.go @@ -1,6 +1,7 @@ package fs import ( + "fmt" "io" "os" "path" @@ -19,10 +20,13 @@ type Reader struct { Name string io.ReadCloser + // for FileInfo Mode os.FileMode ModTime time.Time Size int64 + AllowEmptyFile bool + open sync.Once } @@ -40,7 +44,7 @@ func (fs *Reader) Open(name string) (f File, err error) { switch name { case fs.Name: fs.open.Do(func() { - f = newReaderFile(fs.ReadCloser, fs.fi()) + f = newReaderFile(fs.ReadCloser, fs.fi(), fs.AllowEmptyFile) }) if f == nil { @@ -78,7 +82,7 @@ func (fs *Reader) OpenFile(name string, flag int, perm os.FileMode) (f File, err } fs.open.Do(func() { - f = newReaderFile(fs.ReadCloser, fs.fi()) + f = newReaderFile(fs.ReadCloser, fs.fi(), fs.AllowEmptyFile) }) if f == nil { @@ -158,9 +162,10 @@ func (fs *Reader) Dir(p string) string { return path.Dir(p) } -func newReaderFile(rd io.ReadCloser, fi os.FileInfo) readerFile { - return readerFile{ - ReadCloser: rd, +func newReaderFile(rd io.ReadCloser, fi os.FileInfo, allowEmptyFile bool) *readerFile { + return &readerFile{ + ReadCloser: rd, + AllowEmptyFile: allowEmptyFile, fakeFile: fakeFile{ FileInfo: fi, name: fi.Name(), @@ -170,19 +175,41 @@ func newReaderFile(rd io.ReadCloser, fi os.FileInfo) readerFile { type readerFile struct { io.ReadCloser + AllowEmptyFile, bytesRead bool + fakeFile } -func (r readerFile) Read(p []byte) (int, error) { - return r.ReadCloser.Read(p) +// ErrFileEmpty is returned inside a *os.PathError by Read() for the file +// opened from the fs provided by Reader when no data could be read and +// AllowEmptyFile is not set. +var ErrFileEmpty = errors.New("no data read") + +func (r *readerFile) Read(p []byte) (int, error) { + n, err := r.ReadCloser.Read(p) + if n > 0 { + r.bytesRead = true + } + + // return an error if we did not read any data + if err == io.EOF && !r.AllowEmptyFile && !r.bytesRead { + fmt.Printf("reader: %d bytes read, err %v, bytesRead %v, allowEmpty %v\n", n, err, r.bytesRead, r.AllowEmptyFile) + return n, &os.PathError{ + Path: r.fakeFile.name, + Op: "read", + Err: ErrFileEmpty, + } + } + + return n, err } -func (r readerFile) Close() error { +func (r *readerFile) Close() error { return r.ReadCloser.Close() } // ensure that readerFile implements File -var _ File = readerFile{} +var _ File = &readerFile{} // fakeFile implements all File methods, but only returns errors for anything // except Stat() and Name(). diff --git a/internal/fs/fs_reader_test.go b/internal/fs/fs_reader_test.go index f4cb2bb34..93e8b35ef 100644 --- a/internal/fs/fs_reader_test.go +++ b/internal/fs/fs_reader_test.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "os" "sort" + "strings" "testing" "time" @@ -317,3 +318,66 @@ func TestFSReader(t *testing.T) { }) } } + +func TestFSReaderMinFileSize(t *testing.T) { + var tests = []struct { + name string + data string + allowEmpty bool + readMustErr bool + }{ + { + name: "regular", + data: "foobar", + }, + { + name: "empty", + data: "", + allowEmpty: false, + readMustErr: true, + }, + { + name: "empty2", + data: "", + allowEmpty: true, + readMustErr: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + fs := &Reader{ + Name: "testfile", + ReadCloser: ioutil.NopCloser(strings.NewReader(test.data)), + Mode: 0644, + ModTime: time.Now(), + AllowEmptyFile: test.allowEmpty, + } + + f, err := fs.Open("testfile") + if err != nil { + t.Fatal(err) + } + + buf, err := ioutil.ReadAll(f) + if test.readMustErr { + if err == nil { + t.Fatal("expected error not found, got nil") + } + } else { + if err != nil { + t.Fatal(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) + } + }) + } +}