From 9183e7fd791a85e4d7dfe5f40d2ff289f3a6b70a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Georg=20M=C3=BCller?= <georgmueller@gmx.net>
Date: Fri, 14 Oct 2022 17:12:56 +0200
Subject: [PATCH] request-server: Introduce ReadlinkFileLister

ReadlinkFileLister with its Readlink method allows returning paths without
misusing the os.FileInfo interface, whose Name() method should only return
the base name of a file.

By implementing ReadlinkFileLister, it is possible to easily return
symlinks of any kind (absolute, relative, multiple directory levels)
---
 request-example.go     | 17 +----------------
 request-interfaces.go  | 15 ++++++++++++++-
 request-server_test.go |  4 ++++
 request.go             | 24 +++++++++++++++++++++++-
 4 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/request-example.go b/request-example.go
index ba230ac..519b3b7 100644
--- a/request-example.go
+++ b/request-example.go
@@ -391,21 +391,6 @@ func (fs *root) Filelist(r *Request) (ListerAt, error) {
 			return nil, err
 		}
 		return listerat{file}, nil
-
-	case "Readlink":
-		symlink, err := fs.readlink(r.Filepath)
-		if err != nil {
-			return nil, err
-		}
-
-		// SFTP-v2: The server will respond with a SSH_FXP_NAME packet containing only
-		// one name and a dummy attributes value.
-		return listerat{
-			&memFile{
-				name: symlink,
-				err:  os.ErrNotExist, // prevent accidental use as a reader/writer.
-			},
-		}, nil
 	}
 
 	return nil, errors.New("unsupported")
@@ -434,7 +419,7 @@ func (fs *root) readdir(pathname string) ([]os.FileInfo, error) {
 	return files, nil
 }
 
-func (fs *root) readlink(pathname string) (string, error) {
+func (fs *root) Readlink(pathname string) (string, error) {
 	file, err := fs.lfetch(pathname)
 	if err != nil {
 		return "", err
diff --git a/request-interfaces.go b/request-interfaces.go
index 2e5ee6b..75c6c39 100644
--- a/request-interfaces.go
+++ b/request-interfaces.go
@@ -74,6 +74,11 @@ type StatVFSFileCmder interface {
 // FileLister should return an object that fulfils the ListerAt interface
 // Note in cases of an error, the error text will be sent to the client.
 // Called for Methods: List, Stat, Readlink
+//
+// Since Filelist returns an os.FileInfo, this can make it non-ideal for impelmenting Readlink.
+// This is because the Name receiver method defined by that interface defines that it should only return the base name.
+// However, Readlink is required to be capable of returning essentially any arbitrary valid path relative or absolute.
+// In order to implement this more expressive requirement, implement [ReadlinkFileLister] which will then be used instead.
 type FileLister interface {
 	Filelist(*Request) (ListerAt, error)
 }
@@ -94,7 +99,7 @@ type LstatFileLister interface {
 //
 // Up to v1.13.5 the signature for the RealPath method was:
 //
-// RealPath(string) string
+// # RealPath(string) string
 //
 // we have added a legacyRealPathFileLister that implements the old method
 // to ensure that your code does not break.
@@ -104,6 +109,14 @@ type RealPathFileLister interface {
 	RealPath(string) (string, error)
 }
 
+// ReadlinkFileLister is a FileLister that implements the Readlink method.
+// By implementing the Readlink method, it is possible to return any arbitrary valid path relative or absolute.
+// This allows giving a better response than via the default FileLister (which is limited to os.FileInfo, whose Name method should only return the base name of a file)
+type ReadlinkFileLister interface {
+	FileLister
+	Readlink(string) (string, error)
+}
+
 // This interface is here for backward compatibility only
 type legacyRealPathFileLister interface {
 	FileLister
diff --git a/request-server_test.go b/request-server_test.go
index f2d26f6..ab74de6 100644
--- a/request-server_test.go
+++ b/request-server_test.go
@@ -607,6 +607,10 @@ func TestRequestSymlink(t *testing.T) {
 	for _, s := range symlinks {
 		err := p.cli.Symlink(s.target, s.name)
 		require.NoError(t, err, "Creating symlink %q with target %q failed", s.name, s.target)
+
+		rl, err := p.cli.ReadLink(s.name)
+		require.NoError(t, err, "ReadLink(%q) failed", s.name)
+		require.Equal(t, s.target, rl, "Unexpected result when reading symlink %q", s.name)
 	}
 
 	// test fetching via symlink
diff --git a/request.go b/request.go
index 6a7e6cf..57d788d 100644
--- a/request.go
+++ b/request.go
@@ -295,7 +295,12 @@ func (r *Request) call(handlers Handlers, pkt requestPacket, alloc *allocator, o
 		return filecmd(handlers.FileCmd, r, pkt)
 	case "List":
 		return filelist(handlers.FileList, r, pkt)
-	case "Stat", "Lstat", "Readlink":
+	case "Stat", "Lstat":
+		return filestat(handlers.FileList, r, pkt)
+	case "Readlink":
+		if readlinkFileLister, ok := handlers.FileList.(ReadlinkFileLister); ok {
+			return readlink(readlinkFileLister, r, pkt)
+		}
 		return filestat(handlers.FileList, r, pkt)
 	default:
 		return statusFromError(pkt.id(), fmt.Errorf("unexpected method: %s", r.Method))
@@ -599,6 +604,23 @@ func filestat(h FileLister, r *Request, pkt requestPacket) responsePacket {
 	}
 }
 
+func readlink(readlinkFileLister ReadlinkFileLister, r *Request, pkt requestPacket) responsePacket {
+	resolved, err := readlinkFileLister.Readlink(r.Filepath)
+	if err != nil {
+		return statusFromError(pkt.id(), err)
+	}
+	return &sshFxpNamePacket{
+		ID: pkt.id(),
+		NameAttrs: []*sshFxpNameAttr{
+			{
+				Name:     resolved,
+				LongName: resolved,
+				Attrs:    emptyFileStat,
+			},
+		},
+	}
+}
+
 // init attributes of request object from packet data
 func requestMethod(p requestPacket) (method string) {
 	switch p.(type) {