From b65fc0bdc2431e64c3203637bf7389e1295ccb7a Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 17 Sep 2022 16:47:29 +0200 Subject: [PATCH] ftpd: return relative paths for NLST reponses Fixes #993 Signed-off-by: Nicola Murino --- ftpd/ftpd_test.go | 137 +++++++++++++++++++++++++++--------------- ftpd/handler.go | 9 ++- ftpd/internal_test.go | 2 + go.mod | 3 +- go.sum | 8 +-- 5 files changed, 105 insertions(+), 54 deletions(-) diff --git a/ftpd/ftpd_test.go b/ftpd/ftpd_test.go index 5585293e..bd004101 100644 --- a/ftpd/ftpd_test.go +++ b/ftpd/ftpd_test.go @@ -614,8 +614,18 @@ func TestListDirWithWildcards(t *testing.T) { assert.NoError(t, err) sftpUser, _, err := httpdtest.AddUser(getTestSFTPUser(), http.StatusCreated) assert.NoError(t, err) + + defer func() { + _, err = httpdtest.RemoveUser(sftpUser, http.StatusOK) + assert.NoError(t, err) + _, err = httpdtest.RemoveUser(localUser, http.StatusOK) + assert.NoError(t, err) + err = os.RemoveAll(localUser.GetHomeDir()) + assert.NoError(t, err) + }() + for _, user := range []dataprovider.User{localUser, sftpUser} { - client, err := getFTPClient(user, true, nil) + client, err := getFTPClient(user, true, nil, ftp.DialWithDisabledMLSD(true)) if assert.NoError(t, err) { dir1 := "test.dir" dir2 := "test.dir1" @@ -633,58 +643,96 @@ func TestListDirWithWildcards(t *testing.T) { localDownloadPath := filepath.Join(homeBasePath, testDLFileName) err = ftpDownloadFile(fileName, localDownloadPath, testFileSize, client, 0) assert.NoError(t, err) - entries, err := client.NameList(fileName) - if assert.NoError(t, err) { - assert.Len(t, entries, 1) - assert.Contains(t, entries, fileName) - } - entries, err = client.NameList(".") - assert.NoError(t, err) - assert.Len(t, entries, 3) - entries, err = client.NameList("/test.*") - if assert.NoError(t, err) { - assert.Len(t, entries, 2) - assert.Contains(t, entries, dir1) - assert.Contains(t, entries, dir2) - } - entries, err = client.NameList("/*.dir?") - if assert.NoError(t, err) { - assert.Len(t, entries, 1) - assert.Contains(t, entries, dir2) - } - entries, err = client.NameList("/test.???") - if assert.NoError(t, err) { - assert.Len(t, entries, 1) - assert.Contains(t, entries, dir1) + entries, err := client.List(fileName) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, fileName, entries[0].Name) + nListEntries, err := client.NameList(fileName) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, nListEntries, fileName) + entries, err = client.List(".") + require.NoError(t, err) + require.Len(t, entries, 3) + nListEntries, err = client.NameList(".") + require.NoError(t, err) + require.Len(t, nListEntries, 3) + entries, err = client.List("/test.*") + require.NoError(t, err) + require.Len(t, entries, 2) + found := 0 + for _, e := range entries { + switch e.Name { + case dir1, dir2: + found++ + } } + assert.Equal(t, 2, found) + nListEntries, err = client.NameList("/test.*") + require.NoError(t, err) + require.Len(t, entries, 2) + assert.Contains(t, nListEntries, dir1) + assert.Contains(t, nListEntries, dir2) + entries, err = client.List("/*.dir?") + require.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, dir2, entries[0].Name) + nListEntries, err = client.NameList("/*.dir?") + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, nListEntries, dir2) + entries, err = client.List("/test.???") + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, dir1, entries[0].Name) + nListEntries, err = client.NameList("/test.???") + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, nListEntries, dir1) _, err = client.NameList("/missingdir/test.*") assert.Error(t, err) + _, err = client.List("/missingdir/test.*") + assert.Error(t, err) _, err = client.NameList("test[-]") if assert.Error(t, err) { assert.Contains(t, err.Error(), path.ErrBadPattern.Error()) } + _, err = client.List("test[-]") + if assert.Error(t, err) { + assert.Contains(t, err.Error(), path.ErrBadPattern.Error()) + } subDir := path.Join(dir1, "sub.d") err = client.MakeDir(subDir) assert.NoError(t, err) err = client.ChangeDir(path.Dir(subDir)) assert.NoError(t, err) - entries, err = client.NameList("sub.?") - if assert.NoError(t, err) { - assert.Len(t, entries, 1) - assert.Contains(t, entries, path.Base(subDir)) - } - entries, err = client.NameList("../*.dir?") - if assert.NoError(t, err) { - assert.Len(t, entries, 1) - assert.Contains(t, entries, path.Join("../", dir2)) - } + entries, err = client.List("sub.?") + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, path.Base(subDir), entries[0].Name) + nListEntries, err = client.NameList("sub.?") + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, nListEntries, path.Base(subDir)) + entries, err = client.List("../*.dir?") + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, path.Join("../", dir2), entries[0].Name) + nListEntries, err = client.NameList("../*.dir?") + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, nListEntries, path.Join("../", dir2)) + err = client.ChangeDir("/") assert.NoError(t, err) - entries, err = client.NameList(path.Join(dir1, "sub.*")) - if assert.NoError(t, err) { - assert.Len(t, entries, 1) - assert.Contains(t, entries, path.Join(dir1, "sub.d")) - } + entries, err = client.List(path.Join(dir1, "sub.*")) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, path.Join(dir1, "sub.d"), entries[0].Name) + nListEntries, err = client.NameList(path.Join(dir1, "sub.*")) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Contains(t, nListEntries, path.Join(dir1, "sub.d")) err = client.RemoveDir(subDir) assert.NoError(t, err) err = client.RemoveDir(dir1) @@ -699,13 +747,6 @@ func TestListDirWithWildcards(t *testing.T) { assert.NoError(t, err) } } - - _, err = httpdtest.RemoveUser(sftpUser, http.StatusOK) - assert.NoError(t, err) - _, err = httpdtest.RemoveUser(localUser, http.StatusOK) - assert.NoError(t, err) - err = os.RemoveAll(localUser.GetHomeDir()) - assert.NoError(t, err) } func TestStartDirectory(t *testing.T) { @@ -3354,8 +3395,10 @@ func getFTPClientImplicitTLS(user dataprovider.User) (*ftp.ServerConn, error) { return client, err } -func getFTPClient(user dataprovider.User, useTLS bool, tlsConfig *tls.Config) (*ftp.ServerConn, error) { +func getFTPClient(user dataprovider.User, useTLS bool, tlsConfig *tls.Config, dialOptions ...ftp.DialOption, +) (*ftp.ServerConn, error) { ftpOptions := []ftp.DialOption{ftp.DialWithTimeout(5 * time.Second)} + ftpOptions = append(ftpOptions, dialOptions...) if useTLS { if tlsConfig == nil { tlsConfig = &tls.Config{ diff --git a/ftpd/handler.go b/ftpd/handler.go index 46402f8b..6fe64909 100644 --- a/ftpd/handler.go +++ b/ftpd/handler.go @@ -300,7 +300,9 @@ func (c *Connection) ReadDir(name string) ([]os.FileInfo, error) { // we only support wildcards for the last path level, for example: // - *.xml is supported // - dir*/*.xml is not supported - return c.getListDirWithWildcards(path.Dir(name), baseName) + name = path.Dir(name) + c.clientContext.SetListPath(name) + return c.getListDirWithWildcards(name, baseName) } return c.ListDir(name) @@ -510,7 +512,10 @@ func (c *Connection) getListDirWithWildcards(dirName, pattern string) ([]os.File return files, err } validIdx := 0 - relativeBase := getPathRelativeTo(c.clientContext.Path(), dirName) + var relativeBase string + if c.clientContext.GetLastCommand() != "NLST" { + relativeBase = getPathRelativeTo(c.clientContext.Path(), dirName) + } for _, fi := range files { match, err := path.Match(pattern, fi.Name()) if err != nil { diff --git a/ftpd/internal_test.go b/ftpd/internal_test.go index 8772f350..59699498 100644 --- a/ftpd/internal_test.go +++ b/ftpd/internal_test.go @@ -280,6 +280,8 @@ func (cc mockFTPClientContext) Path() string { func (cc mockFTPClientContext) SetPath(name string) {} +func (cc mockFTPClientContext) SetListPath(name string) {} + func (cc mockFTPClientContext) SetDebug(debug bool) {} func (cc mockFTPClientContext) Debug() bool { diff --git a/go.mod b/go.mod index 2dc292a4..946138b2 100644 --- a/go.mod +++ b/go.mod @@ -156,7 +156,7 @@ require ( golang.org/x/tools v0.1.12 // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect google.golang.org/appengine v1.6.7 // indirect - google.golang.org/genproto v0.0.0-20220916134934-764224ccc2d1 // indirect + google.golang.org/genproto v0.0.0-20220916172020-2692e8806bfa // indirect google.golang.org/grpc v1.49.0 // indirect google.golang.org/protobuf v1.28.1 // indirect gopkg.in/ini.v1 v1.67.0 // indirect @@ -166,6 +166,7 @@ require ( ) replace ( + github.com/fclairamb/ftpserverlib => github.com/drakkan/ftpserverlib v0.0.0-20220917142547-394d5e183aeb github.com/jlaffaye/ftp => github.com/drakkan/ftp v0.0.0-20201114075148-9b9adce499a9 golang.org/x/crypto => github.com/drakkan/crypto v0.0.0-20220831070616-b69bc2ec2993 golang.org/x/net => github.com/drakkan/net v0.0.0-20220916171046-9879e3f5b22a diff --git a/go.sum b/go.sum index 52f7dc34..85533d24 100644 --- a/go.sum +++ b/go.sum @@ -268,6 +268,8 @@ github.com/drakkan/crypto v0.0.0-20220831070616-b69bc2ec2993 h1:P9lP7U92yt5GETGd github.com/drakkan/crypto v0.0.0-20220831070616-b69bc2ec2993/go.mod h1:SiM6ypd8Xu1xldObYtbDztuUU7xUzMnUULfphXFZmro= github.com/drakkan/ftp v0.0.0-20201114075148-9b9adce499a9 h1:LPH1dEblAOO/LoG7yHPMtBLXhQmjaga91/DDjWk9jWA= github.com/drakkan/ftp v0.0.0-20201114075148-9b9adce499a9/go.mod h1:2lmrmq866uF2tnje75wQHzmPXhmSWUt7Gyx2vgK1RCU= +github.com/drakkan/ftpserverlib v0.0.0-20220917142547-394d5e183aeb h1:sd63fxu7eKejDU0fBiGvVejeEEXNcGv6SVmXrFsDaKM= +github.com/drakkan/ftpserverlib v0.0.0-20220917142547-394d5e183aeb/go.mod h1:Nwsxl2ZzyPiSCgB1rZGnEscTenwkxPhCn1D+Hm/k9JA= github.com/drakkan/net v0.0.0-20220916171046-9879e3f5b22a h1:DXelKiOuwaiblhsGccshNDZLf5etRwx3IWInVf2f3Z8= github.com/drakkan/net v0.0.0-20220916171046-9879e3f5b22a/go.mod h1:YDH+HFinaLZZlnHAfSS6ZXJJ9M9t4Dl22yv3iI2vPwk= github.com/eikenb/pipeat v0.0.0-20210730190139-06b3e6902001 h1:/ZshrfQzayqRSBDodmp3rhNCHJCff+utvgBuWRbiqu4= @@ -284,8 +286,6 @@ github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go. github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= -github.com/fclairamb/ftpserverlib v0.19.1 h1:OIqW+AdcsUEq4apudrluDD1c4iCRidLAoQzJRBUJnbg= -github.com/fclairamb/ftpserverlib v0.19.1/go.mod h1:cVeFR3wvEjgtK99686UXJaTvqZk8jbjHFnhaC23LGpc= github.com/fclairamb/go-log v0.4.1 h1:rLtdSG9x2pK41AIAnE8WYpl05xBJfw1ZyYxZaXFcBsM= github.com/fclairamb/go-log v0.4.1/go.mod h1:sw1KvnkZ4wKCYkvy4SL3qVZcJSWFP8Ure4pM3z+KNn4= github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= @@ -1229,8 +1229,8 @@ google.golang.org/genproto v0.0.0-20220523171625-347a074981d8/go.mod h1:RAyBrSAP google.golang.org/genproto v0.0.0-20220608133413-ed9918b62aac/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= google.golang.org/genproto v0.0.0-20220616135557-88e70c0c3a90/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= google.golang.org/genproto v0.0.0-20220624142145-8cd45d7dbd1f/go.mod h1:KEWEmljWE5zPzLBa/oHl6DaEt9LmfH6WtH1OHIvleBA= -google.golang.org/genproto v0.0.0-20220916134934-764224ccc2d1 h1:f+XAjNNl0e5qs8BbB5iQMTYGQjpDbsG4nyAyAuKg3M4= -google.golang.org/genproto v0.0.0-20220916134934-764224ccc2d1/go.mod h1:0Nb8Qy+Sk5eDzHnzlStwW3itdNaWoZA5XeSG+R3JHSo= +google.golang.org/genproto v0.0.0-20220916172020-2692e8806bfa h1:VWkrxnAx2C2hirAP+W5ADU7e/+93Yhk//ioKd2XFyDI= +google.golang.org/genproto v0.0.0-20220916172020-2692e8806bfa/go.mod h1:0Nb8Qy+Sk5eDzHnzlStwW3itdNaWoZA5XeSG+R3JHSo= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM=