diff --git a/vfs/file.go b/vfs/file.go index 5fa495c2b..33c2f7f26 100644 --- a/vfs/file.go +++ b/vfs/file.go @@ -260,13 +260,13 @@ func (f *File) OpenRead() (fh *ReadFileHandle, err error) { } // OpenWrite open the file for write -func (f *File) OpenWrite() (fh *WriteFileHandle, err error) { +func (f *File) OpenWrite(flags int) (fh *WriteFileHandle, err error) { if f.d.vfs.Opt.ReadOnly { return nil, EROFS } // fs.Debugf(o, "File.OpenWrite") - fh, err = newWriteFileHandle(f.d, f, f.Path()) + fh, err = newWriteFileHandle(f.d, f, f.Path(), flags) if err != nil { err = errors.Wrap(err, "open for write") fs.Errorf(f, "File.OpenWrite failed: %v", err) @@ -393,13 +393,13 @@ func (f *File) Open(flags int) (fd Handle, err error) { // Open write only and hope the user doesn't // want to read. If they do they will get an // EPERM plus an Error log. - fd, err = f.OpenWrite() + fd, err = f.OpenWrite(flags) } } else if write { if CacheMode >= CacheModeWrites { fd, err = f.OpenRW(flags) } else { - fd, err = f.OpenWrite() + fd, err = f.OpenWrite(flags) } } else if read { if CacheMode >= CacheModeFull { diff --git a/vfs/file_test.go b/vfs/file_test.go index 1c9eb9c67..db575154e 100644 --- a/vfs/file_test.go +++ b/vfs/file_test.go @@ -111,7 +111,7 @@ func TestFileOpenWrite(t *testing.T) { defer r.Finalise() vfs, file, _ := fileCreate(t, r) - fd, err := file.OpenWrite() + fd, err := file.OpenWrite(os.O_WRONLY | os.O_TRUNC) require.NoError(t, err) newContents := []byte("this is some new contents") @@ -123,7 +123,7 @@ func TestFileOpenWrite(t *testing.T) { assert.Equal(t, int64(25), file.Size()) vfs.Opt.ReadOnly = true - _, err = file.OpenWrite() + _, err = file.OpenWrite(os.O_WRONLY | os.O_TRUNC) assert.Equal(t, EROFS, err) } diff --git a/vfs/help.go b/vfs/help.go index f1e93e5f4..15cc4f72d 100644 --- a/vfs/help.go +++ b/vfs/help.go @@ -52,7 +52,8 @@ This will mean some operations are not possible * Files can't be opened for both read AND write * Files opened for write can't be seeked - * Files open for read/write with O_TRUNC will be opened write only + * Existing files opened for write must have O_TRUNC set + * Files open for read with O_TRUNC will be opened write only * Files open for write only will behave as if O_TRUNC was supplied * Open modes O_APPEND, O_TRUNC are ignored * If an upload fails it can't be retried @@ -66,7 +67,7 @@ write will be a lot more compatible, but uses the minimal disk space. These operations are not possible * Files opened for write only can't be seeked - * Files open for write only will behave as if O_TRUNC was supplied + * Existing files opened for write must have O_TRUNC set * Files opened for write only will ignore O_APPEND, O_TRUNC * If an upload fails it can't be retried diff --git a/vfs/write.go b/vfs/write.go index 5c68ba540..c34e6bf33 100644 --- a/vfs/write.go +++ b/vfs/write.go @@ -21,6 +21,9 @@ type WriteFileHandle struct { file *File writeCalled bool // set the first time Write() is called offset int64 + opened bool + flags int + truncated bool } // Check interfaces @@ -30,17 +33,38 @@ var ( _ io.Closer = (*WriteFileHandle)(nil) ) -func newWriteFileHandle(d *Dir, f *File, remote string) (*WriteFileHandle, error) { +func newWriteFileHandle(d *Dir, f *File, remote string, flags int) (*WriteFileHandle, error) { fh := &WriteFileHandle{ remote: remote, + flags: flags, result: make(chan error, 1), file: f, } + fh.file.addWriter(fh) + return fh, nil +} + +// returns whether it is OK to truncate the file +func (fh *WriteFileHandle) safeToTruncate() bool { + return fh.truncated || fh.flags&os.O_TRUNC != 0 || !fh.file.exists() +} + +// openPending opens the file if there is a pending open +// +// call with the lock held +func (fh *WriteFileHandle) openPending() (err error) { + if fh.opened { + return nil + } + if !fh.safeToTruncate() { + fs.Errorf(fh.remote, "WriteFileHandle: Can't open for write without O_TRUNC on existing file without cache-mode >= writes") + return EPERM + } var pipeReader *io.PipeReader pipeReader, fh.pipeWriter = io.Pipe() go func() { // NB Rcat deals with Stats.Transferring etc - o, err := fs.Rcat(d.f, remote, pipeReader, time.Now()) + o, err := fs.Rcat(fh.file.d.f, fh.remote, pipeReader, time.Now()) if err != nil { fs.Errorf(fh.remote, "WriteFileHandle.New Rcat failed: %v", err) } @@ -49,10 +73,11 @@ func newWriteFileHandle(d *Dir, f *File, remote string) (*WriteFileHandle, error fh.o = o fh.result <- err }() - fh.file.addWriter(fh) fh.file.setSize(0) - d.addObject(fh.file) // make sure the directory has this object in it now - return fh, nil + fh.truncated = true + fh.file.d.addObject(fh.file) // make sure the directory has this object in it now + fh.opened = true + return nil } // String converts it to printable @@ -97,13 +122,16 @@ func (fh *WriteFileHandle) WriteAt(p []byte, off int64) (n int, err error) { func (fh *WriteFileHandle) writeAt(p []byte, off int64) (n int, err error) { // fs.Debugf(fh.remote, "WriteFileHandle.Write len=%d", len(p)) if fh.closed { - fs.Errorf(fh.remote, "WriteFileHandle.Write error: %v", EBADF) + fs.Errorf(fh.remote, "WriteFileHandle.Write: error: %v", EBADF) return 0, ECLOSED } if fh.offset != off { - fs.Errorf(fh.remote, "WriteFileHandle.Write can't seek in file") + fs.Errorf(fh.remote, "WriteFileHandle.Write: can't seek in file without cache-mode >= writes") return 0, ESPIPE } + if err = fh.openPending(); err != nil { + return 0, err + } fh.writeCalled = true n, err = fh.pipeWriter.Write(p) fh.offset += int64(n) @@ -146,15 +174,22 @@ func (fh *WriteFileHandle) Offset() (offset int64) { // closed already. // // Must be called with fh.mu held -func (fh *WriteFileHandle) close() error { +func (fh *WriteFileHandle) close() (err error) { if fh.closed { return ECLOSED } fh.closed = true // leave writer open until file is transferred defer fh.file.delWriter(fh) + // If file not opened and not safe to truncate then then leave file intact + if !fh.opened && !fh.safeToTruncate() { + return nil + } + if err = fh.openPending(); err != nil { + return err + } writeCloseErr := fh.pipeWriter.Close() - err := <-fh.result + err = <-fh.result if err == nil { fh.file.setObject(fh.o) err = writeCloseErr @@ -244,16 +279,19 @@ func (fh *WriteFileHandle) Truncate(size int64) (err error) { return ECLOSED } if size != fh.offset { - fs.Errorf(fh.remote, "Truncate: Can't change size without cache") + fs.Errorf(fh.remote, "WriteFileHandle: Truncate: Can't change size without cache") return EPERM } // File is correct size + if size == 0 { + fh.truncated = true + } return nil } // Read reads up to len(p) bytes into p. func (fh *WriteFileHandle) Read(p []byte) (n int, err error) { - fs.Errorf(fh.remote, "Read: Can't read and write to file without cache") + fs.Errorf(fh.remote, "WriteFileHandle: Read: Can't read and write to file without cache") return 0, EPERM } @@ -261,7 +299,7 @@ func (fh *WriteFileHandle) Read(p []byte) (n int, err error) { // underlying input source. It returns the number of bytes read (0 <= // n <= len(p)) and any error encountered. func (fh *WriteFileHandle) ReadAt(p []byte, off int64) (n int, err error) { - fs.Errorf(fh.remote, "ReadAt: Can't read and write to file without cache") + fs.Errorf(fh.remote, "WriteFileHandle: ReadAt: Can't read and write to file without cache") return 0, EPERM } diff --git a/vfs/write_test.go b/vfs/write_test.go index 45d66f6b4..883fe15b1 100644 --- a/vfs/write_test.go +++ b/vfs/write_test.go @@ -69,6 +69,12 @@ func TestWriteFileHandleMethods(t *testing.T) { err = fh.Sync() assert.NoError(t, err) + // Truncate - can only truncate where the file pointer is + err = fh.Truncate(5) + assert.NoError(t, err) + err = fh.Truncate(6) + assert.Equal(t, EPERM, err) + // Close assert.NoError(t, fh.Close()) @@ -83,6 +89,35 @@ func TestWriteFileHandleMethods(t *testing.T) { // check the underlying r.Fremote but not the modtime file1 := fstest.NewItem("file1", "hello", t1) fstest.CheckListingWithPrecision(t, r.Fremote, []fstest.Item{file1}, []string{}, fs.ModTimeNotSupported) + + // Check trying to open the file now it exists then closing it + // immediately is OK + h, err := vfs.OpenFile("file1", os.O_WRONLY|os.O_CREATE, 0777) + require.NoError(t, err) + assert.NoError(t, h.Close()) + checkListing(t, root, []string{"file1,5,false"}) + + // Check trying to open the file and writing it now it exists + // returns an error + h, err = vfs.OpenFile("file1", os.O_WRONLY|os.O_CREATE, 0777) + _, err = h.Write([]byte("hello1")) + require.Equal(t, EPERM, err) + assert.NoError(t, h.Close()) + checkListing(t, root, []string{"file1,5,false"}) + + // Check opening the file with O_TRUNC does actually truncate + // it even if we don't write to it + h, err = vfs.OpenFile("file1", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0777) + require.NoError(t, err) + assert.NoError(t, h.Close()) + checkListing(t, root, []string{"file1,0,false"}) + + // Check opening the file with O_TRUNC and writing does work + h, err = vfs.OpenFile("file1", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0777) + _, err = h.WriteString("hello12") + require.NoError(t, err) + assert.NoError(t, h.Close()) + checkListing(t, root, []string{"file1,7,false"}) } func TestWriteFileHandleWriteAt(t *testing.T) {