From c4fbf2c779143b7dc73016966128eeadd56af770 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Sun, 6 Jan 2019 14:31:11 +0100 Subject: [PATCH] Return error when reading zero byte from stdin This commit changes the internal file system implementation for reading data from stdin, it now returns an error when no bytes could be read. I think it's worth failing in this case, the user instructed restic to read some data from stdin, and no data was read at all. Maybe it was in a pipe and some earlier stage failed. See #2135 for a short discussion. --- internal/archiver/archiver_test.go | 2 - internal/fs/fs_reader.go | 45 ++++++++++++++++----- internal/fs/fs_reader_test.go | 64 ++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 11 deletions(-) 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) + } + }) + } +}