NLST: return paths relative to the working directory (#368)

NLST: return paths relative to the working directory

Based on RFC 959 NLST is intended to return information that can be used
by a program to further process the files automatically.

Also use path.IsAbs/path.Join to build absolute paths

Author: drakkan

client_handler.go

@@ -87,6 +87,7 @@ type clientHandler struct {
 	reader              *bufio.Reader   // Reader on the TCP connection
 	user                string          // Authenticated user
 	path                string          // Current path
+	listPath            string          // Path for NLST/LIST requests
 	clnt                string          // Identified client
 	command             string          // Command received on the connection
 	connectedAt         time.Time       // Date of connection
@@ -151,6 +152,22 @@ func (c *clientHandler) SetPath(value string) {
 	c.path = value
 }
 
+// getListPath returns the path for the last LIST/NLST request
+func (c *clientHandler) getListPath() string {
+	c.paramsMutex.RLock()
+	defer c.paramsMutex.RUnlock()
+
+	return c.listPath
+}
+
+// SetListPath changes the path for the last LIST/NLST request
+func (c *clientHandler) SetListPath(value string) {
+	c.paramsMutex.Lock()
+	defer c.paramsMutex.Unlock()
+
+	c.listPath = value
+}
+
 // Debug defines if we will list all interaction
 func (c *clientHandler) Debug() bool {
 	c.paramsMutex.RLock()

driver.go

@@ -141,6 +141,12 @@ type ClientContext interface {
 	// Calling this method after the authentication step could lead to undefined behavior
 	SetPath(value string)
 
+	// SetListPath allows to change the path for the last LIST/NLST request.
+	// This method is useful if the driver expands wildcards and so the returned results
+	// refer to a path different from the requested one.
+	// The value must be cleaned using path.Clean
+	SetListPath(value string)
+
 	// SetDebug activates the debugging of this connection commands
 	SetDebug(debug bool)
 

handle_dirs.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path"
 	"strings"
@@ -19,11 +20,42 @@ var errFileList = errors.New("listing a file isn't allowed")
 var supportedlistArgs = []string{"-al", "-la", "-a", "-l"}
 
 func (c *clientHandler) absPath(p string) string {
-	if strings.HasPrefix(p, "/") {
+	if path.IsAbs(p) {
 		return path.Clean(p)
 	}
 
-	return path.Clean(c.Path() + "/" + p)
+	return path.Join(c.Path(), p)
+}
+
+// getRelativePath returns the specified path as relative to the
+// current working directory. The specified path must be cleaned
+func (c *clientHandler) getRelativePath(p string) string {
+	var sb strings.Builder
+	base := c.Path()
+
+	for {
+		if base == p {
+			return sb.String()
+		}
+
+		if !strings.HasSuffix(base, "/") {
+			base += "/"
+		}
+
+		if strings.HasPrefix(p, base) {
+			sb.WriteString(strings.TrimPrefix(p, base))
+
+			return sb.String()
+		}
+
+		if base == "/" || base == "./" {
+			return p
+		}
+
+		sb.WriteString("../")
+
+		base = path.Dir(path.Clean(base))
+	}
 }
 
 func (c *clientHandler) handleCWD(param string) error {
@@ -156,7 +188,7 @@ func (c *clientHandler) checkLISTArgs(args string) string {
 func (c *clientHandler) handleLIST(param string) error {
 	info := fmt.Sprintf("LIST %v", param)
 
-	if files, err := c.getFileList(param, true); err == nil || err == io.EOF {
+	if files, _, err := c.getFileList(param, true); err == nil || err == io.EOF {
 		if tr, errTr := c.TransferOpen(info); errTr == nil {
 			err = c.dirTransferLIST(tr, files)
 			c.TransferClose(err)
@@ -175,9 +207,9 @@ func (c *clientHandler) handleLIST(param string) error {
 func (c *clientHandler) handleNLST(param string) error {
 	info := fmt.Sprintf("NLST %v", param)
 
-	if files, err := c.getFileList(param, true); err == nil || err == io.EOF {
+	if files, parentDir, err := c.getFileList(param, true); err == nil || err == io.EOF {
 		if tr, errTrOpen := c.TransferOpen(info); errTrOpen == nil {
-			err = c.dirTransferNLST(tr, files)
+			err = c.dirTransferNLST(tr, files, parentDir)
 			c.TransferClose(err)
 
 			return nil
@@ -191,15 +223,18 @@ func (c *clientHandler) handleNLST(param string) error {
 	return nil
 }
 
-func (c *clientHandler) dirTransferNLST(w io.Writer, files []os.FileInfo) error {
+func (c *clientHandler) dirTransferNLST(w io.Writer, files []os.FileInfo, parentDir string) error {
 	if len(files) == 0 {
 		_, err := w.Write([]byte(""))
 
 		return err
 	}
 
 	for _, file := range files {
-		if _, err := fmt.Fprintf(w, "%s\r\n", file.Name()); err != nil {
+		// Based on RFC 959 NLST is intended to return information that can be used
+		// by a program to further process the files automatically.
+		// So we return paths relative to the current working directory
+		if _, err := fmt.Fprintf(w, "%s\r\n", path.Join(c.getRelativePath(parentDir), file.Name())); err != nil {
 			return err
 		}
 	}
@@ -216,7 +251,7 @@ func (c *clientHandler) handleMLSD(param string) error {
 
 	info := fmt.Sprintf("MLSD %v", param)
 
-	if files, err := c.getFileList(param, false); err == nil || err == io.EOF {
+	if files, _, err := c.getFileList(param, false); err == nil || err == io.EOF {
 		if tr, errTr := c.TransferOpen(info); errTr == nil {
 			err = c.dirTransferMLSD(tr, files)
 			c.TransferClose(err)
@@ -312,39 +347,46 @@ func (c *clientHandler) writeMLSxEntry(w io.Writer, file os.FileInfo) error {
 	return err
 }
 
-func (c *clientHandler) getFileList(param string, filePathAllowed bool) ([]os.FileInfo, error) {
+func (c *clientHandler) getFileList(param string, filePathAllowed bool) ([]os.FileInfo, string, error) {
 	if !c.server.settings.DisableLISTArgs {
 		param = c.checkLISTArgs(param)
 	}
 	// directory or filePath
 	listPath := c.absPath(param)
+	c.SetListPath(listPath)
 
 	// return list of single file if directoryPath points to file and filePathAllowed
 	info, err := c.driver.Stat(listPath)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
 	if !info.IsDir() {
 		if filePathAllowed {
-			return []os.FileInfo{info}, nil
+			return []os.FileInfo{info}, path.Dir(c.getListPath()), nil
 		}
 
-		return nil, errFileList
+		return nil, "", errFileList
 	}
 
+	var files []fs.FileInfo
+
 	if fileList, ok := c.driver.(ClientDriverExtensionFileList); ok {
-		return fileList.ReadDir(listPath)
+		files, err = fileList.ReadDir(listPath)
+
+		return files, c.getListPath(), err
 	}
 
 	directory, errOpenFile := c.driver.Open(listPath)
 	if errOpenFile != nil {
-		return nil, errOpenFile
+		return nil, "", errOpenFile
 	}
 
 	defer c.closeDirectory(listPath, directory)
 
-	return directory.Readdir(-1)
+	files, err = directory.Readdir(-1)
+
+	return files, c.getListPath(), err
 }
 
 func (c *clientHandler) closeDirectory(directoryPath string, directory afero.File) {

handle_dirs_test.go

@@ -3,6 +3,7 @@ package ftpserver
 import (
 	"crypto/tls"
 	"fmt"
+	"io"
 	"net"
 	"path"
 	"testing"
@@ -14,6 +15,29 @@ import (
 
 const DirKnown = "known"
 
+func TestGetRelativePaths(t *testing.T) {
+	type relativePathTest struct {
+		workingDir, path, result string
+	}
+	var tests = []relativePathTest{
+		{"/", "/p", "p"},
+		{"/", "/", ""},
+		{"/p", "/p", ""},
+		{"/p", "/p1", "../p1"},
+		{"/p", "/p/p1", "p1"},
+		{"/p/p1", "/p/p2/p3", "../p2/p3"},
+		{"/", "p", "p"},
+	}
+
+	handler := clientHandler{}
+
+	for _, test := range tests {
+		handler.SetPath(test.workingDir)
+		result := handler.getRelativePath(test.path)
+		require.Equal(t, test.result, result)
+	}
+}
+
 func TestDirListing(t *testing.T) {
 	// MLSD is disabled we relies on LIST of files listing
 	s := NewTestServerWithDriver(t, &TestServerDriver{Debug: false, Settings: &Settings{DisableMLSD: true}})
@@ -278,13 +302,19 @@ func TestDirListingWithSpace(t *testing.T) {
 	require.Equal(t, StatusFileOK, rc)
 	require.Equal(t, fmt.Sprintf("CD worked on /%s", dirName), response)
 
-	_, err = raw.PrepareDataConn()
+	dcGetter, err := raw.PrepareDataConn()
 	require.NoError(t, err)
 
 	rc, response, err = raw.SendCommand("NLST /")
 	require.NoError(t, err)
 	require.Equal(t, StatusFileStatusOK, rc, response)
 
+	dc, err := dcGetter()
+	require.NoError(t, err)
+	resp, err := io.ReadAll(dc)
+	require.NoError(t, err)
+	require.Equal(t, "../"+dirName+"\r\n", string(resp))
+
 	rc, _, err = raw.ReadResponse()
 	require.NoError(t, err)
 	require.Equal(t, StatusClosingDataConn, rc)
@@ -561,16 +591,22 @@ func TestMLSDAndNLSTFilePathError(t *testing.T) {
 	_, err = c.ReadDir(fileName)
 	require.Error(t, err, "MLSD is enabled, MLSD for filePath must fail")
 
-	// NLST shouldn't work for filePath
+	// NLST should work for filePath
 	raw, err := c.OpenRawConn()
 	require.NoError(t, err, "Couldn't open raw connection")
 
 	defer func() { require.NoError(t, raw.Close()) }()
 
-	_, err = raw.PrepareDataConn()
+	dcGetter, err := raw.PrepareDataConn()
 	require.NoError(t, err)
 
-	rc, response, err := raw.SendCommand("NLST /" + fileName)
+	rc, response, err := raw.SendCommand("NLST /../" + fileName)
 	require.NoError(t, err)
 	require.Equal(t, StatusFileStatusOK, rc, response)
+
+	dc, err := dcGetter()
+	require.NoError(t, err)
+	resp, err := io.ReadAll(dc)
+	require.NoError(t, err)
+	require.Equal(t, fileName+"\r\n", string(resp))
 }