vfs: make sure write only handles never truncate files they shouldn't

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 {

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)
 }
 

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
 

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
 }
 

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) {