From cde05ddf9fe653e802f6eaaaa15cf1991b72c2c2 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Thu, 22 Sep 2022 07:18:37 +0200 Subject: [PATCH] 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 --- client_handler.go | 17 +++++++++++ driver.go | 6 ++++ handle_dirs.go | 72 +++++++++++++++++++++++++++++++++++---------- handle_dirs_test.go | 44 ++++++++++++++++++++++++--- 4 files changed, 120 insertions(+), 19 deletions(-) diff --git a/client_handler.go b/client_handler.go index d616b175..34a98d8a 100644 --- a/client_handler.go +++ b/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() diff --git a/driver.go b/driver.go index a35c14f2..866f4b93 100644 --- a/driver.go +++ b/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) diff --git a/handle_dirs.go b/handle_dirs.go index e2c9cbb0..361e0ef0 100644 --- a/handle_dirs.go +++ b/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,7 +223,7 @@ 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("")) @@ -199,7 +231,10 @@ func (c *clientHandler) dirTransferNLST(w io.Writer, files []os.FileInfo) error } 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) { diff --git a/handle_dirs_test.go b/handle_dirs_test.go index e8ffae51..525f44a0 100644 --- a/handle_dirs_test.go +++ b/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)) }