diff --git a/client_handler.go b/client_handler.go index 6d336b72..d3313295 100644 --- a/client_handler.go +++ b/client_handler.go @@ -360,7 +360,7 @@ func (c *clientHandler) Close() error { // closing the connection from a different goroutine should be safe err := c.conn.Close() if err != nil { - err = NewNetworkError("error closing control connection", err) + err = newNetworkError("error closing control connection", err) } return err @@ -650,7 +650,7 @@ func (c *clientHandler) TransferOpen(info string) (net.Conn, error) { c.writeMessage(StatusCannotOpenDataConnection, err.Error()) - err = NewNetworkError("Unable to open transfer", err) + err = newNetworkError("Unable to open transfer", err) return nil, err } diff --git a/errors.go b/errors.go index 51706f2c..6319bba5 100644 --- a/errors.go +++ b/errors.go @@ -31,7 +31,7 @@ type DriverError struct { err error } -func NewDriverError(str string, err error) DriverError { +func newDriverError(str string, err error) DriverError { return DriverError{str: str, err: err} } @@ -43,12 +43,13 @@ func (e DriverError) Unwrap() error { return e.err } +// NetworkError is a wrapper for any error that occur while contacting the network type NetworkError struct { str string err error } -func NewNetworkError(str string, err error) NetworkError { +func newNetworkError(str string, err error) NetworkError { return NetworkError{str: str, err: err} } @@ -60,12 +61,13 @@ func (e NetworkError) Unwrap() error { return e.err } +// FileAccessError is a wrapper for any error that occur while accessing the file system type FileAccessError struct { str string err error } -func NewFileAccessError(str string, err error) FileAccessError { +func newFileAccessError(str string, err error) FileAccessError { return FileAccessError{str: str, err: err} } diff --git a/errors_test.go b/errors_test.go index 0282def8..56e3421a 100644 --- a/errors_test.go +++ b/errors_test.go @@ -29,9 +29,10 @@ func TestTransferCloseStorageExceeded(t *testing.T) { } func TestErrorTypes(t *testing.T) { - a := assert.New(t) + // a := assert.New(t) t.Run("DriverError", func(t *testing.T) { - var err error = NewDriverError("test", os.ErrPermission) + a := assert.New(t) + var err error = newDriverError("test", os.ErrPermission) a.Equal("driver error: test: permission denied", err.Error()) a.ErrorIs(err, os.ErrPermission) @@ -41,7 +42,8 @@ func TestErrorTypes(t *testing.T) { }) t.Run("NetworkError", func(t *testing.T) { - var err error = NewNetworkError("test", os.ErrPermission) + a := assert.New(t) + var err error = newNetworkError("test", os.ErrPermission) a.Equal("network error: test: permission denied", err.Error()) a.ErrorIs(err, os.ErrPermission) @@ -51,7 +53,8 @@ func TestErrorTypes(t *testing.T) { }) t.Run("FileAccessError", func(t *testing.T) { - var err error = NewFileAccessError("test", os.ErrPermission) + a := assert.New(t) + var err error = newFileAccessError("test", os.ErrPermission) a.Equal("file access error: test: permission denied", err.Error()) a.ErrorIs(err, os.ErrPermission) var specificError FileAccessError diff --git a/handle_dirs.go b/handle_dirs.go index 9b6ab7f3..cdd09d64 100644 --- a/handle_dirs.go +++ b/handle_dirs.go @@ -228,7 +228,7 @@ func (c *clientHandler) dirTransferNLST(w io.Writer, files []os.FileInfo, parent _, err := w.Write([]byte("")) if err != nil { - err = NewNetworkError("couldn't send NLST data", err) + err = newNetworkError("couldn't send NLST data", err) } return err @@ -239,7 +239,7 @@ func (c *clientHandler) dirTransferNLST(w io.Writer, files []os.FileInfo, parent // 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 NewNetworkError("couldn't send NLST data", err) + return newNetworkError("couldn't send NLST data", err) } } @@ -304,7 +304,7 @@ func (c *clientHandler) dirTransferLIST(w io.Writer, files []os.FileInfo) error _, err := w.Write([]byte("")) if err != nil { - err = NewNetworkError("error writing LIST entry", err) + err = newNetworkError("error writing LIST entry", err) } return err @@ -325,7 +325,7 @@ func (c *clientHandler) dirTransferMLSD(w io.Writer, files []os.FileInfo) error _, err := w.Write([]byte("")) if err != nil { - err = NewNetworkError("error writing MLSD entry", err) + err = newNetworkError("error writing MLSD entry", err) } return err @@ -374,7 +374,7 @@ func (c *clientHandler) getFileList(param string, filePathAllowed bool) ([]os.Fi // return list of single file if directoryPath points to file and filePathAllowed info, err := c.driver.Stat(listPath) if err != nil { - return nil, "", NewFileAccessError("couldn't stat", err) + return nil, "", newFileAccessError("couldn't stat", err) } if !info.IsDir() { @@ -395,7 +395,7 @@ func (c *clientHandler) getFileList(param string, filePathAllowed bool) ([]os.Fi directory, errOpenFile := c.driver.Open(listPath) if errOpenFile != nil { - return nil, "", NewFileAccessError("couldn't open directory", errOpenFile) + return nil, "", newFileAccessError("couldn't open directory", errOpenFile) } defer c.closeDirectory(listPath, directory) diff --git a/handle_files.go b/handle_files.go index fa333ff1..bde6ff5f 100644 --- a/handle_files.go +++ b/handle_files.go @@ -165,7 +165,7 @@ func (c *clientHandler) doFileTransfer(tr net.Conn, file io.ReadWriter, write bo fileTransferError.TransferError(err) } - err = NewNetworkError("error transferring data", err) + err = newNetworkError("error transferring data", err) } return err @@ -700,14 +700,14 @@ func (c *clientHandler) computeHashForFile(filePath string, algo HASHAlgo, start if start > 0 { _, err = file.Seek(start, io.SeekStart) if err != nil { - return "", NewFileAccessError("couldn't seek file", err) + return "", newFileAccessError("couldn't seek file", err) } } _, err = io.CopyN(h, file, end-start) if err != nil && err != io.EOF { - return "", NewFileAccessError("couldn't read file", err) + return "", newFileAccessError("couldn't read file", err) } return hex.EncodeToString(h.Sum(nil)), nil @@ -717,7 +717,7 @@ func (c *clientHandler) getFileHandle(name string, flags int, offset int64) (Fil if fileTransfer, ok := c.driver.(ClientDriverExtentionFileTransfer); ok { ft, err := fileTransfer.GetHandle(name, flags, offset) if err != nil { - err = NewDriverError("calling GetHandle", err) + err = newDriverError("calling GetHandle", err) } return ft, err @@ -726,7 +726,7 @@ func (c *clientHandler) getFileHandle(name string, flags int, offset int64) (Fil ft, err := c.driver.OpenFile(name, flags, os.ModePerm) if err != nil { - err = NewDriverError("calling OpenFile", err) + err = newDriverError("calling OpenFile", err) } return ft, err diff --git a/server.go b/server.go index 1e1a4546..0462b219 100644 --- a/server.go +++ b/server.go @@ -114,7 +114,7 @@ func (server *FtpServer) loadSettings() error { s, err := server.driver.GetSettings() if err != nil || s == nil { - return NewDriverError("couldn't load settings", err) + return newDriverError("couldn't load settings", err) } if s.PublicHost != "" { @@ -172,7 +172,7 @@ func (server *FtpServer) Listen() error { if err != nil { server.Logger.Error("cannot listen on main port", "err", err, "listenAddr", server.settings.ListenAddr) - return NewNetworkError("cannot listen on main port", err) + return newNetworkError("cannot listen on main port", err) } if server.settings.TLSRequired == ImplicitEncryption { @@ -183,7 +183,7 @@ func (server *FtpServer) Listen() error { if err != nil || tlsConfig == nil { server.Logger.Error("Cannot get tls config", "err", err) - return NewDriverError("cannot get tls config", err) + return newDriverError("cannot get tls config", err) } server.listener = tls.NewListener(server.listener, tlsConfig) @@ -252,7 +252,7 @@ func (server *FtpServer) Serve() error { server.Logger.Error("Listener accept error", "err", err) - return NewNetworkError("listener accept error", err) + return newNetworkError("listener accept error", err) } tempDelay = 0 @@ -301,7 +301,7 @@ func (server *FtpServer) Stop() error { "err", err, ) - return NewNetworkError("couln't close listener", err) + return newNetworkError("couln't close listener", err) } return nil diff --git a/server_test.go b/server_test.go index 95b82631..a78027d9 100644 --- a/server_test.go +++ b/server_test.go @@ -89,7 +89,7 @@ func TestCannotListen(t *testing.T) { portBlockerListener, err := net.Listen("tcp", "127.0.0.1:0") a.NoError(err) - defer portBlockerListener.Close() + defer func() { a.NoError(portBlockerListener.Close()) }() server := FtpServer{ Logger: lognoop.NewNoOpLogger(), @@ -112,7 +112,7 @@ func TestListenWithBadTLSSettings(t *testing.T) { portBlockerListener, err := net.Listen("tcp", "127.0.0.1:0") a.NoError(err) - defer portBlockerListener.Close() + defer func() { a.NoError(portBlockerListener.Close()) }() server := FtpServer{ Logger: lognoop.NewNoOpLogger(), diff --git a/transfer_active.go b/transfer_active.go index 82d904b8..7881b5a5 100644 --- a/transfer_active.go +++ b/transfer_active.go @@ -101,7 +101,7 @@ 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) + return nil, newNetworkError("could not establish active connection", err) } if a.tlsConfig != nil { @@ -118,7 +118,7 @@ func (a *activeTransferHandler) Open() (net.Conn, error) { func (a *activeTransferHandler) Close() error { if a.conn != nil { if err := a.conn.Close(); err != nil { - return NewNetworkError("could not close active connection", err) + return newNetworkError("could not close active connection", err) } } @@ -161,7 +161,7 @@ func parsePORTAddr(param string) (*net.TCPAddr, error) { addr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("%s:%d", ip, port)) if err != nil { - err = NewNetworkError(fmt.Sprintf("could not resolve %s", param), err) + err = newNetworkError(fmt.Sprintf("could not resolve %s", param), err) } return addr, err @@ -203,7 +203,7 @@ func parseEPRTAddr(param string) (addr *net.TCPAddr, err error) { addr, err = net.ResolveTCPAddr("tcp", net.JoinHostPort(ip.String(), strconv.Itoa(portI))) if err != nil { - err = NewNetworkError(fmt.Sprintf("could not resolve addr %v:%v", ip, portI), err) + err = newNetworkError(fmt.Sprintf("could not resolve addr %v:%v", ip, portI), err) } return addr, err diff --git a/transfer_pasv.go b/transfer_pasv.go index 223c1fc4..01bf3848 100644 --- a/transfer_pasv.go +++ b/transfer_pasv.go @@ -47,11 +47,6 @@ func (e *ipValidationError) Error() string { return e.error } -const ( - PortSearchMinAttempts = 10 - PortSearchMaxAttempts = 1000 -) - func (c *clientHandler) getCurrentIP() ([]string, error) { // Provide our external IP address so the ftp client can connect back to us ip := c.server.settings.PublicHost @@ -84,14 +79,19 @@ func (c *clientHandler) getCurrentIP() ([]string, error) { // ErrNoAvailableListeningPort is returned when no port could be found to accept incoming connection var ErrNoAvailableListeningPort = errors.New("could not find any port to listen to") +const ( + portSearchMinAttempts = 10 + portSearchMaxAttempts = 1000 +) + func (c *clientHandler) findListenerWithinPortRange(portRange *PortRange) (*net.TCPListener, error) { nbAttempts := portRange.End - portRange.Start // Making sure we trying a reasonable amount of ports before giving up - if nbAttempts < PortSearchMinAttempts { - nbAttempts = PortSearchMinAttempts - } else if nbAttempts > PortSearchMaxAttempts { - nbAttempts = PortSearchMaxAttempts + if nbAttempts < portSearchMinAttempts { + nbAttempts = portSearchMinAttempts + } else if nbAttempts > portSearchMaxAttempts { + nbAttempts = portSearchMaxAttempts } for i := 0; i < nbAttempts; i++ { @@ -102,7 +102,7 @@ func (c *clientHandler) findListenerWithinPortRange(portRange *PortRange) (*net. if errResolve != nil { c.logger.Error("Problem resolving local port", "err", errResolve, "port", port) - return nil, NewNetworkError(fmt.Sprintf("could not resolve port %d", port), errResolve) + return nil, newNetworkError(fmt.Sprintf("could not resolve port %d", port), errResolve) } tcpListener, errListen := net.ListenTCP("tcp", laddr)