diff --git a/vfs/file.go b/vfs/file.go index 09b8a85d8..e679984d8 100644 --- a/vfs/file.go +++ b/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 } diff --git a/vfs/write.go b/vfs/write.go index e18f1d001..b820d8172 100644 --- a/vfs/write.go +++ b/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 } diff --git a/vfs/write_test.go b/vfs/write_test.go index b059cdad1..629628555 100644 --- a/vfs/write_test.go +++ b/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)