Repository: pkg/sftp
Pull request #577 (2024-02-09 09:35:41)

add CloserListerAt
The `ListerAt` is stored in the `Request` state and reused across requests. Some implementations don't store the entire `[]os.FileInfo` buffer in the `ListerAt` implementation but instead return an open file and get/return `[]os.FileInfo` on request. For these implementation a `Close` method is required.

The use case is if you want to list a large directory on memory constrained systems.

drakkan: weird, `TestRequestStatVFS` seems to consistently fail in 32-bit tests

Patch:
diff --git a/request-interfaces.go b/request-interfaces.go
index da56ed4..13e7577 100644
--- a/request-interfaces.go
+++ b/request-interfaces.go
@@ -144,6 +144,8 @@ type NameLookupFileLister interface {
 //
 // If a populated entry implements [FileInfoExtendedData], extended attributes will also be returned to the client.
 //
+// The request server code will call Close() on ListerAt if an io.Closer type assertion succeeds.
+//
 // Note in cases of an error, the error text will be sent to the client.
 type ListerAt interface {
 	ListAt([]os.FileInfo, int64) (int, error)
diff --git a/request.go b/request.go
index 266abc0..cd652cf 100644
--- a/request.go
+++ b/request.go
@@ -121,6 +121,22 @@ func (s *state) getListerAt() ListerAt {
 	return s.listerAt
 }
 
+func (s *state) closeListerAt() error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	var err error
+
+	if s.listerAt != nil {
+		if c, ok := s.listerAt.(io.Closer); ok {
+			err = c.Close()
+		}
+		s.listerAt = nil
+	}
+
+	return err
+}
+
 // Request contains the data and state for the incoming service request.
 type Request struct {
 	// Get, Put, Setstat, Stat, Rename, Remove
@@ -230,9 +246,9 @@ func (r *Request) close() error {
 		}
 	}()
 
-	rd, wr, rw := r.getAllReaderWriters()
+	err := r.state.closeListerAt()
 
-	var err error
+	rd, wr, rw := r.getAllReaderWriters()
 
 	// Close errors on a Writer are far more likely to be the important one.
 	// As they can be information that there was a loss of data.


Test patch:
diff --git a/request-server_test.go b/request-server_test.go
index 6825bf4..64f380c 100644
--- a/request-server_test.go
+++ b/request-server_test.go
@@ -793,6 +793,37 @@ func TestRequestReaddir(t *testing.T) {
 	checkRequestServerAllocator(t, p)
 }
 
+type testListerAtCloser struct {
+	isClosed bool
+}
+
+func (l *testListerAtCloser) ListAt([]os.FileInfo, int64) (int, error) {
+	return 0, io.EOF
+}
+
+func (l *testListerAtCloser) Close() error {
+	l.isClosed = true
+	return nil
+}
+
+func TestRequestServerListerAtCloser(t *testing.T) {
+	p := clientRequestServerPair(t)
+	defer p.Close()
+
+	handle, err := p.cli.opendir(context.Background(), "/")
+	require.NoError(t, err)
+	require.Len(t, p.svr.openRequests, 1)
+	req, ok := p.svr.getRequest(handle)
+	require.True(t, ok)
+	listerAt := &testListerAtCloser{}
+	req.setListerAt(listerAt)
+	assert.NotNil(t, req.state.getListerAt())
+	err = p.cli.close(handle)
+	assert.NoError(t, err)
+	require.Len(t, p.svr.openRequests, 0)
+	assert.True(t, listerAt.isClosed)
+}
+
 func TestRequestStatVFS(t *testing.T) {
 	if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
 		t.Skip("StatVFS is implemented on linux and darwin")