vfs: fix writing to a read only directory creating spurious directory entries

Before this fix, when a write to a read only directory failed, rclone
would leav spurious directory entries in the directory.

This confuses `rclone serve webdav` into giving this error

    http: superfluous response.WriteHeader

This fixes the VFS layer to remove any directory entries where the
file creation did not succeed.

Fixes #5702

vfs/file.go

@@ -615,10 +615,6 @@ func (f *File) Remove() (err error) {
 		wasWriting = d.vfs.cache.Remove(f.Path())
 	}
 
-	// Remove the item from the directory listing
-	// called with File.mu released
-	d.delObject(f.Name())
-
 	f.muRW.Lock() // muRW must be locked before mu to avoid
 	f.mu.Lock()   // deadlock in RWFileHandle.openPending and .close
 	if f.o != nil {
@@ -635,6 +631,12 @@ func (f *File) Remove() (err error) {
 			fs.Debugf(f._path(), "File.Remove file error: %v", err)
 		}
 	}
+
+	// Remove the item from the directory listing
+	// called with File.mu released when there is no error removing the underlying file
+	if err == nil {
+		d.delObject(f.Name())
+	}
 	return err
 }
 

vfs/write.go

@@ -203,6 +203,11 @@ func (fh *WriteFileHandle) close() (err error) {
 	if err == nil {
 		fh.file.setObject(fh.o)
 		err = writeCloseErr
+	} else {
+		// Remove vfs file entry when no object is present
+		if fh.file.getObject() == nil {
+			_ = fh.file.Remove()
+		}
 	}
 	return err
 }

vfs/write_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"io"
 	"os"
+	"runtime"
 	"sync"
 	"testing"
 	"time"
@@ -28,6 +29,63 @@ func writeHandleCreate(t *testing.T) (r *fstest.Run, vfs *VFS, fh *WriteFileHand
 	return r, vfs, fh
 }
 
+// Test write when underlying storage is readonly, must be run as non-root
+func TestWriteFileHandleReadonly(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skipf("Skipping test on %s", runtime.GOOS)
+	}
+	if *fstest.RemoteName != "" {
+		t.Skip("Skipping test on non local remote")
+	}
+	r, vfs, fh := writeHandleCreate(t)
+
+	// Write a file, so underlying remote will be created
+	_, err := fh.Write([]byte("hello"))
+	assert.NoError(t, err)
+
+	err = fh.Close()
+	assert.NoError(t, err)
+
+	var info os.FileInfo
+	info, err = os.Stat(r.FremoteName)
+	assert.NoError(t, err)
+
+	// Remove write permission
+	oldMode := info.Mode()
+	err = os.Chmod(r.FremoteName, oldMode^(oldMode&0222))
+	assert.NoError(t, err)
+
+	var h Handle
+	h, err = vfs.OpenFile("file2", os.O_WRONLY|os.O_CREATE, 0777)
+	require.NoError(t, err)
+
+	var ok bool
+	fh, ok = h.(*WriteFileHandle)
+	require.True(t, ok)
+
+	// error is propagated to Close()
+	_, err = fh.Write([]byte("hello"))
+	assert.NoError(t, err)
+
+	err = fh.Close()
+	assert.NotNil(t, err)
+
+	// Remove should fail
+	err = vfs.Remove("file1")
+	assert.NotNil(t, err)
+
+	// Only file1 should exist
+	_, err = vfs.Stat("file1")
+	assert.NoError(t, err)
+
+	_, err = vfs.Stat("file2")
+	assert.Equal(t, true, errors.Is(err, os.ErrNotExist))
+
+	// Restore old permission
+	err = os.Chmod(r.FremoteName, oldMode)
+	assert.NoError(t, err)
+}
+
 func TestWriteFileHandleMethods(t *testing.T) {
 	r, vfs, fh := writeHandleCreate(t)