fix(nfs): bound READ sizes and preserve terminal cursor progress

go.mod

@@ -19,3 +19,6 @@ require (
 	github.com/polydawn/rio v0.0.0-20220823181337-7c31ad9831a4 // indirect
 	github.com/warpfork/go-errcat v0.0.0-20180917083543-335044ffc86e // indirect
 )
+
+// Preserve terminal READ cursor progress until the client fix is available upstream.
+replace github.com/willscott/go-nfs-client => github.com/snajpa/go-nfs-client v0.0.0-20261010180736-6436a64df37a

go.sum

@@ -25,12 +25,12 @@ github.com/rasky/go-xdr v0.0.0-20170124162913-1a41d1a06c93/go.mod h1:Nfe4efndBz4
 github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M=
 github.com/smartystreets/assertions v1.2.0 h1:42S6lae5dvLc7BrLu/0ugRtcFVjoJNMC/N3yZFZkDFs=
 github.com/smartystreets/goconvey v1.7.2 h1:9RBaZCeXEQ3UselpuwUQHltGVXvdwm6cv1hgR6gDIPg=
+github.com/snajpa/go-nfs-client v0.0.0-20261010180736-6436a64df37a h1:ksGJkAfNUhqZATloV9tlTik6+xx/Lg3e+txpko5RMX0=
+github.com/snajpa/go-nfs-client v0.0.0-20261010180736-6436a64df37a/go.mod h1:Tq++Lr/FgiS3X48q5FETemXiSLGuYMQT2sPjYNPJSwA=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
 github.com/warpfork/go-errcat v0.0.0-20180917083543-335044ffc86e h1:FIB2fi7XJGHIdf5rWNsfFQqatIKxutT45G+wNuMQNgs=
 github.com/warpfork/go-errcat v0.0.0-20180917083543-335044ffc86e/go.mod h1:/qe02xr3jvTUz8u/PV0FHGpP8t96OQNP7U9BJMwMLEw=
 github.com/warpfork/go-wish v0.0.0-20200122115046-b9ea61034e4a h1:G++j5e0OC488te356JvdhaM8YS6nMsjLAYF7JxCv07w=
-github.com/willscott/go-nfs-client v0.0.0-20240104095149-b44639837b00 h1:U0DnHRZFzoIV1oFEZczg5XyPut9yxk9jjtax/9Bxr/o=
-github.com/willscott/go-nfs-client v0.0.0-20240104095149-b44639837b00/go.mod h1:Tq++Lr/FgiS3X48q5FETemXiSLGuYMQT2sPjYNPJSwA=
 github.com/willscott/memphis v0.0.0-20241203204924-a148a489d367 h1:A9hsyc7kKeultwdUhS99FVq2S4xT6QVZqOEptPGjHpM=
 github.com/willscott/memphis v0.0.0-20241203204924-a148a489d367/go.mod h1:mAQkn9EwN7WZdbH1DnV+9Nmr3oMjPbG4a0zDM2yI2iA=
 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8=

nfs_errors_test.go

@@ -187,6 +187,8 @@ func (r renameErrFS) Rename(oldpath, newpath string) error {
 	return r.Filesystem.Rename(oldpath, newpath)
 }
 
+func (r renameErrFS) Unwrap() billy.Filesystem { return r.Filesystem }
+
 // TestOnRenameStatusFromErrno checks that each errno a rename can report
 // reaches the client as the status RFC 1813 gives RENAME for it, rather than
 // as an NFS3ERR_IO the client can only pass on as EIO.

nfs_onfsinfo.go

@@ -51,8 +51,8 @@ func onFSInfo(ctx context.Context, w *response, userHandle Handler) error {
 	}
 
 	res := fsinfores{
-		Rtmax:       1 << 30,
-		Rtpref:      1 << 30,
+		Rtmax:       MaxRead,
+		Rtpref:      MaxRead,
 		Rtmult:      4096,
 		Wtmax:       1 << 30,
 		Wtpref:      1 << 30,

nfs_onread.go

@@ -52,36 +52,56 @@ func onRead(ctx context.Context, w *response, userHandle Handler) error {
 	defer fh.Close()
 
 	resp := nfsReadResponse{}
+	var fileSize uint64
+	haveSize := false
 
 	if obj.Count > CheckRead {
 		info, err := fs.Stat(fs.Join(path...))
 		if err != nil {
 			return &NFSStatusError{NFSStatusAccess, err}
 		}
-		if info.Size()-int64(obj.Offset) < int64(obj.Count) {
-			obj.Count = uint32(uint64(info.Size()) - obj.Offset)
+		if info.Size() >= 0 {
+			fileSize, haveSize = uint64(info.Size()), true
+			if obj.Offset >= fileSize {
+				obj.Count = 0
+			} else if remaining := fileSize - obj.Offset; remaining < uint64(obj.Count) {
+				obj.Count = uint32(remaining)
+			}
 		}
 	}
 	if obj.Count > MaxRead {
 		obj.Count = MaxRead
 	}
+	// billy.ReaderAt takes a signed offset. A larger protocol offset is
+	// beyond every representable file size and must not wrap negative.
+	if obj.Offset > uint64(1<<63-1) {
+		obj.Count = 0
+		resp.EOF = 1
+	}
 	resp.Data = make([]byte, obj.Count)
 	// todo: multiple reads if size isn't full
-	cnt, err := fh.ReadAt(resp.Data, int64(obj.Offset))
+	var cnt int
+	if obj.Count != 0 {
+		cnt, err = fh.ReadAt(resp.Data, int64(obj.Offset))
+	}
 	if err != nil && !errors.Is(err, io.EOF) {
 		return &NFSStatusError{NFSStatusIO, err}
 	}
 	resp.Count = uint32(cnt)
 	resp.Data = resp.Data[:resp.Count]
-	if errors.Is(err, io.EOF) {
+	attrs := tryStat(fs, path)
+	if attrs != nil && attrs.Type == FileTypeRegular {
+		fileSize, haveSize = attrs.Filesize, true
+	}
+	if errors.Is(err, io.EOF) || haveSize && (obj.Offset >= fileSize || uint64(resp.Count) >= fileSize-obj.Offset) {
 		resp.EOF = 1
 	}
 
 	writer := bytes.NewBuffer([]byte{})
 	if err := xdr.Write(writer, uint32(NFSStatusOk)); err != nil {
 		return &NFSStatusError{NFSStatusServerFault, err}
 	}
-	if err := WritePostOpAttrs(writer, tryStat(fs, path)); err != nil {
+	if err := WritePostOpAttrs(writer, attrs); err != nil {
 		return &NFSStatusError{NFSStatusServerFault, err}
 	}
 

nfs_onrename.go

@@ -32,8 +32,10 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error {
 	if err != nil {
 		return &NFSStatusError{NFSStatusStale, err}
 	}
-	// check the two fs are the same
-	if !reflect.DeepEqual(fs, fs2) {
+	// Middleware can attach a different request context to each lookup. Only
+	// the mounted filesystem's identity determines whether RENAME is allowed;
+	// comparing wrapper contents also races mutable filesystem state.
+	if !sameFilesystem(fs, fs2) {
 		return &NFSStatusError{NFSStatusNotSupp, os.ErrPermission}
 	}
 
@@ -102,3 +104,26 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error {
 	}
 	return nil
 }
+
+func sameFilesystem(left, right billy.Filesystem) bool {
+	unwrap := func(fs billy.Filesystem) billy.Filesystem {
+		for {
+			value := reflect.ValueOf(fs)
+			if !value.IsValid() || (value.Kind() == reflect.Ptr && value.IsNil()) {
+				return nil
+			}
+			wrapper, ok := fs.(interface{ Unwrap() billy.Filesystem })
+			if !ok {
+				return fs
+			}
+			fs = wrapper.Unwrap()
+		}
+	}
+	left, right = unwrap(left), unwrap(right)
+	typ := reflect.TypeOf(left)
+	// Only nonnil pointer instances have object identity we can prove without
+	// reading mutable fields. A struct's type can be comparable while one of
+	// its interface fields contains a non-comparable value, so type-level
+	// comparability alone does not make interface equality safe.
+	return typ != nil && typ == reflect.TypeOf(right) && typ.Kind() == reflect.Ptr && left == right
+}

nfs_onwrite.go

@@ -3,6 +3,7 @@ package nfs
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"math"
 	"os"
@@ -70,6 +71,9 @@ func onWrite(ctx context.Context, w *response, userHandle Handler) error {
 	}
 	if req.Offset > 0 {
 		if _, err := file.Seek(int64(req.Offset), io.SeekStart); err != nil {
+			if closeErr := file.Close(); closeErr != nil {
+				err = fmt.Errorf("%w; closing write file: %v", err, closeErr)
+			}
 			return &NFSStatusError{NFSStatusIO, err}
 		}
 	}
@@ -79,6 +83,9 @@ func onWrite(ctx context.Context, w *response, userHandle Handler) error {
 	}
 	writtenCount, err := file.Write(req.Data[:end])
 	if err != nil {
+		if closeErr := file.Close(); closeErr != nil {
+			err = fmt.Errorf("%w; closing write file: %v", err, closeErr)
+		}
 		Log.Errorf("Error writing: %v", err)
 		return &NFSStatusError{statusFromWriteError(err), err}
 	}