From ffcde438b0bd2c1323257e04d646d7a17a088a22 Mon Sep 17 00:00:00 2001 From: fclairamb Date: Sun, 10 Mar 2024 22:21:07 +0100 Subject: [PATCH] progress --- .golangci.yml | 6 +++++- client_handler_test.go | 1 - control_unix.go | 1 - driver.go | 4 ---- driver_test.go | 17 ++++++++++------- handle_auth.go | 2 -- handle_dirs.go | 9 +++------ handle_dirs_test.go | 4 ++-- handle_files.go | 5 ----- server.go | 14 +++----------- transfer_active.go | 4 ---- transfer_pasv.go | 1 - transfer_test.go | 9 ++++++--- 13 files changed, 29 insertions(+), 48 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 3ae146ef..0535eb8d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -202,6 +202,7 @@ linters: - godox - goerr113 - gofmt + # - gofumpt -> conflicts with wsl - goimports - gosimple # - golint --> revive @@ -237,6 +238,7 @@ linters: - prealloc - nestif - nilerr + - nilnil - nolintlint - nlreturn - rowserrcheck @@ -274,10 +276,12 @@ linters: - unused - usestdlibvars - varnamelen + - wastedassign # - varcheck -> unused - whitespace - # - wrapcheck + # - wrapcheck -> too much effort for now - wsl + # - zerologlint -> Will most probably never use it fast: false issues: diff --git a/client_handler_test.go b/client_handler_test.go index cdfcd3fc..56c74957 100644 --- a/client_handler_test.go +++ b/client_handler_test.go @@ -28,7 +28,6 @@ func TestConcurrency(t *testing.T) { } client, err := goftp.DialConfig(conf, server.Addr()) - if err != nil { panic(fmt.Sprintf("Couldn't connect: %v", err)) } diff --git a/control_unix.go b/control_unix.go index a81aa5a4..c15f19c9 100644 --- a/control_unix.go +++ b/control_unix.go @@ -25,7 +25,6 @@ func Control(_, _ string, c syscall.RawConn) error { return } }) - if err != nil { return fmt.Errorf("unable to set control options: %w", err) } diff --git a/driver.go b/driver.go index d95f8716..80316a77 100644 --- a/driver.go +++ b/driver.go @@ -78,14 +78,12 @@ type ClientDriver interface { // ClientDriverExtensionAllocate is an extension to support the "ALLO" - file allocation - command type ClientDriverExtensionAllocate interface { - // AllocateSpace reserves the space necessary to upload files AllocateSpace(size int) error } // ClientDriverExtensionSymlink is an extension to support the "SITE SYMLINK" - symbolic link creation - command type ClientDriverExtensionSymlink interface { - // Symlink creates a symlink Symlink(oldname, newname string) error @@ -96,7 +94,6 @@ type ClientDriverExtensionSymlink interface { // ClientDriverExtensionFileList is a convenience extension to allow to return file listing // without requiring to implement the methods Open/Readdir for your custom afero.File type ClientDriverExtensionFileList interface { - // ReadDir reads the directory named by name and return a list of directory entries. ReadDir(name string) ([]os.FileInfo, error) } @@ -104,7 +101,6 @@ type ClientDriverExtensionFileList interface { // ClientDriverExtentionFileTransfer is a convenience extension to allow to transfer files // without requiring to implement the methods Create/Open/OpenFile for your custom afero.File. type ClientDriverExtentionFileTransfer interface { - // GetHandle return an handle to upload or download a file based on flags: // os.O_RDONLY indicates a download // os.O_WRONLY indicates an upload and can be combined with os.O_APPEND (resume) or diff --git a/driver_test.go b/driver_test.go index 69193b51..1e11572d 100644 --- a/driver_test.go +++ b/driver_test.go @@ -57,7 +57,7 @@ func (driver *TestServerDriver) Init() { { dir, _ := os.MkdirTemp("", "example") - if err := os.MkdirAll(dir, 0750); err != nil { + if err := os.MkdirAll(dir, 0o750); err != nil { panic(err) } @@ -260,7 +260,7 @@ func (driver *TestServerDriver) AuthUser(_ ClientContext, user, pass string) (Cl return clientdriver, nil } else if user == "nil" && pass == "nil" { // Definitely a bad behavior (but can be done on the driver side) - return nil, nil + return nil, nil //nolint:nilnil } return nil, errBadUserNameOrPassword @@ -370,7 +370,8 @@ func (driver *TestServerDriver) PreAuthUser(cc ClientContext, _ string) error { } func (driver *TestServerDriver) VerifyConnection(_ ClientContext, _ string, - _ *tls.Conn) (ClientDriver, error) { + _ *tls.Conn, +) (ClientDriver, error) { switch driver.TLSVerificationReply { case tlsVerificationFailed: return nil, errInvalidTLSCertificate @@ -379,10 +380,10 @@ func (driver *TestServerDriver) VerifyConnection(_ ClientContext, _ string, return clientdriver, nil case tlsVerificationOK: - return nil, nil + return nil, nil //nolint:nilnil } - return nil, nil + return nil, nil //nolint:nilnil } func (driver *TestServerDriver) WrapPassiveListener(listener net.Listener) (net.Listener, error) { @@ -458,8 +459,10 @@ func (driver *TestClientDriver) GetAvailableSpace(dirName string) (int64, error) return int64(123), nil } -var errInvalidChownUser = errors.New("invalid chown on user") -var errInvalidChownGroup = errors.New("invalid chown on group") +var ( + errInvalidChownUser = errors.New("invalid chown on user") + errInvalidChownGroup = errors.New("invalid chown on group") +) func (driver *TestClientDriver) Chown(name string, uid int, gid int) error { if uid != 0 && uid != authUserID { diff --git a/handle_auth.go b/handle_auth.go index ca0c4376..7c200abd 100644 --- a/handle_auth.go +++ b/handle_auth.go @@ -9,7 +9,6 @@ import ( func (c *clientHandler) handleUSER(user string) error { if verifier, ok := c.server.driver.(MainDriverExtensionUserVerifier); ok { err := verifier.PreAuthUser(c, user) - if err != nil { c.writeMessage(StatusNotLoggedIn, fmt.Sprintf("User rejected: %v", err)) c.disconnect() @@ -51,7 +50,6 @@ func (c *clientHandler) handleUserTLS(user string) bool { } driver, err := verifier.VerifyConnection(c, user, tlsConn) - if err != nil { c.writeMessage(StatusNotLoggedIn, fmt.Sprintf("TLS verification failed: %v", err)) c.disconnect() diff --git a/handle_dirs.go b/handle_dirs.go index e13edc5d..d3473e58 100644 --- a/handle_dirs.go +++ b/handle_dirs.go @@ -77,7 +77,7 @@ func (c *clientHandler) handleCWD(param string) error { func (c *clientHandler) handleMKD(param string) error { pathAbsolute := c.absPath(param) - if err := c.driver.Mkdir(pathAbsolute, 0755); err == nil { + if err := c.driver.Mkdir(pathAbsolute, 0o755); err == nil { // handleMKD confirms to "quote-doubling" // https://tools.ietf.org/html/rfc959 , page 63 c.writeMessage(StatusPathCreated, fmt.Sprintf(`Created dir "%s"`, quoteDoubling(pathAbsolute))) @@ -97,7 +97,7 @@ func (c *clientHandler) handleMKDIR(params string) { p := c.absPath(params) - if err := c.driver.MkdirAll(p, 0755); err == nil { + if err := c.driver.MkdirAll(p, 0o755); err == nil { c.writeMessage(StatusFileOK, "Created dir "+p) } else { c.writeMessage(StatusActionNotTaken, fmt.Sprintf("Couldn't create dir %s: %v", p, err)) @@ -226,7 +226,6 @@ func (c *clientHandler) handleNLST(param string) error { func (c *clientHandler) dirTransferNLST(writer io.Writer, files []os.FileInfo, parentDir string) error { if len(files) == 0 { _, err := writer.Write([]byte("")) - if err != nil { err = newNetworkError("couldn't send NLST data", err) } @@ -306,7 +305,6 @@ func (c *clientHandler) fileStat(file os.FileInfo) string { func (c *clientHandler) dirTransferLIST(writer io.Writer, files []os.FileInfo) error { if len(files) == 0 { _, err := writer.Write([]byte("")) - if err != nil { err = newNetworkError("error writing LIST entry", err) } @@ -327,7 +325,6 @@ func (c *clientHandler) dirTransferLIST(writer io.Writer, files []os.FileInfo) e func (c *clientHandler) dirTransferMLSD(writer io.Writer, files []os.FileInfo) error { if len(files) == 0 { _, err := writer.Write([]byte("")) - if err != nil { err = newNetworkError("error writing MLSD entry", err) } @@ -343,6 +340,7 @@ func (c *clientHandler) dirTransferMLSD(writer io.Writer, files []os.FileInfo) e return nil } + func (c *clientHandler) writeMLSxEntry(writer io.Writer, file os.FileInfo) error { var listType string if file.IsDir() { @@ -359,7 +357,6 @@ func (c *clientHandler) writeMLSxEntry(writer io.Writer, file os.FileInfo) error file.ModTime().UTC().Format(dateFormatMLSD), file.Name(), ) - if err != nil { err = fmt.Errorf("error writing MLSD entry: %w", err) } diff --git a/handle_dirs_test.go b/handle_dirs_test.go index 0ddcd2d2..ac69c3ba 100644 --- a/handle_dirs_test.go +++ b/handle_dirs_test.go @@ -19,7 +19,7 @@ func TestGetRelativePaths(t *testing.T) { type relativePathTest struct { workingDir, path, result string } - var tests = []relativePathTest{ + tests := []relativePathTest{ {"/", "/p", "p"}, {"/", "/", ""}, {"/p", "/p", ""}, @@ -590,7 +590,7 @@ func TestMLSDAndNLSTFilePathError(t *testing.T) { defer func() { panicOnError(client.Close()) }() // MLSD shouldn't work for filePaths - var fileName = "testfile.ext" + fileName := "testfile.ext" _, err = client.ReadDir(fileName) require.Error(t, err, "MLSD for not existing filePath must fail") diff --git a/handle_files.go b/handle_files.go index f6ccf4e2..bbf895ff 100644 --- a/handle_files.go +++ b/handle_files.go @@ -70,7 +70,6 @@ func (c *clientHandler) transferFile(write bool, appendFile bool, param, info st } file, err = c.getFileHandle(path, fileFlag, c.ctxRest) - // If this fail, can stop right here and reset the seek position if err != nil { if !c.isCommandAborted() { @@ -420,7 +419,6 @@ func (c *clientHandler) handleSTATFile(param string) error { path := c.absPath(param) info, err := c.driver.Stat(path) - if err != nil { c.writeMessage(StatusFileActionNotTaken, fmt.Sprintf("Could not STAT: %v", err)) @@ -496,7 +494,6 @@ func (c *clientHandler) handleMLST(param string) error { func (c *clientHandler) handleALLO(param string) error { // We should probably add a method in the driver size, err := strconv.Atoi(param) - if err != nil { c.writeMessage(StatusSyntaxErrorParameters, fmt.Sprintf("Couldn't parse size: %v", err)) @@ -701,7 +698,6 @@ func (c *clientHandler) computeHashForFile(filePath string, algo HASHAlgo, start } file, err = c.getFileHandle(filePath, os.O_RDONLY, start) - if err != nil { return "", err } @@ -735,7 +731,6 @@ func (c *clientHandler) getFileHandle(name string, flags int, offset int64) (Fil } file, err := c.driver.OpenFile(name, flags, os.ModePerm) - if err != nil { err = newDriverError("calling OpenFile", err) } diff --git a/server.go b/server.go index eb20a5e2..91b4b0a5 100644 --- a/server.go +++ b/server.go @@ -13,10 +13,8 @@ import ( lognoop "github.com/fclairamb/go-log/noop" ) -var ( - // ErrNotListening is returned when we are performing an action that is only valid while listening - ErrNotListening = errors.New("we aren't listening") -) +// ErrNotListening is returned when we are performing an action that is only valid while listening +var ErrNotListening = errors.New("we aren't listening") // CommandDescription defines which function should be used and if it should be open to anyone or only logged in users type CommandDescription struct { @@ -95,9 +93,7 @@ var commandsMap = map[string]*CommandDescription{ //nolint:gochecknoglobals "EPRT": {Fn: (*clientHandler).handlePORT}, } -var ( - specialAttentionCommands = []string{"ABOR", "STAT", "QUIT"} //nolint:gochecknoglobals -) +var specialAttentionCommands = []string{"ABOR", "STAT", "QUIT"} //nolint:gochecknoglobals // FtpServer is where everything is stored // We want to keep it as simple as possible @@ -163,7 +159,6 @@ func parseIPv4(publicHost string) (string, error) { // It's not a blocking call func (server *FtpServer) Listen() error { err := server.loadSettings() - if err != nil { return fmt.Errorf("could not load settings: %w", err) } @@ -174,7 +169,6 @@ func (server *FtpServer) Listen() error { } else { // Otherwise, it's what we currently use server.listener, err = server.createListener() - if err != nil { return fmt.Errorf("could not create listener: %w", err) } @@ -187,7 +181,6 @@ func (server *FtpServer) Listen() error { func (server *FtpServer) createListener() (net.Listener, error) { listener, err := net.Listen("tcp", server.settings.ListenAddr) - if err != nil { server.Logger.Error("cannot listen on main port", "err", err, "listenAddr", server.settings.ListenAddr) @@ -227,7 +220,6 @@ func (server *FtpServer) Serve() error { for { connection, err := server.listener.Accept() - if err != nil { if ok, finalErr := server.handleAcceptError(err, &tempDelay); ok { return finalErr diff --git a/transfer_active.go b/transfer_active.go index a3c51096..8ba853a6 100644 --- a/transfer_active.go +++ b/transfer_active.go @@ -99,7 +99,6 @@ func (a *activeTransferHandler) Open() (net.Conn, error) { } conn, err := dialer.Dial("tcp", a.raddr.String()) - if err != nil { return nil, newNetworkError("could not establish active connection", err) } @@ -151,7 +150,6 @@ func parsePORTAddr(param string) (*net.TCPAddr, error) { } portByte2, err := strconv.Atoi(params[5]) - if err != nil { return nil, ErrRemoteAddrFormat } @@ -159,7 +157,6 @@ func parsePORTAddr(param string) (*net.TCPAddr, error) { port := portByte1<<8 + portByte2 addr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("%s:%d", ipParts, port)) - if err != nil { err = newNetworkError("could not resolve "+param, err) } @@ -204,7 +201,6 @@ func parseEPRTAddr(param string) (*net.TCPAddr, error) { } addr, err = net.ResolveTCPAddr("tcp", net.JoinHostPort(ipAddress.String(), strconv.Itoa(portI))) - if err != nil { err = newNetworkError(fmt.Sprintf("could not resolve addr %v:%v", ipAddress, portI), err) } diff --git a/transfer_pasv.go b/transfer_pasv.go index 414db8f1..bd0c0324 100644 --- a/transfer_pasv.go +++ b/transfer_pasv.go @@ -225,7 +225,6 @@ func (p *passiveTransferHandler) ConnectionWait(wait time.Duration) (net.Conn, e } p.connection, err = p.listener.Accept() - if err != nil { return nil, fmt.Errorf("failed to accept passive transfer connection: %w", err) } diff --git a/transfer_test.go b/transfer_test.go index 04eff1a3..20aee28f 100644 --- a/transfer_test.go +++ b/transfer_test.go @@ -263,7 +263,8 @@ func TestTransfer(t *testing.T) { Settings: &Settings{ ActiveTransferPortNon20: true, TLSRequired: ImplicitEncryption, - }}) + }, + }) testTransferOnConnection(t, server, false, true, true) testTransferOnConnection(t, server, true, true, true) @@ -747,8 +748,10 @@ func TestABORWithoutOpenTransfer(t *testing.T) { require.NoError(t, err) require.Equal(t, StatusFileActionPending, returnCode, response) - for _, cmd := range []string{"RETR delay-io-fail-to-seek.bin", "LIST delay-io-fail-to-readdir", - "NLST delay-io-fail-to-readdir", "MLSD delay-io-fail-to-readdir"} { + for _, cmd := range []string{ + "RETR delay-io-fail-to-seek.bin", "LIST delay-io-fail-to-readdir", + "NLST delay-io-fail-to-readdir", "MLSD delay-io-fail-to-readdir", + } { _, err = raw.PrepareDataConn() require.NoError(t, err)