diff --git a/client_handler_test.go b/client_handler_test.go index 74f60408..1e52e28f 100644 --- a/client_handler_test.go +++ b/client_handler_test.go @@ -133,3 +133,24 @@ func isStringInSlice(s string, list []string) bool { return false } + +func TestUnkowCommand(t *testing.T) { + s := NewTestServer(t, true) + conf := goftp.Config{ + User: authUser, + Password: authPass, + } + + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") + + defer func() { panicOnError(c.Close()) }() + + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") + + rc, response, err := raw.SendCommand("UNSUPPORTED") + require.NoError(t, err) + require.Equal(t, StatusSyntaxErrorNotRecognised, rc) + require.Equal(t, "Unknown command", response) +} diff --git a/go.mod b/go.mod index 3e76ee7d..3c9836e6 100644 --- a/go.mod +++ b/go.mod @@ -7,8 +7,7 @@ require ( github.com/secsy/goftp v0.0.0-20200609142545-aa2de14babf4 github.com/spf13/afero v1.5.1 github.com/stretchr/testify v1.6.1 - golang.org/x/text v0.3.4 // indirect - gopkg.in/dutchcoders/goftp.v1 v1.0.0-20170301105846-ed59a591ce14 + golang.org/x/text v0.3.4 // indirect ) replace github.com/secsy/goftp => github.com/drakkan/goftp v0.0.0-20200916091733-843d4cca4bb2 diff --git a/go.sum b/go.sum index 59423315..c75db567 100644 --- a/go.sum +++ b/go.sum @@ -304,7 +304,6 @@ golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20191220142924-d4481acd189f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= -golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4 h1:0YWbFKbhXG/wIiuHDSKpS0Iy7FSA+u45VtBMfQcFTTc= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -347,8 +346,6 @@ gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLks gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/cheggaaa/pb.v1 v1.0.25/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw= -gopkg.in/dutchcoders/goftp.v1 v1.0.0-20170301105846-ed59a591ce14 h1:tHqNpm9sPaE6BSuMLXBzgTwukQLdBEt4OYU2coQjEQQ= -gopkg.in/dutchcoders/goftp.v1 v1.0.0-20170301105846-ed59a591ce14/go.mod h1:nzmlZQ+UqB5+55CRTV/dOaiK8OrPl6Co96Ob8lH4Wxw= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys= gopkg.in/gcfg.v1 v1.2.3/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o= diff --git a/handle_auth_test.go b/handle_auth_test.go index 9e519ffa..c39db549 100644 --- a/handle_auth_test.go +++ b/handle_auth_test.go @@ -2,7 +2,6 @@ package ftpserver import ( "crypto/tls" - "fmt" "net" "testing" "time" @@ -17,12 +16,6 @@ func panicOnError(err error) { } } -func reportError(err error) { - if err != nil { - fmt.Println("Error reporting:", err) - } -} - func TestLoginSuccess(t *testing.T) { s := NewTestServer(t, true) // send a NOOP before the login, this doesn't seems possible using secsy/goftp so use the old way ... diff --git a/handle_dirs.go b/handle_dirs.go index 57c89413..119587fa 100644 --- a/handle_dirs.go +++ b/handle_dirs.go @@ -111,10 +111,6 @@ func (c *clientHandler) checkLISTArgs() { } func (c *clientHandler) handleLIST() error { - if !c.server.settings.DisableLISTArgs { - c.checkLISTArgs() - } - if files, err := c.getFileList(); err == nil || err == io.EOF { if tr, errTr := c.TransferOpen(); errTr == nil { err = c.dirTransferLIST(tr, files) @@ -258,6 +254,10 @@ func (c *clientHandler) writeMLSxOutput(w io.Writer, file os.FileInfo) error { } func (c *clientHandler) getFileList() ([]os.FileInfo, error) { + if !c.server.settings.DisableLISTArgs { + c.checkLISTArgs() + } + directoryPath := c.absPath(c.param) if fileList, ok := c.driver.(ClientDriverExtensionFileList); ok { diff --git a/handle_dirs_test.go b/handle_dirs_test.go index 53aeeaec..6ae18e37 100644 --- a/handle_dirs_test.go +++ b/handle_dirs_test.go @@ -3,252 +3,175 @@ package ftpserver import ( "crypto/tls" "fmt" - "runtime" - "strings" + "net" + "path" "testing" + "time" - "gopkg.in/dutchcoders/goftp.v1" + "github.com/secsy/goftp" + "github.com/stretchr/testify/require" ) const DirKnown = "known" -// TestDirAccess relies on LIST of files listing func TestDirListing(t *testing.T) { + // MLSD is disabled we relies on LIST of files listing s := NewTestServerWithDriver(t, &TestServerDriver{Debug: true, Settings: &Settings{DisableMLSD: true}}) - - var connErr error - - var ftp *goftp.FTP - - if ftp, connErr = goftp.Connect(s.Addr()); connErr != nil { - t.Fatal("Couldn't connect", connErr) + conf := goftp.Config{ + User: authUser, + Password: authPass, } - defer func() { panicOnError(ftp.Quit()) }() - - if _, err := ftp.List("/"); err == nil { - t.Fatal("We could list files before login") - } + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") - if err := ftp.Login("test", "test"); err != nil { - t.Fatal("Failed to login:", err) - } + defer func() { panicOnError(c.Close()) }() - if err := ftp.Mkd("/" + DirKnown); err != nil { - t.Fatal("Couldn't create dir:", err) - } + dirName, err := c.Mkdir(DirKnown) + require.NoError(t, err, "Couldn't create dir") + require.Equal(t, path.Join("/", DirKnown), dirName) - if lines, err := ftp.List("/"); err != nil { - t.Fatal("Couldn't list files:", err) - } else { - found := false - for _, line := range lines { - line = line[0 : len(line)-2] - if len(line) < 47 { - break - } - fileName := line[47:] - if fileName == DirKnown { - found = true - } - } - if !found { - t.Fatal("Couldn't find the dir") - } - } + contents, err := c.ReadDir("/") + require.NoError(t, err) + require.Len(t, contents, 1) + require.Equal(t, DirKnown, contents[0].Name()) } func TestDirListingPathArg(t *testing.T) { + // MLSD is disabled we relies on LIST of files listing s := NewTestServerWithDriver(t, &TestServerDriver{Debug: true, Settings: &Settings{DisableMLSD: true}}) - - var connErr error - - var ftp *goftp.FTP - - if ftp, connErr = goftp.Connect(s.Addr()); connErr != nil { - t.Fatal("Couldn't connect", connErr) + conf := goftp.Config{ + User: authUser, + Password: authPass, } - defer func() { panicOnError(ftp.Quit()) }() + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") - if err := ftp.Login("test", "test"); err != nil { - t.Fatal("Failed to login:", err) - } + defer func() { panicOnError(c.Close()) }() for _, dir := range []string{"/" + DirKnown, "/" + DirKnown + "/1"} { - if err := ftp.Mkd(dir); err != nil { - t.Fatal("Couldn't create dir:", err) - } + _, err = c.Mkdir(dir) + require.NoError(t, err, "Couldn't create dir") } - if lines, err := ftp.List(DirKnown); err != nil { - t.Fatal("Couldn't list files:", err) - } else { - found := false - for _, line := range lines { - line = line[0 : len(line)-2] - if len(line) < 47 { - break - } - fileName := line[47:] - if fileName == "1" { - found = true - } - } - if !found { - t.Fatal("Couldn't find the dir") - } - } + contents, err := c.ReadDir(DirKnown) + require.NoError(t, err) + require.Len(t, contents, 1) + require.Equal(t, "1", contents[0].Name()) - if lines, err := ftp.List(""); err != nil { - t.Fatal("Couldn't list files:", err) - } else { - found := false - for _, line := range lines { - line = line[0 : len(line)-2] - if len(line) < 47 { - break - } - fileName := line[47:] - if fileName == DirKnown { - found = true - } - } - if !found { - t.Fatal("Couldn't find the dir") - } - } + contents, err = c.ReadDir("") + require.NoError(t, err) + require.Len(t, contents, 1) + require.Equal(t, DirKnown, contents[0].Name()) } -// TestDirAccess relies on LIST of files listing func TestDirHandling(t *testing.T) { s := NewTestServer(t, true) + conf := goftp.Config{ + User: authUser, + Password: authPass, + } - var connErr error + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") - var ftp *goftp.FTP + defer func() { panicOnError(c.Close()) }() - if ftp, connErr = goftp.Connect(s.Addr()); connErr != nil { - t.Fatal("Couldn't connect", connErr) - } + // Getwd will send a PWD command + p, err := c.Getwd() + require.NoError(t, err) + require.Equal(t, "/", p, "Bad path") - defer func() { panicOnError(ftp.Quit()) }() + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") - if err := ftp.Login("test", "test"); err != nil { - t.Fatal("Failed to login:", err) - } + rc, _, err := raw.SendCommand("CWD /unknown") + require.NoError(t, err) + require.Equal(t, StatusActionNotTaken, rc) - if path, err := ftp.Pwd(); err != nil { - t.Fatal("Couldn't test PWD", err) - } else if path != "/" { - t.Fatal("Bad path:", path) - } + _, err = c.Mkdir("/" + DirKnown) + require.NoError(t, err) - if err := ftp.Cwd("/unknown"); err == nil { - t.Fatal("We should have had an error") - } + contents, err := c.ReadDir("/") + require.NoError(t, err) + require.Len(t, contents, 1) + require.Equal(t, DirKnown, contents[0].Name()) - if err := ftp.Mkd("/" + DirKnown); err != nil { - t.Fatal("Couldn't create dir:", err) - } + rc, _, err = raw.SendCommand("CWD /" + DirKnown) + require.NoError(t, err) + require.Equal(t, StatusFileOK, rc) - if entry, err := ftp.List("/"); err != nil { - t.Fatal("Couldn't list files") - } else { - found := false - for _, entry := range entry { - pathentry := validMLSxEntryPattern.FindStringSubmatch(entry) - if len(pathentry) != 2 { - t.Errorf("MLSx file listing contains invalid entry: \"%s\"", entry) - } else if pathentry[1] == DirKnown { - found = true - } - } - if !found { - t.Error("Newly created dir was not found during listing of files") - } - } + testSubdir := " strange\\ sub dìr" + rc, _, err = raw.SendCommand(fmt.Sprintf("MKD %v", testSubdir)) + require.NoError(t, err) + require.Equal(t, StatusPathCreated, rc) - if err := ftp.Cwd("/" + DirKnown); err != nil { - t.Fatal("Couldn't access the known dir:", err) - } + rc, response, err := raw.SendCommand(fmt.Sprintf("CWD %v", testSubdir)) + require.NoError(t, err) + require.Equal(t, StatusFileOK, rc, response) - if err := ftp.Rmd("/" + DirKnown); err != nil { - t.Fatal("Couldn't ftpDelete the known dir:", err) - } + rc, response, err = raw.SendCommand("CDUP") + require.NoError(t, err) + require.Equal(t, StatusFileOK, rc) + require.Equal(t, "CDUP worked on /"+DirKnown, response) - if err := ftp.Rmd("/" + DirKnown); err == nil { - t.Fatal("We shouldn't have been able to ftpDelete known again") - } + err = c.Rmdir(path.Join("/", DirKnown, testSubdir)) + require.NoError(t, err) + + err = c.Rmdir(path.Join("/", DirKnown)) + require.NoError(t, err) + + err = c.Rmdir("/" + DirKnown) + require.Error(t, err, "We shouldn't have been able to ftpDelete known again") } // TestDirListingWithSpace uses the MLSD for files listing func TestDirListingWithSpace(t *testing.T) { s := NewTestServer(t, true) + conf := goftp.Config{ + User: authUser, + Password: authPass, + } - var connErr error + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") - var ftp *goftp.FTP + defer func() { panicOnError(c.Close()) }() - if ftp, connErr = goftp.Connect(s.Addr()); connErr != nil { - t.Fatal("Couldn't connect", connErr) - } + dirName := " with spaces " - defer func() { panicOnError(ftp.Quit()) }() + _, err = c.Mkdir(dirName) + require.NoError(t, err, "Couldn't create dir") - if err := ftp.Login("test", "test"); err != nil { - t.Fatal("Failed to login:", err) - } - - if err := ftp.Mkd("/ with spaces "); err != nil { - t.Fatal("Couldn't create dir:", err) - } + contents, err := c.ReadDir("/") + require.NoError(t, err) + require.Len(t, contents, 1) + require.Equal(t, dirName, contents[0].Name()) - if lines, err := ftp.List("/"); err != nil { - t.Fatal("Couldn't list files:", err) - } else { - found := false - for _, line := range lines { - line = line[0 : len(line)-2] - if len(line) < 47 { - break - } - spl := strings.SplitN(line, "; ", 2) - fileName := spl[1] - expectedfileName := " with spaces " - if runtime.GOOS == "windows" { - expectedfileName = " with spaces" - } - if fileName == expectedfileName { - found = true - } - } - if !found { - t.Fatal("Couldn't find the dir") - } - } + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") - if err := ftp.Cwd("/ with spaces "); err != nil { - t.Fatal("Couldn't access the known dir:", err) - } + rc, response, err := raw.SendCommand(fmt.Sprintf("CWD /%s", dirName)) + require.NoError(t, err) + require.Equal(t, StatusFileOK, rc) + require.Equal(t, fmt.Sprintf("CD worked on /%s", dirName), response) } func TestCleanPath(t *testing.T) { s := NewTestServer(t, true) - - var connErr error - - var ftp *goftp.FTP - - if ftp, connErr = goftp.Connect(s.Addr()); connErr != nil { - t.Fatal("Couldn't connect", connErr) + conf := goftp.Config{ + User: authUser, + Password: authPass, } - defer func() { panicOnError(ftp.Quit()) }() + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") - if err := ftp.Login("test", "test"); err != nil { - t.Fatal("Failed to login:", err) - } + defer func() { panicOnError(c.Close()) }() + + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") // various path purity tests @@ -260,15 +183,14 @@ func TestCleanPath(t *testing.T) { "/./", "/././.", } { - if err := ftp.Cwd(dir); err != nil { - t.Fatal("Couldn't Cwd to a valid path:", err) - } - - if path, err := ftp.Pwd(); err != nil { - t.Fatal("PWD failed:", err) - } else if path != "/" { - t.Fatal("Faulty path:", path) - } + rc, response, err := raw.SendCommand(fmt.Sprintf("CWD %s", dir)) + require.NoError(t, err) + require.Equal(t, StatusFileOK, rc) + require.Equal(t, "CD worked on /", response) + + p, err := c.Getwd() + require.NoError(t, err) + require.Equal(t, "/", p) } } @@ -279,102 +201,125 @@ func TestTLSTransfer(t *testing.T) { }) s.settings.TLSRequired = MandatoryEncryption - ftp, err := goftp.Connect(s.Addr()) - if err != nil { - t.Fatal("Couldn't connect:", err) + conf := goftp.Config{ + User: authUser, + Password: authPass, + TLSConfig: &tls.Config{ + // nolint:gosec + InsecureSkipVerify: true, + }, + TLSMode: goftp.TLSExplicit, } - defer func() { reportError(ftp.Quit()) }() + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") - config := &tls.Config{ - // nolint:gosec - InsecureSkipVerify: true, - } - if err = ftp.AuthTLS(config); err != nil { - t.Fatal("Couldn't upgrade connection to TLS:", err) - } + defer func() { panicOnError(c.Close()) }() - if err = ftp.Login("test", "test"); err != nil { - t.Fatal("Failed to login:", err) - } + contents, err := c.ReadDir("/") + require.NoError(t, err) + require.Len(t, contents, 0) - if _, err := ftp.List("/"); err != nil { - t.Fatal("Couldn't list files") - } + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") - code, _ := ftp.RawCmd("PROT C") - if code != StatusOK { - t.Fatal("unable to send PROT C") - } + rc, response, err := raw.SendCommand("PROT C") + require.NoError(t, err) + require.Equal(t, StatusOK, rc) + require.Equal(t, "OK", response) - if _, err := ftp.List("/"); err == nil { - t.Fatal("List files should fail, TLS is required") - } else if !strings.Contains(err.Error(), "TLS is required") { - t.Fatal("unexpected error:", err) - } + rc, _, err = raw.SendCommand("PASV") + require.NoError(t, err) + require.Equal(t, StatusEnteringPASV, rc) + + rc, response, err = raw.SendCommand("MLSD /") + require.NoError(t, err) + require.Equal(t, StatusServiceNotAvailable, rc) + require.Equal(t, "Unable to open transfer: TLS is required", response) } -func TestListArgs(t *testing.T) { +func TestDirListingBeforeLogin(t *testing.T) { s := NewTestServer(t, true) + conn, err := net.DialTimeout("tcp", s.Addr(), 5*time.Second) + require.NoError(t, err) - var connErr error - var ftp *goftp.FTP + defer func() { + err = conn.Close() + require.NoError(t, err) + }() - if ftp, connErr = goftp.Connect(s.Addr()); connErr != nil { - t.Fatal("Couldn't connect", connErr) - } + buf := make([]byte, 1024) + n, err := conn.Read(buf) + require.NoError(t, err) + + response := string(buf[:n]) + require.Equal(t, "220 TEST Server\r\n", response) + + _, err = conn.Write([]byte("LIST\r\n")) + require.NoError(t, err) + + n, err = conn.Read(buf) + require.NoError(t, err) - defer func() { panicOnError(ftp.Quit()) }() + response = string(buf[:n]) + require.Equal(t, "530 Please login with USER and PASS\r\n", response) +} + +func TestListArgs(t *testing.T) { + t.Run("with-mlsd", func(t *testing.T) { + testListDirArgs(t, NewTestServer(t, true)) + }) - if err := ftp.Login("test", "test"); err != nil { - t.Fatal("Failed to login:", err) + t.Run("without-mlsd", func(t *testing.T) { + testListDirArgs(t, NewTestServerWithDriver(t, &TestServerDriver{Debug: true, Settings: &Settings{DisableMLSD: true}})) + }) +} + +func testListDirArgs(t *testing.T, s *FtpServer) { + conf := goftp.Config{ + User: authUser, + Password: authPass, } + testDir := "testdir" + + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") + + defer func() { panicOnError(c.Close()) }() for _, arg := range supportedlistArgs { s.settings.DisableLISTArgs = true - if _, err := ftp.List(arg); err == nil { - t.Fatalf("list args are disabled \"list %v\" must fail", arg) - } + _, err = c.ReadDir(arg) + require.Error(t, err, fmt.Sprintf("list args are disabled \"list %v\" must fail", arg)) s.settings.DisableLISTArgs = false - if _, err := ftp.List(arg); err != nil { - t.Fatal("unexpected error", err) - } - - if err := ftp.Mkd(arg); err != nil { - t.Fatal("unexpected error", err) - } - - if err := ftp.Mkd(fmt.Sprintf("%v/testdir", arg)); err != nil { - t.Fatal("unexpected error", err) - } + contents, err := c.ReadDir(arg) + require.NoError(t, err) + require.Len(t, contents, 0) - contents, err := ftp.List(arg) - if err != nil { - t.Fatal("unexpected error", err) - } + _, err = c.Mkdir(arg) + require.NoError(t, err) - if len(contents) != 1 { - t.Fatal("unexpected dir contents", contents) - } + _, err = c.Mkdir(fmt.Sprintf("%v/%v", arg, testDir)) + require.NoError(t, err) - if !strings.Contains(contents[0], "testdir") { - t.Fatal("unexpected dir contents", contents) - } + contents, err = c.ReadDir(arg) + require.NoError(t, err) + require.Len(t, contents, 1) + require.Equal(t, contents[0].Name(), testDir) - contents, err = ftp.List(fmt.Sprintf("%v %v", arg, arg)) - if err != nil { - t.Fatal("unexpected error", err) - } + contents, err = c.ReadDir(fmt.Sprintf("%v %v", arg, arg)) + require.NoError(t, err) + require.Len(t, contents, 1) + require.Equal(t, contents[0].Name(), testDir) - if len(contents) != 1 { - t.Fatal("unexpected dir contents", contents) - } + // cleanup + err = c.Rmdir(fmt.Sprintf("%v/%v", arg, testDir)) + require.NoError(t, err) - if !strings.Contains(contents[0], "testdir") { - t.Fatal("unexpected dir contents", contents) - } + err = c.Rmdir(arg) + require.NoError(t, err) } } diff --git a/handle_files_test.go b/handle_files_test.go index 4d2d0eba..a1392cd3 100644 --- a/handle_files_test.go +++ b/handle_files_test.go @@ -1,6 +1,7 @@ package ftpserver import ( + "crypto/tls" "fmt" "io/ioutil" "os" @@ -85,7 +86,6 @@ func TestALLO(t *testing.T) { User: authUser, Password: authPass, } - c, err := goftp.DialConfig(conf, s.Addr()) require.NoError(t, err, "Couldn't connect") @@ -105,13 +105,44 @@ func TestALLO(t *testing.T) { require.Equal(t, StatusOK, rc, "Should have been accepted") } +func TestCHMOD(t *testing.T) { + s := NewTestServerWithDriver(t, &TestServerDriver{ + Debug: true, + TLS: true, + }) + conf := goftp.Config{ + User: authUser, + Password: authPass, + TLSConfig: &tls.Config{ + // nolint:gosec + InsecureSkipVerify: true, + }, + TLSMode: goftp.TLSExplicit, + } + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") + + // Creating a tiny file + ftpUpload(t, c, createTemporaryFile(t, 10), "file") + + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") + + rc, _, err := raw.SendCommand("SITE CHMOD a file") + require.NoError(t, err) + require.Equal(t, StatusActionNotTaken, rc, "Should have been refused") + + rc, _, err = raw.SendCommand("SITE CHMOD 600 file") + require.NoError(t, err) + require.Equal(t, StatusOK, rc, "Should have been accepted") +} + func TestCHOWN(t *testing.T) { s := NewTestServer(t, true) conf := goftp.Config{ User: authUser, Password: authPass, } - c, err := goftp.DialConfig(conf, s.Addr()) require.NoError(t, err, "Couldn't connect") @@ -150,12 +181,19 @@ func TestCHOWN(t *testing.T) { } func TestMFMT(t *testing.T) { - s := NewTestServer(t, true) + s := NewTestServerWithDriver(t, &TestServerDriver{ + Debug: true, + TLS: true, + }) conf := goftp.Config{ User: authUser, Password: authPass, + TLSConfig: &tls.Config{ + // nolint:gosec + InsecureSkipVerify: true, + }, + TLSMode: goftp.TLSExplicit, } - c, err := goftp.DialConfig(conf, s.Addr()) require.NoError(t, err, "Couldn't connect") @@ -164,9 +202,7 @@ func TestMFMT(t *testing.T) { // Creating a tiny file ftpUpload(t, c, createTemporaryFile(t, 10), "file") - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") // Good @@ -201,7 +237,6 @@ func TestSYMLINK(t *testing.T) { User: authUser, Password: authPass, } - c, err := goftp.DialConfig(conf, s.Addr()) require.NoError(t, err, "Couldn't connect") @@ -210,9 +245,7 @@ func TestSYMLINK(t *testing.T) { // Creating a tiny file ftpUpload(t, c, createTemporaryFile(t, 10), "file") - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") // Creating a bad clunky is authorized @@ -246,7 +279,6 @@ func TestSTATFile(t *testing.T) { User: authUser, Password: authPass, } - c, err := goftp.DialConfig(conf, s.Addr()) require.NoError(t, err, "Couldn't connect") @@ -262,9 +294,7 @@ func TestSTATFile(t *testing.T) { _, err = c.Mkdir("/dir/sub") require.NoError(t, err) - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") rc, _, err := raw.SendCommand("STAT file") @@ -281,6 +311,32 @@ func TestSTATFile(t *testing.T) { require.Equal(t, StatusFileActionNotTaken, rc) } +func TestMDTM(t *testing.T) { + s := NewTestServer(t, true) + conf := goftp.Config{ + User: authUser, + Password: authPass, + } + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") + + defer func() { panicOnError(c.Close()) }() + + // Creating a tiny file + ftpUpload(t, c, createTemporaryFile(t, 10), "file") + + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") + + rc, _, err := raw.SendCommand("MDTM file") + require.NoError(t, err) + require.Equal(t, StatusFileStatus, rc) + + rc, _, err = raw.SendCommand("MDTM missing") + require.NoError(t, err) + require.Equal(t, StatusActionNotTaken, rc) +} + func TestHASHCommand(t *testing.T) { s := NewTestServerWithDriver( t, @@ -314,9 +370,7 @@ func TestHASHCommand(t *testing.T) { ftpUpload(t, c, tempFile, "file.txt") - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") // ask hash for a directory @@ -369,9 +423,7 @@ func TestCustomHASHCommands(t *testing.T) { ftpUpload(t, c, tempFile, "file.txt") - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") customCommands := make(map[string]string) diff --git a/handle_misc_test.go b/handle_misc_test.go index 64310895..98fc1039 100644 --- a/handle_misc_test.go +++ b/handle_misc_test.go @@ -1,6 +1,7 @@ package ftpserver import ( + "crypto/tls" "fmt" "strings" "testing" @@ -23,9 +24,7 @@ func TestSiteCommand(t *testing.T) { defer func() { panicOnError(c.Close()) }() - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") rc, response, err := raw.SendCommand("SITE HELP") @@ -103,9 +102,7 @@ func TestCLNT(t *testing.T) { defer func() { panicOnError(c.Close()) }() - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") rc, _, err := raw.SendCommand("CLNT NcFTP 3.2.6 macosx10.15") @@ -125,9 +122,7 @@ func TestOPTSUTF8(t *testing.T) { defer func() { panicOnError(c.Close()) }() - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") for _, cmd := range []string{"OPTS UTF8", "OPTS UTF8 ON"} { @@ -158,9 +153,7 @@ func TestOPTSHASH(t *testing.T) { defer func() { panicOnError(c.Close()) }() - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") rc, message, err := raw.SendCommand("OPTS HASH") @@ -193,23 +186,16 @@ func TestOPTSHASH(t *testing.T) { func TestAVBL(t *testing.T) { s := NewTestServer(t, true) - conf := goftp.Config{ User: authUser, Password: authPass, } - - var err error - var c *goftp.Client - - c, err = goftp.DialConfig(conf, s.Addr()) + c, err := goftp.DialConfig(conf, s.Addr()) require.NoError(t, err, "Couldn't connect") defer func() { panicOnError(c.Close()) }() - var raw goftp.RawConn - - raw, err = c.OpenRawConn() + raw, err := c.OpenRawConn() require.NoError(t, err, "Couldn't open raw connection") rc, response, err := raw.SendCommand("AVBL") @@ -238,3 +224,58 @@ func TestAVBL(t *testing.T) { require.Equal(t, StatusActionNotTaken, rc) require.Equal(t, fmt.Sprintf("Couldn't get space for path %v: %v", noavblDir, errAvblNotPermitted.Error()), response) } + +func TestQuit(t *testing.T) { + s := NewTestServerWithDriver(t, &TestServerDriver{ + Debug: true, + TLS: true, + }) + conf := goftp.Config{ + User: authUser, + Password: authPass, + TLSConfig: &tls.Config{ + // nolint:gosec + InsecureSkipVerify: true, + }, + TLSMode: goftp.TLSExplicit, + } + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") + + defer func() { panicOnError(c.Close()) }() + + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") + + rc, _, err := raw.SendCommand("QUIT") + require.NoError(t, err) + require.Equal(t, StatusClosingControlConn, rc) +} + +func TestTYPE(t *testing.T) { + s := NewTestServer(t, true) + conf := goftp.Config{ + User: authUser, + Password: authPass, + } + + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") + + defer func() { panicOnError(c.Close()) }() + + raw, err := c.OpenRawConn() + require.NoError(t, err, "Couldn't open raw connection") + + rc, _, err := raw.SendCommand("TYPE I") + require.NoError(t, err) + require.Equal(t, StatusOK, rc) + + rc, _, err = raw.SendCommand("TYPE A") + require.NoError(t, err) + require.Equal(t, StatusOK, rc) + + rc, _, err = raw.SendCommand("TYPE wrong") + require.NoError(t, err) + require.Equal(t, StatusSyntaxErrorNotRecognised, rc) +} diff --git a/server_test.go b/server_test.go index 44c56766..a1eccd9e 100644 --- a/server_test.go +++ b/server_test.go @@ -1,20 +1,16 @@ package ftpserver -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/require" +) func TestPortCommandFormatOK(t *testing.T) { net, err := parsePORTAddr("127,0,0,1,239,163") - if err != nil { - t.Fatal("Problem parsing", err) - } - - if net.IP.String() != "127.0.0.1" { - t.Fatal("Problem parsing IP", net.IP.String()) - } - - if net.Port != 239<<8+163 { - t.Fatal("Problem parsing port", net.Port) - } + require.NoError(t, err, "Problem parsing") + require.Equal(t, "127.0.0.1", net.IP.String(), "Problem parsing IP") + require.Equal(t, 239<<8+163, net.Port, "Problem parsing port") } func TestPortCommandFormatInvalid(t *testing.T) { @@ -24,9 +20,7 @@ func TestPortCommandFormatInvalid(t *testing.T) { } for _, f := range badFormats { _, err := parsePORTAddr(f) - if err == nil { - t.Fatal("This should have failed", f) - } + require.Error(t, err, "This should have failed") } } @@ -49,9 +43,7 @@ func TestQuoteDoubling(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - if got := quoteDoubling(tt.args.s); got != tt.want { - t.Errorf("quoteDoubling() = %v, want %v", got, tt.want) - } + require.Equal(t, tt.want, quoteDoubling(tt.args.s)) }) } } diff --git a/transfer_test.go b/transfer_test.go index 2e015d4f..896bd75c 100644 --- a/transfer_test.go +++ b/transfer_test.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "encoding/hex" "errors" + "fmt" "io" "io/ioutil" "math/rand" @@ -21,43 +22,34 @@ func createTemporaryFile(t *testing.T, targetSize int) *os.File { var fileErr error - if file, fileErr = ioutil.TempFile("", "ftpserver"); fileErr != nil { - t.Fatal("Temporary creation error:", fileErr) - return nil - } + file, fileErr = ioutil.TempFile("", "ftpserver") + require.NoError(t, fileErr, "Temporary file creation error") // nolint: gosec src := rand.New(rand.NewSource(0)) - if _, err := io.CopyN(file, src, int64(targetSize)); err != nil { - t.Fatal("Couldn't copy:", err) - return nil - } + _, err := io.CopyN(file, src, int64(targetSize)) + require.NoError(t, err, "Couldn't copy") t.Cleanup(func() { - if err := os.Remove(file.Name()); err != nil { - t.Fatalf("Problem deleting file \"%s\": %v", file.Name(), err) - } + err := os.Remove(file.Name()) + require.NoError(t, err, fmt.Sprintf("Problem deleting file %#v", file.Name())) }) return file } func hashFile(t *testing.T, file *os.File) string { - if _, err := file.Seek(0, 0); err != nil { - t.Fatal("Couldn't seek:", err) - } + _, err := file.Seek(0, 0) + require.NoError(t, err, "Couldn't seek") hashser := sha256.New() - - if _, err := io.Copy(hashser, file); err != nil { - t.Fatal("Couldn't hashUpload:", err) - } + _, err = io.Copy(hashser, file) + require.NoError(t, err, "Couldn't hashUpload") hash := hex.EncodeToString(hashser.Sum(nil)) - if _, err := file.Seek(0, 0); err != nil { - t.Fatal("Couldn't seek:", err) - } + _, err = file.Seek(0, 0) + require.NoError(t, err, "Couldn't seek") return hash } @@ -91,21 +83,18 @@ func ftpUpload(t *testing.T, ftp *goftp.Client, file io.ReadSeeker, filename str func ftpDownloadAndHash(t *testing.T, ftp *goftp.Client, filename string) string { hasher := sha256.New() - if err := ftp.Retrieve(filename, hasher); err != nil { - t.Fatal("Couldn't fetch file:", err) - } + err := ftp.Retrieve(filename, hasher) + require.NoError(t, err, "Couldn't fetch file") return hex.EncodeToString(hasher.Sum(nil)) } func ftpDelete(t *testing.T, ftp *goftp.Client, filename string) { - if err := ftp.Delete(filename); err != nil { - t.Fatal("Couldn't delete file "+filename+":", err) - } + err := ftp.Delete(filename) + require.NoError(t, err, "Couldn't delete file "+filename) - if err := ftp.Delete(filename); err == nil { - t.Fatal("Should have had a problem deleting " + filename) - } + err = ftp.Delete(filename) + require.Error(t, err, "Should have had a problem deleting "+filename) } func TestTransferIPv6(t *testing.T) { @@ -188,12 +177,8 @@ func testTransferOnConnection(t *testing.T, server *FtpServer, active, enableTLS } } - var err error - var c *goftp.Client - - if c, err = goftp.DialConfig(conf, server.Addr()); err != nil { - t.Fatal("Couldn't connect", err) - } + c, err := goftp.DialConfig(conf, server.Addr()) + require.NoError(t, err, "Couldn't connect") defer func() { panicOnError(c.Close()) }() @@ -210,9 +195,7 @@ func testTransferOnConnection(t *testing.T, server *FtpServer, active, enableTLS } // We make sure the hashes of the two files match - if hashUpload != hashDownload { - t.Fatal("The two files don't have the same hash:", hashUpload, "!=", hashDownload) - } + require.Equal(t, hashUpload, hashDownload, "The two files don't have the same hash") } func TestActiveModeDisabled(t *testing.T) { @@ -229,135 +212,84 @@ func TestActiveModeDisabled(t *testing.T) { Password: authPass, ActiveTransfers: true, } - - var err error - var c *goftp.Client - - if c, err = goftp.DialConfig(conf, server.Addr()); err != nil { - t.Fatal("Couldn't connect", err) - } + c, err := goftp.DialConfig(conf, server.Addr()) + require.NoError(t, err, "Couldn't connect") defer func() { panicOnError(c.Close()) }() file := createTemporaryFile(t, 10*1024) err = c.Store("file.bin", file) - - if err == nil { - t.Fatal("active mode is disabled, upload must fail") - } - - if !strings.Contains(err.Error(), "421-PORT command is disabled") { - t.Fatal("unexpected error", err) - } + require.Error(t, err, "active mode is disabled, upload must fail") + require.True(t, strings.Contains(err.Error(), "421-PORT command is disabled")) } // TestFailedTransfer validates the handling of failed transfer caused by file access issues func TestFailedTransfer(t *testing.T) { s := NewTestServer(t, true) - conf := goftp.Config{ User: authUser, Password: authPass, } - - var err error - - var c *goftp.Client - - if c, err = goftp.DialConfig(conf, s.Addr()); err != nil { - t.Fatal("Couldn't connect", err) - } + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") defer func() { panicOnError(c.Close()) }() // We create a 1KB file and upload it file := createTemporaryFile(t, 1*1024) - if err = c.Store("/non/existing/path/file.bin", file); err == nil { - t.Fatal("This upload should have failed") - } + err = c.Store("/non/existing/path/file.bin", file) + require.Error(t, err, "This upload should have failed") - if err = c.Store("file.bin", file); err != nil { - t.Fatal("This upload should have succeeded", err) - } + err = c.Store("file.bin", file) + require.NoError(t, err, "This upload should have succeeded") } func TestBogusTransferStart(t *testing.T) { s := NewTestServer(t, true) - - c, err := goftp.DialConfig(goftp.Config{User: "test", Password: "test"}, s.Addr()) - if err != nil { - t.Fatal(err) + conf := goftp.Config{ + User: authUser, + Password: authPass, } + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") rc, err := c.OpenRawConn() - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) { // Completely bogus port declaration status, resp, err := rc.SendCommand("PORT something") - if err != nil { - t.Fatal(err) - } - - if status != StatusSyntaxErrorNotRecognised { - t.Fatal("Bad status:", status, resp) - } + require.NoError(t, err) + require.Equal(t, StatusSyntaxErrorNotRecognised, status, resp) } { // Completely bogus port declaration status, resp, err := rc.SendCommand("EPRT something") - if err != nil { - t.Fatal(err) - } - - if status != StatusSyntaxErrorNotRecognised { - t.Fatal("Bad status:", status, resp) - } + require.NoError(t, err) + require.Equal(t, StatusSyntaxErrorNotRecognised, status, resp) } { // Bad port number: 0 status, resp, err := rc.SendCommand("EPRT |2|::1|0|") - if err != nil { - t.Fatal(err) - } - - if status != StatusSyntaxErrorNotRecognised { - t.Fatal("Bad status:", status, resp) - } + require.NoError(t, err) + require.Equal(t, StatusSyntaxErrorNotRecognised, status, resp) } { // Bad IP status, resp, err := rc.SendCommand("EPRT |1|253.254.255.256|2000|") - if err != nil { - t.Fatal(err) - } - - if status != StatusSyntaxErrorNotRecognised { - t.Fatal("Bad status:", status, resp) - } + require.NoError(t, err) + require.Equal(t, StatusSyntaxErrorNotRecognised, status, resp) } { // Bad protocol type: 3 status, resp, err := rc.SendCommand("EPRT |3|::1|2000|") - if err != nil { - t.Fatal(err) - } - - if status != StatusSyntaxErrorNotRecognised { - t.Fatal("Bad status:", status, resp) - } + require.NoError(t, err) + require.Equal(t, StatusSyntaxErrorNotRecognised, status, resp) } { // We end-up on a positive note status, resp, err := rc.SendCommand("EPRT |1|::1|2000|") - if err != nil { - t.Fatal(err) - } - - if status != StatusOK { - t.Fatal("Bad status:", status, resp) - } + require.NoError(t, err) + require.Equal(t, StatusOK, status, resp) } } @@ -365,35 +297,21 @@ func TestFailedFileClose(t *testing.T) { driver := &TestServerDriver{ Debug: true, } - s := NewTestServerWithDriver(t, driver) - conf := goftp.Config{ User: authUser, Password: authPass, } - - var err error - - var c *goftp.Client - - if c, err = goftp.DialConfig(conf, s.Addr()); err != nil { - t.Fatal("Couldn't connect", err) - } + c, err := goftp.DialConfig(conf, s.Addr()) + require.NoError(t, err, "Couldn't connect") defer func() { panicOnError(c.Close()) }() file := createTemporaryFile(t, 1*1024) driver.FileOverride = &failingCloser{File: *file} err = c.Store("file.bin", file) - - if err == nil { - t.Fatal("this upload should not succeed", err) - } - - if !strings.Contains(err.Error(), errFailingCloser.Error()) { - t.Errorf("got %s as the error message, want it to contain %s", err, errFailingCloser) - } + require.Error(t, err, "this upload should not succeed") + require.True(t, strings.Contains(err.Error(), errFailingCloser.Error())) } type failingCloser struct {