serve nfs: fix writing files via Finder on macOS - fixes #7503

Before this change, writing files to an `nfsmount` via Finder on macOS would
cause critical errors, rendering `nfsmount` effectively unusable on macOS. This
change fixes the issue so that writes via Finder should be possible.

The issue was primarily caused by the handler's HandleLimit being set to -1. -1 is
the correct default for a NullAuthHandler, but not for a CachingHandler, which
interprets -1 not as "no limit" but as "no cache".

This change sets a high default of 1000000, and gives the user control over it
with a new --nfs-cache-handle-limit flag (available in both `serve nfs` and
`nfsmount`. A minimum of 5 is enforced, as any lower than this will be
insufficient to support directory listing.

cmd/mountlib/mount.md

@@ -263,6 +263,10 @@ This method spins up an NFS server using [serve nfs](/commands/rclone_serve_nfs/
 it to the specified mountpoint. If you run this in background mode using |--daemon|, you will need to
 send SIGTERM signal to the rclone process using |kill| command to stop the mount.
 
+Note that `--nfs-cache-handle-limit` controls the maximum number of cached file handles stored by the `nfsmount` caching handler.
+This should not be set too low or you may experience errors when trying to access files. The default is 1000000,
+but consider lowering this limit if the server's system resource usage causes problems.
+
 #### macFUSE Notes
 
 If installing macFUSE using [dmg packages](https://github.com/osxfuse/osxfuse/releases) from

cmd/nfsmount/nfsmount.go

@@ -22,7 +22,8 @@ import (
 )
 
 var (
-	sudo = false
+	sudo         = false
+	nfsServerOpt nfs.Options
 )
 
 func init() {
@@ -33,10 +34,11 @@ func init() {
 	mountlib.AddRc(name, mount)
 	cmdFlags := cmd.Flags()
 	flags.BoolVarP(cmdFlags, &sudo, "sudo", "", sudo, "Use sudo to run the mount command as root.", "")
+	nfs.AddFlags(cmdFlags, &nfsServerOpt)
 }
 
 func mount(VFS *vfs.VFS, mountpoint string, opt *mountlib.Options) (asyncerrors <-chan error, unmount func() error, err error) {
-	s, err := nfs.NewServer(context.Background(), VFS, &nfs.Options{})
+	s, err := nfs.NewServer(context.Background(), VFS, &nfsServerOpt)
 	if err != nil {
 		return
 	}

cmd/serve/nfs/filesystem.go

@@ -22,27 +22,35 @@ type FS struct {
 
 // ReadDir implements read dir
 func (f *FS) ReadDir(path string) (dir []os.FileInfo, err error) {
+	fs.Debugf("nfs", "ReadDir %v", path)
 	return f.vfs.ReadDir(path)
 }
 
 // Create implements creating new files
 func (f *FS) Create(filename string) (billy.File, error) {
+	fs.Debugf("nfs", "Create %v", filename)
 	return f.vfs.Create(filename)
 }
 
 // Open opens a file
 func (f *FS) Open(filename string) (billy.File, error) {
-	return f.vfs.Open(filename)
+	file, err := f.vfs.Open(filename)
+	fs.Debugf("nfs", "Open %v file: %v err: %v", filename, file, err)
+	return file, err
 }
 
 // OpenFile opens a file
 func (f *FS) OpenFile(filename string, flag int, perm os.FileMode) (billy.File, error) {
-	return f.vfs.OpenFile(filename, flag, perm)
+	file, err := f.vfs.OpenFile(filename, flag, perm)
+	fs.Debugf("nfs", "OpenFile %v flag: %v perm: %v file: %v err: %v", filename, flag, perm, file, err)
+	return file, err
 }
 
 // Stat gets the file stat
 func (f *FS) Stat(filename string) (os.FileInfo, error) {
-	return f.vfs.Stat(filename)
+	node, err := f.vfs.Stat(filename)
+	fs.Debugf("nfs", "Stat %v node: %v err: %v", filename, node, err)
+	return node, err
 }
 
 // Rename renames a file
@@ -85,7 +93,9 @@ func (f *FS) MkdirAll(filename string, perm os.FileMode) error {
 
 // Lstat gets the stats for symlink
 func (f *FS) Lstat(filename string) (os.FileInfo, error) {
-	return f.vfs.Stat(filename)
+	node, err := f.vfs.Stat(filename)
+	fs.Debugf("nfs", "Lstat %v node: %v err: %v", filename, node, err)
+	return node, err
 }
 
 // Symlink is not supported over NFS

cmd/serve/nfs/handler.go

@@ -5,22 +5,32 @@ package nfs
 
 import (
 	"context"
+	"fmt"
 	"net"
 
 	"github.com/go-git/go-billy/v5"
+	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/vfs"
 	"github.com/willscott/go-nfs"
 	nfshelper "github.com/willscott/go-nfs/helpers"
 )
 
 // NewBackendAuthHandler creates a handler for the provided filesystem
-func NewBackendAuthHandler(vfs *vfs.VFS) nfs.Handler {
-	return &BackendAuthHandler{vfs}
+func NewBackendAuthHandler(vfs *vfs.VFS, opt *Options) nfs.Handler {
+	handler := &BackendAuthHandler{
+		vfs: vfs,
+		opt: opt,
+	}
+	handler.opt.HandleLimit = handler.opt.Limit()
+	handler.cache = cacheHelper(handler, handler.HandleLimit())
+	return handler
 }
 
 // BackendAuthHandler returns a NFS backing that exposes a given file system in response to all mount requests.
 type BackendAuthHandler struct {
-	vfs *vfs.VFS
+	vfs   *vfs.VFS
+	opt   *Options
+	cache nfs.Handler
 }
 
 // Mount backs Mount RPC Requests, allowing for access control policies.
@@ -50,17 +60,17 @@ func (h *BackendAuthHandler) FSStat(ctx context.Context, f billy.Filesystem, s *
 
 // ToHandle handled by CachingHandler
 func (h *BackendAuthHandler) ToHandle(f billy.Filesystem, s []string) []byte {
-	return []byte{}
+	return h.cache.ToHandle(f, s)
 }
 
 // FromHandle handled by CachingHandler
-func (h *BackendAuthHandler) FromHandle([]byte) (billy.Filesystem, []string, error) {
-	return nil, []string{}, nil
+func (h *BackendAuthHandler) FromHandle(b []byte) (billy.Filesystem, []string, error) {
+	return h.cache.FromHandle(b)
 }
 
 // HandleLimit handled by cachingHandler
 func (h *BackendAuthHandler) HandleLimit() int {
-	return -1
+	return h.opt.HandleLimit
 }
 
 // InvalidateHandle is called on removes or renames
@@ -68,8 +78,136 @@ func (h *BackendAuthHandler) InvalidateHandle(billy.Filesystem, []byte) error {
 	return nil
 }
 
-func newHandler(vfs *vfs.VFS) nfs.Handler {
-	handler := NewBackendAuthHandler(vfs)
-	cacheHelper := nfshelper.NewCachingHandler(handler, 1024)
+func newHandler(vfs *vfs.VFS, opt *Options) nfs.Handler {
+	handler := NewBackendAuthHandler(vfs, opt)
+	nfs.SetLogger(&LogIntercepter{Level: nfs.DebugLevel})
+	return handler
+}
+
+func cacheHelper(handler nfs.Handler, limit int) nfs.Handler {
+	cacheHelper := nfshelper.NewCachingHandler(handler, limit)
 	return cacheHelper
 }
+
+// Limit overrides the --nfs-cache-handle-limit value if out-of-range
+func (o *Options) Limit() int {
+	if o.HandleLimit < 0 {
+		return 1000000
+	}
+	if o.HandleLimit <= 5 {
+		return 5
+	}
+	return o.HandleLimit
+}
+
+// LogIntercepter intercepts noisy go-nfs logs and reroutes them to DEBUG
+type LogIntercepter struct {
+	Level nfs.LogLevel
+}
+
+// Intercept intercepts go-nfs logs and calls fs.Debugf instead
+func (l *LogIntercepter) Intercept(args ...interface{}) {
+	args = append([]interface{}{"[NFS DEBUG] "}, args...)
+	argsS := fmt.Sprint(args...)
+	fs.Debugf(nil, "%v", argsS)
+}
+
+// Interceptf intercepts go-nfs logs and calls fs.Debugf instead
+func (l *LogIntercepter) Interceptf(format string, args ...interface{}) {
+	fs.Debugf(nil, "[NFS DEBUG] "+format, args...)
+}
+
+// Debug reroutes go-nfs Debug messages to Intercept
+func (l *LogIntercepter) Debug(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Debugf reroutes go-nfs Debugf messages to Interceptf
+func (l *LogIntercepter) Debugf(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}
+
+// Error reroutes go-nfs Error messages to Intercept
+func (l *LogIntercepter) Error(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Errorf reroutes go-nfs Errorf messages to Interceptf
+func (l *LogIntercepter) Errorf(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}
+
+// Fatal reroutes go-nfs Fatal messages to Intercept
+func (l *LogIntercepter) Fatal(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Fatalf reroutes go-nfs Fatalf messages to Interceptf
+func (l *LogIntercepter) Fatalf(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}
+
+// GetLevel returns the nfs.LogLevel
+func (l *LogIntercepter) GetLevel() nfs.LogLevel {
+	return l.Level
+}
+
+// Info reroutes go-nfs Info messages to Intercept
+func (l *LogIntercepter) Info(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Infof reroutes go-nfs Infof messages to Interceptf
+func (l *LogIntercepter) Infof(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}
+
+// Panic reroutes go-nfs Panic messages to Intercept
+func (l *LogIntercepter) Panic(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Panicf reroutes go-nfs Panicf messages to Interceptf
+func (l *LogIntercepter) Panicf(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}
+
+// ParseLevel parses the nfs.LogLevel
+func (l *LogIntercepter) ParseLevel(level string) (nfs.LogLevel, error) {
+	return nfs.Log.ParseLevel(level)
+}
+
+// Print reroutes go-nfs Print messages to Intercept
+func (l *LogIntercepter) Print(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Printf reroutes go-nfs Printf messages to Intercept
+func (l *LogIntercepter) Printf(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}
+
+// SetLevel sets the nfs.LogLevel
+func (l *LogIntercepter) SetLevel(level nfs.LogLevel) {
+	l.Level = level
+}
+
+// Trace reroutes go-nfs Trace messages to Intercept
+func (l *LogIntercepter) Trace(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Tracef reroutes go-nfs Tracef messages to Interceptf
+func (l *LogIntercepter) Tracef(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}
+
+// Warn reroutes go-nfs Warn messages to Intercept
+func (l *LogIntercepter) Warn(args ...interface{}) {
+	l.Intercept(args...)
+}
+
+// Warnf reroutes go-nfs Warnf messages to Interceptf
+func (l *LogIntercepter) Warnf(format string, args ...interface{}) {
+	l.Interceptf(format, args...)
+}

cmd/serve/nfs/nfs.go

@@ -25,15 +25,17 @@ import (
 
 // Options contains options for the NFS Server
 type Options struct {
-	ListenAddr string // Port to listen on
+	ListenAddr  string // Port to listen on
+	HandleLimit int    // max file handles cached by go-nfs CachingHandler
 }
 
 var opt Options
 
-// AddFlags adds flags for the sftp
+// AddFlags adds flags for serve nfs (and nfsmount)
 func AddFlags(flagSet *pflag.FlagSet, Opt *Options) {
 	rc.AddOption("nfs", &Opt)
 	flags.StringVarP(flagSet, &Opt.ListenAddr, "addr", "", Opt.ListenAddr, "IPaddress:Port or :Port to bind server to", "")
+	flags.IntVarP(flagSet, &Opt.HandleLimit, "nfs-cache-handle-limit", "", 1000000, "max file handles cached simultaneously (min 5)", "")
 }
 
 func init() {
@@ -72,7 +74,9 @@ NFS mount over local network, you need to specify the listening address and port
 
 Modifying files through NFS protocol requires VFS caching. Usually you will need to specify ` + "`--vfs-cache-mode`" + `
 in order to be able to write to the mountpoint (full is recommended). If you don't specify VFS cache mode,
-the mount will be read-only.
+the mount will be read-only. Note also that ` + "`--nfs-cache-handle-limit`" + ` controls the maximum number of cached file handles stored by the caching handler.
+This should not be set too low or you may experience errors when trying to access files. The default is ` + "`1000000`" + `, but consider lowering this limit if
+the server's system resource usage causes problems.
 
 To serve NFS over the network use following command:
 

cmd/serve/nfs/server.go

@@ -36,7 +36,7 @@ func NewServer(ctx context.Context, vfs *vfs.VFS, opt *Options) (s *Server, err
 		ctx: ctx,
 		opt: *opt,
 	}
-	s.handler = newHandler(vfs)
+	s.handler = newHandler(vfs, opt)
 	s.listener, err = net.Listen("tcp", s.opt.ListenAddr)
 	if err != nil {
 		fs.Errorf(nil, "NFS server failed to listen: %v\n", err)