From d9b87814bfa0409e9c801a9db5cab69519e2330d Mon Sep 17 00:00:00 2001 From: Jiri Sveceny Date: Thu, 19 Feb 2026 01:49:14 +0100 Subject: [PATCH 1/7] Add test for reflect.DeepEqual race in searchReverseCache This test detects the race condition at cachinghandler.go:108 where reflect.DeepEqual reads filesystem internal state while another goroutine modifies it through file operations. The test currently FAILS with -race flag, demonstrating the bug. Run with: go test -race -run TestCachingHandlerReflectDeepEqualRace ./helpers/ --- helpers/cachinghandler_test.go | 67 ++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/helpers/cachinghandler_test.go b/helpers/cachinghandler_test.go index 30ec02f..ceca719 100644 --- a/helpers/cachinghandler_test.go +++ b/helpers/cachinghandler_test.go @@ -5,6 +5,7 @@ import ( "sync" "testing" + "github.com/go-git/go-billy/v5" "github.com/willscott/go-nfs/helpers/memfs" ) @@ -128,3 +129,69 @@ func TestCachingHandlerConcurrentInvalidateHandle(t *testing.T) { wg.Wait() } + +// TestCachingHandlerReflectDeepEqualRace tests for the race condition in +// searchReverseCache where reflect.DeepEqual reads filesystem internal state +// while another goroutine modifies it through file operations. +// +// The race occurs at line 108: reflect.DeepEqual(candidate.f, f) +// reflect.DeepEqual traverses all internal fields of the filesystem objects, +// including mutable maps that can be modified concurrently. +// +// Run with: go test -race -run TestCachingHandlerReflectDeepEqualRace ./helpers/ +func TestCachingHandlerReflectDeepEqualRace(t *testing.T) { + // Create multiple filesystem instances to trigger reflect.DeepEqual comparisons + const numFilesystems = 5 + filesystems := make([]billy.Filesystem, numFilesystems) + for i := range filesystems { + filesystems[i] = memfs.New() + } + + handler := NewNullAuthHandler(filesystems[0]) + cacheHandler := NewCachingHandler(handler, 1024).(*CachingHandler) + + const numGoroutines = 10 + const numOperations = 100 + + // Pre-populate cache with handles from different filesystems + for _, fs := range filesystems { + for j := 0; j < 10; j++ { + path := []string{fmt.Sprintf("shared-%d.txt", j)} + _ = cacheHandler.ToHandle(fs, path) + } + } + + var wg sync.WaitGroup + wg.Add(numGoroutines * 2) + + // Group 1: ToHandle calls triggering reflect.DeepEqual in searchReverseCache + for i := 0; i < numGoroutines; i++ { + go func(id int) { + defer wg.Done() + for j := 0; j < numOperations; j++ { + fs := filesystems[j%numFilesystems] + path := []string{fmt.Sprintf("shared-%d.txt", j%10)} + _ = cacheHandler.ToHandle(fs, path) + } + }(i) + } + + // Group 2: File operations modifying filesystem internal state + // This races with reflect.DeepEqual reading that state + for i := 0; i < numGoroutines; i++ { + go func(id int) { + defer wg.Done() + fs := filesystems[id%numFilesystems] + for j := 0; j < numOperations; j++ { + filename := fmt.Sprintf("/file-%d-%d.txt", id, j) + f, err := fs.Create(filename) + if err == nil { + _, _ = f.Write([]byte("data")) + _ = f.Close() + } + } + }(i) + } + + wg.Wait() +} From 77a455441da47d5db8ce1fa727e7b621ad0bf509 Mon Sep 17 00:00:00 2001 From: Jiri Sveceny Date: Thu, 19 Feb 2026 01:50:42 +0100 Subject: [PATCH 2/7] Fix reflect.DeepEqual race in searchReverseCache Replace reflect.DeepEqual(candidate.f, f) with interface comparison (==). Problem: - reflect.DeepEqual traverses all internal fields of filesystem objects - This includes mutable maps (storage.files, storage.children) - When another goroutine modifies the filesystem (Create, Write, etc.), those maps are modified while reflect.DeepEqual is reading them - Result: DATA RACE Solution: - Use interface comparison: candidate.f == f - Interface == compares type and underlying pointer only - Does not read internal state, so no race - Semantically correct: we want to check if it's the same FS instance The test was adjusted to use separate filesystems per writer goroutine to avoid triggering unrelated races in memfs (which is not thread-safe). --- helpers/cachinghandler.go | 9 +++++-- helpers/cachinghandler_test.go | 43 ++++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/helpers/cachinghandler.go b/helpers/cachinghandler.go index 97299d5..ea20d83 100644 --- a/helpers/cachinghandler.go +++ b/helpers/cachinghandler.go @@ -4,7 +4,6 @@ import ( "crypto/sha256" "encoding/binary" "io/fs" - "reflect" "sync" "github.com/willscott/go-nfs" @@ -105,7 +104,13 @@ func (c *CachingHandler) searchReverseCache(f billy.Filesystem, path string) []b for _, id := range uuids { if candidate, ok := c.activeHandles.Get(id); ok { - if reflect.DeepEqual(candidate.f, f) { + // Use interface comparison instead of reflect.DeepEqual to avoid + // race conditions. reflect.DeepEqual traverses all internal fields + // of the filesystem, including mutable maps that can be modified + // concurrently by file operations. Interface comparison (==) only + // compares type and pointer, which is sufficient for checking if + // it's the same filesystem instance. + if candidate.f == f { return id[:] } } diff --git a/helpers/cachinghandler_test.go b/helpers/cachinghandler_test.go index ceca719..fbc2ac5 100644 --- a/helpers/cachinghandler_test.go +++ b/helpers/cachinghandler_test.go @@ -138,25 +138,31 @@ func TestCachingHandlerConcurrentInvalidateHandle(t *testing.T) { // reflect.DeepEqual traverses all internal fields of the filesystem objects, // including mutable maps that can be modified concurrently. // +// Note: This test uses separate filesystems per writer goroutine to avoid +// triggering races in memfs itself (which is not thread-safe). +// // Run with: go test -race -run TestCachingHandlerReflectDeepEqualRace ./helpers/ func TestCachingHandlerReflectDeepEqualRace(t *testing.T) { - // Create multiple filesystem instances to trigger reflect.DeepEqual comparisons - const numFilesystems = 5 - filesystems := make([]billy.Filesystem, numFilesystems) - for i := range filesystems { - filesystems[i] = memfs.New() + // Create filesystem instances - one shared for reads, others for writes + const numWriterFS = 10 + readerFS := memfs.New() + writerFilesystems := make([]billy.Filesystem, numWriterFS) + for i := range writerFilesystems { + writerFilesystems[i] = memfs.New() } - handler := NewNullAuthHandler(filesystems[0]) + handler := NewNullAuthHandler(readerFS) cacheHandler := NewCachingHandler(handler, 1024).(*CachingHandler) const numGoroutines = 10 const numOperations = 100 - // Pre-populate cache with handles from different filesystems - for _, fs := range filesystems { - for j := 0; j < 10; j++ { - path := []string{fmt.Sprintf("shared-%d.txt", j)} + // Pre-populate cache with handles from all filesystems using same paths + // This ensures searchReverseCache will compare different FS instances + for j := 0; j < 10; j++ { + path := []string{fmt.Sprintf("shared-%d.txt", j)} + _ = cacheHandler.ToHandle(readerFS, path) + for _, fs := range writerFilesystems { _ = cacheHandler.ToHandle(fs, path) } } @@ -164,24 +170,31 @@ func TestCachingHandlerReflectDeepEqualRace(t *testing.T) { var wg sync.WaitGroup wg.Add(numGoroutines * 2) - // Group 1: ToHandle calls triggering reflect.DeepEqual in searchReverseCache + // Group 1: ToHandle calls triggering filesystem comparisons in searchReverseCache + // With reflect.DeepEqual, this would race with writers modifying FS internals for i := 0; i < numGoroutines; i++ { go func(id int) { defer wg.Done() for j := 0; j < numOperations; j++ { - fs := filesystems[j%numFilesystems] + // Alternate between reader and writer filesystems + var fs billy.Filesystem + if j%2 == 0 { + fs = readerFS + } else { + fs = writerFilesystems[j%numWriterFS] + } path := []string{fmt.Sprintf("shared-%d.txt", j%10)} _ = cacheHandler.ToHandle(fs, path) } }(i) } - // Group 2: File operations modifying filesystem internal state - // This races with reflect.DeepEqual reading that state + // Group 2: File operations on dedicated filesystems (one per goroutine) + // Each goroutine has its own filesystem to avoid memfs internal races for i := 0; i < numGoroutines; i++ { go func(id int) { defer wg.Done() - fs := filesystems[id%numFilesystems] + fs := writerFilesystems[id%numWriterFS] for j := 0; j < numOperations; j++ { filename := fmt.Sprintf("/file-%d-%d.txt", id, j) f, err := fs.Create(filename) From 2a1e8883342625f3fa035193ef11ac910c7379e0 Mon Sep 17 00:00:00 2001 From: Jiri Sveceny Date: Thu, 19 Feb 2026 01:51:14 +0100 Subject: [PATCH 3/7] Add test for slice reference race in getReverseHandles This test detects the race condition where getReverseHandles returns a slice reference that can be modified while being iterated. The race occurs because: 1. getReverseHandles returns c.reverseHandles[path] (slice reference) 2. After RLock is released, searchReverseCache iterates over this slice 3. Concurrent appendReverseHandle/evictReverseCache modify the same slice The test currently FAILS with -race flag, demonstrating the bug. Run with: go test -race -run TestCachingHandlerSliceReferenceRace ./helpers/ --- helpers/cachinghandler_test.go | 43 ++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/helpers/cachinghandler_test.go b/helpers/cachinghandler_test.go index fbc2ac5..8396c79 100644 --- a/helpers/cachinghandler_test.go +++ b/helpers/cachinghandler_test.go @@ -208,3 +208,46 @@ func TestCachingHandlerReflectDeepEqualRace(t *testing.T) { wg.Wait() } + +// TestCachingHandlerSliceReferenceRace tests for the race condition where +// getReverseHandles returns a slice reference that can be modified while +// being iterated in searchReverseCache. +// +// The race occurs because: +// 1. getReverseHandles returns c.reverseHandles[path] - a reference to the slice +// 2. After releasing RLock, searchReverseCache iterates over this slice +// 3. Concurrent appendReverseHandle/evictReverseCache modify the same slice +// +// Run with: go test -race -run TestCachingHandlerSliceReferenceRace ./helpers/ +func TestCachingHandlerSliceReferenceRace(t *testing.T) { + mem := memfs.New() + handler := NewNullAuthHandler(mem) + cacheHandler := NewCachingHandler(handler, 1024).(*CachingHandler) + + const numGoroutines = 20 + const numOperations = 500 + + var wg sync.WaitGroup + wg.Add(numGoroutines) + + // All goroutines use the same small set of paths to maximize contention + // on the same reverseHandles slice entries + for i := 0; i < numGoroutines; i++ { + go func(id int) { + defer wg.Done() + for j := 0; j < numOperations; j++ { + // Use only 5 unique paths to maximize slice contention + path := []string{fmt.Sprintf("race-test-%d.txt", j%5)} + handle := cacheHandler.ToHandle(mem, path) + + // Occasionally invalidate to trigger evictReverseCache + // while other goroutines are in searchReverseCache + if j%7 == 0 { + _ = cacheHandler.InvalidateHandle(mem, handle) + } + } + }(i) + } + + wg.Wait() +} From 377638440f6370e806a0e3d98c03ed51bd9d1d56 Mon Sep 17 00:00:00 2001 From: Jiri Sveceny Date: Thu, 19 Feb 2026 01:51:51 +0100 Subject: [PATCH 4/7] Fix slice reference race by holding RLock during iteration Instead of returning a slice reference from getReverseHandles and iterating outside the lock, hold RLock for the entire iteration in searchReverseCache. Problem: - getReverseHandles returned c.reverseHandles[path] (direct reference) - After RLock was released, the slice could be modified by concurrent appendReverseHandle (append) or evictReverseCache (slice copy) - Result: DATA RACE when iterating in searchReverseCache Solution: - Hold RLock for entire iteration in searchReverseCache - Access c.reverseHandles[path] directly within the lock - Remove unused getReverseHandles function This is more efficient than copying the slice on every call, and writers (appendReverseHandle, evictReverseCache) will simply wait for the short iteration to complete. Safe because: activeHandles.Get() has its own internal locking (LRU). --- helpers/cachinghandler.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/helpers/cachinghandler.go b/helpers/cachinghandler.go index ea20d83..71fc5fe 100644 --- a/helpers/cachinghandler.go +++ b/helpers/cachinghandler.go @@ -100,9 +100,13 @@ func (c *CachingHandler) FromHandle(fh []byte) (billy.Filesystem, []string, erro } func (c *CachingHandler) searchReverseCache(f billy.Filesystem, path string) []byte { - uuids := c.getReverseHandles(path) + // Hold RLock for entire iteration to prevent races with appendReverseHandle + // and evictReverseCache which modify the slice. This is safe because + // activeHandles.Get() has its own internal synchronization (LRU cache). + c.reverseHandlesMu.RLock() + defer c.reverseHandlesMu.RUnlock() - for _, id := range uuids { + for _, id := range c.reverseHandles[path] { if candidate, ok := c.activeHandles.Get(id); ok { // Use interface comparison instead of reflect.DeepEqual to avoid // race conditions. reflect.DeepEqual traverses all internal fields @@ -135,12 +139,6 @@ func (c *CachingHandler) evictReverseCache(path string, handle uuid.UUID) { } } -func (c *CachingHandler) getReverseHandles(path string) []uuid.UUID { - c.reverseHandlesMu.RLock() - defer c.reverseHandlesMu.RUnlock() - return c.reverseHandles[path] -} - func (c *CachingHandler) appendReverseHandle(path string, id uuid.UUID) { c.reverseHandlesMu.Lock() defer c.reverseHandlesMu.Unlock() From 8b8e0fbe91de7f144ebe64f0244162cbc87fd6e6 Mon Sep 17 00:00:00 2001 From: Joe Lombrozo Date: Tue, 17 Mar 2026 11:36:17 -0700 Subject: [PATCH 5/7] add some observability hooks --- conn.go | 11 +++++++++++ server.go | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/conn.go b/conn.go index 6ed6d03..7a0eb52 100644 --- a/conn.go +++ b/conn.go @@ -45,6 +45,17 @@ type conn struct { func (c *conn) serve(ctx context.Context) { connCtx, cancel := context.WithCancel(ctx) defer cancel() + + if hook := c.OnConnect; hook != nil { + connCtx = hook(connCtx, c.Conn) + } + + defer func() { + if hook := c.OnDisconnect; hook != nil { + hook(connCtx, c.Conn) + } + }() + c.writeSerializer = make(chan []byte, 1) go c.serializeWrites(connCtx) diff --git a/server.go b/server.go index f3c4db2..ece7ecd 100644 --- a/server.go +++ b/server.go @@ -14,6 +14,9 @@ type Server struct { Handler ID [8]byte context.Context + + OnConnect func(ctx context.Context, conn net.Conn) context.Context + OnDisconnect func(ctx context.Context, conn net.Conn) } // RegisterMessageHandler registers a handler for a specific @@ -76,6 +79,7 @@ func (s *Server) Serve(l net.Listener) error { return err } tempDelay = 0 + c := s.newConn(conn) go c.serve(baseCtx) } From 810c58b8647e88a63f48477222ac0728d38bce64 Mon Sep 17 00:00:00 2001 From: Joe Lombrozo Date: Tue, 17 Mar 2026 11:55:44 -0700 Subject: [PATCH 6/7] add ctx to more functions --- handler.go | 8 ++++---- helpers/cachinghandler.go | 7 ++++--- helpers/cachinghandler_test.go | 26 +++++++++++++------------- helpers/nullauthhandler.go | 8 ++++---- mount.go | 2 +- nfs_onaccess.go | 2 +- nfs_oncommit.go | 2 +- nfs_oncreate.go | 6 +++--- nfs_onfsinfo.go | 2 +- nfs_onfsstat.go | 2 +- nfs_ongetattr.go | 2 +- nfs_onlink.go | 6 +++--- nfs_onlookup.go | 6 +++--- nfs_onmkdir.go | 6 +++--- nfs_onmknod.go | 6 +++--- nfs_onpathconf.go | 2 +- nfs_onread.go | 2 +- nfs_onreaddir.go | 8 ++++---- nfs_onreaddirplus.go | 6 +++--- nfs_onreadlink.go | 2 +- nfs_onremove.go | 6 +++--- nfs_onrename.go | 8 ++++---- nfs_onsetattr.go | 4 ++-- nfs_onsymlink.go | 6 +++--- nfs_onwrite.go | 2 +- 25 files changed, 69 insertions(+), 68 deletions(-) diff --git a/handler.go b/handler.go index 13a0eaf..dc72d43 100644 --- a/handler.go +++ b/handler.go @@ -16,7 +16,7 @@ type Handler interface { // Change can return 'nil' if filesystem is read-only // If the returned value can be cast to `UnixChange`, mknod and link RPCs will be available. - Change(billy.Filesystem) billy.Change + Change(context.Context, billy.Filesystem) billy.Change // Optional methods - generic helpers or trivial implementations can be sufficient depending on use case. @@ -25,9 +25,9 @@ type Handler interface { // represent file objects as opaque references // Can be safely implemented via helpers/cachinghandler. - ToHandle(fs billy.Filesystem, path []string) []byte - FromHandle(fh []byte) (billy.Filesystem, []string, error) - InvalidateHandle(billy.Filesystem, []byte) error + ToHandle(cxt context.Context, fs billy.Filesystem, path []string) []byte + FromHandle(ctx context.Context, fh []byte) (billy.Filesystem, []string, error) + InvalidateHandle(context.Context, billy.Filesystem, []byte) error // How many handles can be safely maintained by the handler. HandleLimit() int diff --git a/helpers/cachinghandler.go b/helpers/cachinghandler.go index 71fc5fe..73647eb 100644 --- a/helpers/cachinghandler.go +++ b/helpers/cachinghandler.go @@ -1,6 +1,7 @@ package helpers import ( + "context" "crypto/sha256" "encoding/binary" "io/fs" @@ -52,7 +53,7 @@ type entry struct { // ToHandle takes a file and represents it with an opaque handle to reference it. // In stateless nfs (when it's serving a unix fs) this can be the device + inode // but we can generalize with a stateful local cache of handed out IDs. -func (c *CachingHandler) ToHandle(f billy.Filesystem, path []string) []byte { +func (c *CachingHandler) ToHandle(ctx context.Context, f billy.Filesystem, path []string) []byte { joinedPath := f.Join(path...) if handle := c.searchReverseCache(f, joinedPath); handle != nil { @@ -77,7 +78,7 @@ func (c *CachingHandler) ToHandle(f billy.Filesystem, path []string) []byte { } // FromHandle converts from an opaque handle to the file it represents -func (c *CachingHandler) FromHandle(fh []byte) (billy.Filesystem, []string, error) { +func (c *CachingHandler) FromHandle(ctx context.Context, fh []byte) (billy.Filesystem, []string, error) { id, err := uuid.FromBytes(fh) if err != nil { return nil, []string{}, err @@ -145,7 +146,7 @@ func (c *CachingHandler) appendReverseHandle(path string, id uuid.UUID) { c.reverseHandles[path] = append(c.reverseHandles[path], id) } -func (c *CachingHandler) InvalidateHandle(fs billy.Filesystem, handle []byte) error { +func (c *CachingHandler) InvalidateHandle(ctx context.Context, fs billy.Filesystem, handle []byte) error { //Remove from cache id, _ := uuid.FromBytes(handle) entry, ok := c.activeHandles.Get(id) diff --git a/helpers/cachinghandler_test.go b/helpers/cachinghandler_test.go index 8396c79..70cce64 100644 --- a/helpers/cachinghandler_test.go +++ b/helpers/cachinghandler_test.go @@ -31,11 +31,11 @@ func TestCachingHandlerConcurrentToHandle(t *testing.T) { // Each goroutine creates handles for different paths // but also accesses some shared paths to maximize contention path := []string{fmt.Sprintf("file-%d-%d.txt", id, j)} - _ = cacheHandler.ToHandle(mem, path) + _ = cacheHandler.ToHandle(t.Context(), mem, path) // Also access a shared path to increase contention sharedPath := []string{fmt.Sprintf("shared-%d.txt", j%10)} - _ = cacheHandler.ToHandle(mem, sharedPath) + _ = cacheHandler.ToHandle(t.Context(), mem, sharedPath) } }(i) } @@ -57,7 +57,7 @@ func TestCachingHandlerConcurrentToHandleAndFromHandle(t *testing.T) { handles := make([][]byte, 20) for i := 0; i < 20; i++ { path := []string{fmt.Sprintf("precreated-%d.txt", i)} - handles[i] = cacheHandler.ToHandle(mem, path) + handles[i] = cacheHandler.ToHandle(t.Context(), mem, path) } var wg sync.WaitGroup @@ -69,7 +69,7 @@ func TestCachingHandlerConcurrentToHandleAndFromHandle(t *testing.T) { defer wg.Done() for j := 0; j < numOperations; j++ { path := []string{fmt.Sprintf("new-file-%d-%d.txt", id, j)} - _ = cacheHandler.ToHandle(mem, path) + _ = cacheHandler.ToHandle(t.Context(), mem, path) } }(i) } @@ -80,7 +80,7 @@ func TestCachingHandlerConcurrentToHandleAndFromHandle(t *testing.T) { defer wg.Done() for j := 0; j < numOperations; j++ { handle := handles[j%len(handles)] - _, _, _ = cacheHandler.FromHandle(handle) + _, _, _ = cacheHandler.FromHandle(t.Context(), handle) } }(i) } @@ -107,10 +107,10 @@ func TestCachingHandlerConcurrentInvalidateHandle(t *testing.T) { defer wg.Done() for j := 0; j < numOperations; j++ { path := []string{fmt.Sprintf("invalidate-%d-%d.txt", id, j)} - handle := cacheHandler.ToHandle(mem, path) + handle := cacheHandler.ToHandle(t.Context(), mem, path) // Immediately invalidate some handles if j%3 == 0 { - _ = cacheHandler.InvalidateHandle(mem, handle) + _ = cacheHandler.InvalidateHandle(t.Context(), mem, handle) } } }(i) @@ -122,7 +122,7 @@ func TestCachingHandlerConcurrentInvalidateHandle(t *testing.T) { defer wg.Done() for j := 0; j < numOperations; j++ { sharedPath := []string{fmt.Sprintf("shared-invalidate-%d.txt", j%20)} - _ = cacheHandler.ToHandle(mem, sharedPath) + _ = cacheHandler.ToHandle(t.Context(), mem, sharedPath) } }(i) } @@ -161,9 +161,9 @@ func TestCachingHandlerReflectDeepEqualRace(t *testing.T) { // This ensures searchReverseCache will compare different FS instances for j := 0; j < 10; j++ { path := []string{fmt.Sprintf("shared-%d.txt", j)} - _ = cacheHandler.ToHandle(readerFS, path) + _ = cacheHandler.ToHandle(t.Context(), readerFS, path) for _, fs := range writerFilesystems { - _ = cacheHandler.ToHandle(fs, path) + _ = cacheHandler.ToHandle(t.Context(), fs, path) } } @@ -184,7 +184,7 @@ func TestCachingHandlerReflectDeepEqualRace(t *testing.T) { fs = writerFilesystems[j%numWriterFS] } path := []string{fmt.Sprintf("shared-%d.txt", j%10)} - _ = cacheHandler.ToHandle(fs, path) + _ = cacheHandler.ToHandle(t.Context(), fs, path) } }(i) } @@ -238,12 +238,12 @@ func TestCachingHandlerSliceReferenceRace(t *testing.T) { for j := 0; j < numOperations; j++ { // Use only 5 unique paths to maximize slice contention path := []string{fmt.Sprintf("race-test-%d.txt", j%5)} - handle := cacheHandler.ToHandle(mem, path) + handle := cacheHandler.ToHandle(t.Context(), mem, path) // Occasionally invalidate to trigger evictReverseCache // while other goroutines are in searchReverseCache if j%7 == 0 { - _ = cacheHandler.InvalidateHandle(mem, handle) + _ = cacheHandler.InvalidateHandle(t.Context(), mem, handle) } } }(i) diff --git a/helpers/nullauthhandler.go b/helpers/nullauthhandler.go index f7571b1..4f7dd60 100644 --- a/helpers/nullauthhandler.go +++ b/helpers/nullauthhandler.go @@ -27,7 +27,7 @@ func (h *NullAuthHandler) Mount(ctx context.Context, conn net.Conn, req nfs.Moun } // Change provides an interface for updating file attributes. -func (h *NullAuthHandler) Change(fs billy.Filesystem) billy.Change { +func (h *NullAuthHandler) Change(ctx context.Context, fs billy.Filesystem) billy.Change { if c, ok := h.fs.(billy.Change); ok { return c } @@ -40,16 +40,16 @@ func (h *NullAuthHandler) FSStat(ctx context.Context, f billy.Filesystem, s *nfs } // ToHandle handled by CachingHandler -func (h *NullAuthHandler) ToHandle(f billy.Filesystem, s []string) []byte { +func (h *NullAuthHandler) ToHandle(ctx context.Context, f billy.Filesystem, s []string) []byte { return []byte{} } // FromHandle handled by CachingHandler -func (h *NullAuthHandler) FromHandle([]byte) (billy.Filesystem, []string, error) { +func (h *NullAuthHandler) FromHandle(context.Context, []byte) (billy.Filesystem, []string, error) { return nil, []string{}, nil } -func (c *NullAuthHandler) InvalidateHandle(billy.Filesystem, []byte) error { +func (c *NullAuthHandler) InvalidateHandle(context.Context, billy.Filesystem, []byte) error { return nil } diff --git a/mount.go b/mount.go index e95d098..019c098 100644 --- a/mount.go +++ b/mount.go @@ -39,7 +39,7 @@ func onMount(ctx context.Context, w *response, userHandle Handler) error { return err } - rootHndl := userHandle.ToHandle(handle, []string{}) + rootHndl := userHandle.ToHandle(ctx, handle, []string{}) if status == MountStatusOk { _ = xdr.Write(writer, rootHndl) diff --git a/nfs_onaccess.go b/nfs_onaccess.go index b29d62e..9632de0 100644 --- a/nfs_onaccess.go +++ b/nfs_onaccess.go @@ -14,7 +14,7 @@ func onAccess(ctx context.Context, w *response, userHandle Handler) error { if err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(roothandle) + fs, path, err := userHandle.FromHandle(ctx, roothandle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_oncommit.go b/nfs_oncommit.go index 0d66837..e8b0a12 100644 --- a/nfs_oncommit.go +++ b/nfs_oncommit.go @@ -18,7 +18,7 @@ func onCommit(ctx context.Context, w *response, userHandle Handler) error { } // The conn will drain the unread offset and count arguments. - fs, path, err := userHandle.FromHandle(handle) + fs, path, err := userHandle.FromHandle(ctx, handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_oncreate.go b/nfs_oncreate.go index 63d7901..cba39ed 100644 --- a/nfs_oncreate.go +++ b/nfs_oncreate.go @@ -47,7 +47,7 @@ func onCreate(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusNotSupp, os.ErrInvalid} } - fs, path, err := userHandle.FromHandle(obj.Handle) + fs, path, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -86,8 +86,8 @@ func onCreate(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusAccess, err} } - fp := userHandle.ToHandle(fs, newFile) - changer := userHandle.Change(fs) + fp := userHandle.ToHandle(ctx, fs, newFile) + changer := userHandle.Change(ctx, fs) if err := attrs.Apply(changer, fs, newFilePath); err != nil { Log.Errorf("Error applying attributes: %v\n", err) return &NFSStatusError{NFSStatusIO, err} diff --git a/nfs_onfsinfo.go b/nfs_onfsinfo.go index d982114..55ec15d 100644 --- a/nfs_onfsinfo.go +++ b/nfs_onfsinfo.go @@ -24,7 +24,7 @@ func onFSInfo(ctx context.Context, w *response, userHandle Handler) error { if err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(roothandle) + fs, path, err := userHandle.FromHandle(ctx, roothandle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_onfsstat.go b/nfs_onfsstat.go index 9e85e40..ab2f111 100644 --- a/nfs_onfsstat.go +++ b/nfs_onfsstat.go @@ -13,7 +13,7 @@ func onFSStat(ctx context.Context, w *response, userHandle Handler) error { if err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(roothandle) + fs, path, err := userHandle.FromHandle(ctx, roothandle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_ongetattr.go b/nfs_ongetattr.go index 5ecc240..2160088 100644 --- a/nfs_ongetattr.go +++ b/nfs_ongetattr.go @@ -14,7 +14,7 @@ func onGetAttr(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(handle) + fs, path, err := userHandle.FromHandle(ctx, handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_onlink.go b/nfs_onlink.go index ed52df5..505f4fc 100644 --- a/nfs_onlink.go +++ b/nfs_onlink.go @@ -27,7 +27,7 @@ func onLink(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(obj.Handle) + fs, path, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -49,8 +49,8 @@ func onLink(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusNotDir, nil} } - fp := userHandle.ToHandle(fs, append(path, string(obj.Filename))) - changer := userHandle.Change(fs) + fp := userHandle.ToHandle(ctx, fs, append(path, string(obj.Filename))) + changer := userHandle.Change(ctx, fs) if changer == nil { return &NFSStatusError{NFSStatusAccess, err} } diff --git a/nfs_onlookup.go b/nfs_onlookup.go index 6654588..d0c5c83 100644 --- a/nfs_onlookup.go +++ b/nfs_onlookup.go @@ -34,7 +34,7 @@ func onLookup(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusInval, err} } - fs, p, err := userHandle.FromHandle(obj.Handle) + fs, p, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -59,7 +59,7 @@ func onLookup(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusAccess, os.ErrPermission} } pPath := p[0 : len(p)-1] - pHandle := userHandle.ToHandle(fs, pPath) + pHandle := userHandle.ToHandle(ctx, fs, pPath) resp, err := lookupSuccessResponse(pHandle, pPath, p, fs) if err != nil { return &NFSStatusError{NFSStatusServerFault, err} @@ -75,7 +75,7 @@ func onLookup(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusNoEnt, os.ErrNotExist} } - newHandle := userHandle.ToHandle(fs, reqPath) + newHandle := userHandle.ToHandle(ctx, fs, reqPath) resp, err := lookupSuccessResponse(newHandle, reqPath, p, fs) if err != nil { return &NFSStatusError{NFSStatusServerFault, err} diff --git a/nfs_onmkdir.go b/nfs_onmkdir.go index 35059b2..07a6d2a 100644 --- a/nfs_onmkdir.go +++ b/nfs_onmkdir.go @@ -26,7 +26,7 @@ func onMkdir(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(obj.Handle) + fs, path, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -59,8 +59,8 @@ func onMkdir(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusAccess, err} } - fp := userHandle.ToHandle(fs, newFolder) - changer := userHandle.Change(fs) + fp := userHandle.ToHandle(ctx, fs, newFolder) + changer := userHandle.Change(ctx, fs) if changer != nil { if err := attrs.Apply(changer, fs, newFolderPath); err != nil { return &NFSStatusError{NFSStatusIO, err} diff --git a/nfs_onmknod.go b/nfs_onmknod.go index b93855e..11721d9 100644 --- a/nfs_onmknod.go +++ b/nfs_onmknod.go @@ -37,14 +37,14 @@ func onMknod(ctx context.Context, w *response, userHandle Handler) error { } // see if the filesystem supports mknod - fs, path, err := userHandle.FromHandle(obj.Handle) + fs, path, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } if !billy.CapabilityCheck(fs, billy.WriteCapability) { return &NFSStatusError{NFSStatusROFS, os.ErrPermission} } - c := userHandle.Change(fs) + c := userHandle.Change(ctx, fs) if c == nil { return &NFSStatusError{NFSStatusAccess, os.ErrPermission} } @@ -67,7 +67,7 @@ func onMknod(ctx context.Context, w *response, userHandle Handler) error { } else if !parent.IsDir() { return &NFSStatusError{NFSStatusNotDir, nil} } - fp := userHandle.ToHandle(fs, append(path, string(obj.Filename))) + fp := userHandle.ToHandle(ctx, fs, append(path, string(obj.Filename))) switch nfs_ftype(ftype) { case FTYPE_NF3CHR: diff --git a/nfs_onpathconf.go b/nfs_onpathconf.go index 68973dd..b842436 100644 --- a/nfs_onpathconf.go +++ b/nfs_onpathconf.go @@ -15,7 +15,7 @@ func onPathConf(ctx context.Context, w *response, userHandle Handler) error { if err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(roothandle) + fs, path, err := userHandle.FromHandle(ctx, roothandle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_onread.go b/nfs_onread.go index b34949e..a0dcb9e 100644 --- a/nfs_onread.go +++ b/nfs_onread.go @@ -37,7 +37,7 @@ func onRead(ctx context.Context, w *response, userHandle Handler) error { if err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(obj.Handle) + fs, path, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_onreaddir.go b/nfs_onreaddir.go index b59239e..5992c34 100644 --- a/nfs_onreaddir.go +++ b/nfs_onreaddir.go @@ -40,12 +40,12 @@ func onReadDir(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusTooSmall, io.ErrShortBuffer} } - fs, p, err := userHandle.FromHandle(obj.Handle) + fs, p, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } - contents, verifier, err := getDirListingWithVerifier(userHandle, obj.Handle, obj.CookieVerif) + contents, verifier, err := getDirListingWithVerifier(ctx, userHandle, obj.Handle, obj.CookieVerif) if err != nil { return err } @@ -137,9 +137,9 @@ func onReadDir(ctx context.Context, w *response, userHandle Handler) error { return nil } -func getDirListingWithVerifier(userHandle Handler, fsHandle []byte, verifier uint64) ([]fs.FileInfo, uint64, error) { +func getDirListingWithVerifier(ctx context.Context, userHandle Handler, fsHandle []byte, verifier uint64) ([]fs.FileInfo, uint64, error) { // figure out what directory it is. - fs, p, err := userHandle.FromHandle(fsHandle) + fs, p, err := userHandle.FromHandle(ctx, fsHandle) if err != nil { return nil, 0, &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_onreaddirplus.go b/nfs_onreaddirplus.go index e6796de..de75a15 100644 --- a/nfs_onreaddirplus.go +++ b/nfs_onreaddirplus.go @@ -46,12 +46,12 @@ func onReadDirPlus(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusTooSmall, nil} } - fs, p, err := userHandle.FromHandle(obj.Handle) + fs, p, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } - contents, verifier, err := getDirListingWithVerifier(userHandle, obj.Handle, obj.CookieVerif) + contents, verifier, err := getDirListingWithVerifier(ctx, userHandle, obj.Handle, obj.CookieVerif) if err != nil { return err } @@ -102,7 +102,7 @@ func onReadDirPlus(ctx context.Context, w *response, userHandle Handler) error { } filePath := joinPath(p, c.Name()) - handle := userHandle.ToHandle(fs, filePath) + handle := userHandle.ToHandle(ctx, fs, filePath) attrs := ToFileAttribute(c, path.Join(filePath...)) entities = append(entities, readDirPlusEntity{ FileID: attrs.Fileid, diff --git a/nfs_onreadlink.go b/nfs_onreadlink.go index d1379a1..1b65fc2 100644 --- a/nfs_onreadlink.go +++ b/nfs_onreadlink.go @@ -14,7 +14,7 @@ func onReadLink(ctx context.Context, w *response, userHandle Handler) error { if err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(handle) + fs, path, err := userHandle.FromHandle(ctx, handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } diff --git a/nfs_onremove.go b/nfs_onremove.go index 53a7ca7..624eb23 100644 --- a/nfs_onremove.go +++ b/nfs_onremove.go @@ -15,7 +15,7 @@ func onRemove(ctx context.Context, w *response, userHandle Handler) error { if err := xdr.Read(w.req.Body, &obj); err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(obj.Handle) + fs, path, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -45,7 +45,7 @@ func onRemove(ctx context.Context, w *response, userHandle Handler) error { preCacheData := ToFileAttribute(dirInfo, fullPath).AsCache() toDelete := fs.Join(append(path, string(obj.Filename))...) - toDeleteHandle := userHandle.ToHandle(fs, append(path, string(obj.Filename))) + toDeleteHandle := userHandle.ToHandle(ctx, fs, append(path, string(obj.Filename))) err = fs.Remove(toDelete) if err != nil { @@ -58,7 +58,7 @@ func onRemove(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusIO, err} } - if err := userHandle.InvalidateHandle(fs, toDeleteHandle); err != nil { + if err := userHandle.InvalidateHandle(ctx, fs, toDeleteHandle); err != nil { return &NFSStatusError{NFSStatusServerFault, err} } diff --git a/nfs_onrename.go b/nfs_onrename.go index 2cba4fe..458c7db 100644 --- a/nfs_onrename.go +++ b/nfs_onrename.go @@ -19,7 +19,7 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error { if err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs, fromPath, err := userHandle.FromHandle(from.Handle) + fs, fromPath, err := userHandle.FromHandle(ctx, from.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -28,7 +28,7 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error { if err = xdr.Read(w.req.Body, &to); err != nil { return &NFSStatusError{NFSStatusInval, err} } - fs2, toPath, err := userHandle.FromHandle(to.Handle) + fs2, toPath, err := userHandle.FromHandle(ctx, to.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -71,7 +71,7 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error { } preDestData := ToFileAttribute(toDirInfo, toDirPath).AsCache() - oldHandle := userHandle.ToHandle(fs, append(fromPath, string(from.Filename))) + oldHandle := userHandle.ToHandle(ctx, fs, append(fromPath, string(from.Filename))) fromLoc := fs.Join(append(fromPath, string(from.Filename))...) toLoc := fs.Join(append(toPath, string(to.Filename))...) @@ -87,7 +87,7 @@ func onRename(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusIO, err} } - if err := userHandle.InvalidateHandle(fs, oldHandle); err != nil { + if err := userHandle.InvalidateHandle(ctx, fs, oldHandle); err != nil { return &NFSStatusError{NFSStatusServerFault, err} } diff --git a/nfs_onsetattr.go b/nfs_onsetattr.go index 910ba3a..30256f3 100644 --- a/nfs_onsetattr.go +++ b/nfs_onsetattr.go @@ -16,7 +16,7 @@ func onSetAttr(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(handle) + fs, path, err := userHandle.FromHandle(ctx, handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -53,7 +53,7 @@ func onSetAttr(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusROFS, os.ErrPermission} } - changer := userHandle.Change(fs) + changer := userHandle.Change(ctx, fs) if err := attrs.Apply(changer, fs, fs.Join(path...)); err != nil { // Already an nfsstatuserror return err diff --git a/nfs_onsymlink.go b/nfs_onsymlink.go index a58c9bc..5c6881e 100644 --- a/nfs_onsymlink.go +++ b/nfs_onsymlink.go @@ -26,7 +26,7 @@ func onSymlink(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(obj.Handle) + fs, path, err := userHandle.FromHandle(ctx, obj.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } @@ -53,8 +53,8 @@ func onSymlink(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusAccess, err} } - fp := userHandle.ToHandle(fs, append(path, string(obj.Filename))) - changer := userHandle.Change(fs) + fp := userHandle.ToHandle(ctx, fs, append(path, string(obj.Filename))) + changer := userHandle.Change(ctx, fs) if changer != nil { if err := attrs.Apply(changer, fs, newFilePath); err != nil { return &NFSStatusError{NFSStatusIO, err} diff --git a/nfs_onwrite.go b/nfs_onwrite.go index 80d4187..861ef16 100644 --- a/nfs_onwrite.go +++ b/nfs_onwrite.go @@ -35,7 +35,7 @@ func onWrite(ctx context.Context, w *response, userHandle Handler) error { return &NFSStatusError{NFSStatusInval, err} } - fs, path, err := userHandle.FromHandle(req.Handle) + fs, path, err := userHandle.FromHandle(ctx, req.Handle) if err != nil { return &NFSStatusError{NFSStatusStale, err} } From f59b77ca8555c7875418dbd2c2687bf99c977f83 Mon Sep 17 00:00:00 2001 From: Joe Lombrozo Date: Wed, 18 Mar 2026 15:44:20 -0700 Subject: [PATCH 7/7] also return the conn so we can wrap that --- conn.go | 2 +- server.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/conn.go b/conn.go index 7a0eb52..2d6310f 100644 --- a/conn.go +++ b/conn.go @@ -47,7 +47,7 @@ func (c *conn) serve(ctx context.Context) { defer cancel() if hook := c.OnConnect; hook != nil { - connCtx = hook(connCtx, c.Conn) + connCtx, c.Conn = hook(connCtx, c.Conn) } defer func() { diff --git a/server.go b/server.go index ece7ecd..3c0267e 100644 --- a/server.go +++ b/server.go @@ -15,7 +15,7 @@ type Server struct { ID [8]byte context.Context - OnConnect func(ctx context.Context, conn net.Conn) context.Context + OnConnect func(ctx context.Context, conn net.Conn) (context.Context, net.Conn) OnDisconnect func(ctx context.Context, conn net.Conn) }