From ba121eddf0c738df9c204da5e543bfa622cbe33e Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Sat, 14 Sep 2019 13:09:07 +0100 Subject: [PATCH] vfs: make objects of unknown size readable through the VFS These objects (eg Google Docs) appear with 0 length in the VFS. Before this change, these only read 0 bytes. After this change, even though the size appears to be 0, the objects can be read to the end. If the objects are read to the end then the size on the handle will be updated. --- vfs/file_test.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ vfs/read.go | 48 ++++++++++++++++++++++++++++-------------------- 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/vfs/file_test.go b/vfs/file_test.go index c451f2d6c..06219b737 100644 --- a/vfs/file_test.go +++ b/vfs/file_test.go @@ -7,6 +7,8 @@ import ( "testing" "github.com/rclone/rclone/fstest" + "github.com/rclone/rclone/fstest/mockfs" + "github.com/rclone/rclone/fstest/mockobject" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -110,6 +112,51 @@ func TestFileOpenRead(t *testing.T) { require.NoError(t, fd.Close()) } +func TestFileOpenReadUnknownSize(t *testing.T) { + var ( + contents = []byte("file contents") + remote = "file.txt" + ctx = context.Background() + ) + + // create a mock object which returns size -1 + o := mockobject.New(remote).WithContent(contents, mockobject.SeekModeNone) + o.SetUnknownSize(true) + assert.Equal(t, int64(-1), o.Size()) + + // add it to a mock fs + f := mockfs.NewFs("test", "root") + f.AddObject(o) + testObj, err := f.NewObject(ctx, remote) + require.NoError(t, err) + assert.Equal(t, int64(-1), testObj.Size()) + + // create a VFS from that mockfs + vfs := New(f, nil) + + // find the file + node, err := vfs.Stat(remote) + require.NoError(t, err) + require.True(t, node.IsFile()) + file := node.(*File) + + // open it + fd, err := file.openRead() + require.NoError(t, err) + assert.Equal(t, int64(0), fd.Size()) + + // check the contents are not empty even though size is empty + gotContents, err := ioutil.ReadAll(fd) + require.NoError(t, err) + assert.Equal(t, contents, gotContents) + t.Logf("gotContents = %q", gotContents) + + // check that file size has been updated + assert.Equal(t, int64(len(contents)), fd.Size()) + + require.NoError(t, fd.Close()) +} + func TestFileOpenWrite(t *testing.T) { r := fstest.NewRun(t) defer r.Finalise() diff --git a/vfs/read.go b/vfs/read.go index af6b3b487..cd65f0e08 100644 --- a/vfs/read.go +++ b/vfs/read.go @@ -16,19 +16,20 @@ import ( // ReadFileHandle is an open for read file handle on a File type ReadFileHandle struct { baseHandle - done func(err error) - mu sync.Mutex - closed bool // set if handle has been closed - r *accounting.Account - readCalled bool // set if read has been called - size int64 // size of the object - offset int64 // offset of read of o - roffset int64 // offset of Read() calls - noSeek bool - file *File - hash *hash.MultiHasher - opened bool - remote string + done func(err error) + mu sync.Mutex + closed bool // set if handle has been closed + r *accounting.Account + readCalled bool // set if read has been called + size int64 // size of the object (0 for unknown length) + offset int64 // offset of read of o + roffset int64 // offset of Read() calls + noSeek bool + sizeUnknown bool // set if size of source is not known + file *File + hash *hash.MultiHasher + opened bool + remote string } // Check interfaces @@ -51,11 +52,12 @@ func newReadFileHandle(f *File) (*ReadFileHandle, error) { } fh := &ReadFileHandle{ - remote: o.Remote(), - noSeek: f.d.vfs.Opt.NoSeek, - file: f, - hash: mhash, - size: nonNegative(o.Size()), + remote: o.Remote(), + noSeek: f.d.vfs.Opt.NoSeek, + file: f, + hash: mhash, + size: nonNegative(o.Size()), + sizeUnknown: o.Size() < 0, } return fh, nil } @@ -208,6 +210,7 @@ func (fh *ReadFileHandle) ReadAt(p []byte, off int64) (n int, err error) { // Implementation of ReadAt - call with lock held func (fh *ReadFileHandle) readAt(p []byte, off int64) (n int, err error) { + // defer log.Trace(fh.remote, "p[%d], off=%d", len(p), off)("n=%d, err=%v", &n, &err) err = fh.openPending() // FIXME pending open could be more efficient in the presense of seek (and retries) if err != nil { return 0, err @@ -250,7 +253,12 @@ func (fh *ReadFileHandle) readAt(p []byte, off int64) (n int, err error) { // } if err == nil { break - } else if (err == io.ErrUnexpectedEOF || err == io.EOF) && newOffset == fh.size { + } else if (err == io.ErrUnexpectedEOF || err == io.EOF) && (newOffset == fh.size || fh.sizeUnknown) { + if fh.sizeUnknown { + // size is now known since we have read to the end + fh.sizeUnknown = false + fh.size = newOffset + } // Have read to end of file - reset error err = nil break @@ -331,7 +339,7 @@ func (fh *ReadFileHandle) checkHash() error { func (fh *ReadFileHandle) Read(p []byte) (n int, err error) { fh.mu.Lock() defer fh.mu.Unlock() - if fh.roffset >= fh.size { + if fh.roffset >= fh.size && !fh.sizeUnknown { return 0, io.EOF } n, err = fh.readAt(p, fh.roffset)