fix: avoid logging error when server stop is requested

client_handler_test.go

@@ -3,6 +3,7 @@ package ftpserver
 import (
 	"fmt"
 	"net"
+	"slices"
 	"sync"
 	"testing"
 	"time"
@@ -20,7 +21,7 @@ func TestConcurrency(t *testing.T) {
 	waitGroup := sync.WaitGroup{}
 	waitGroup.Add(nbClients)
 
-	for i := 0; i < nbClients; i++ {
+	for range nbClients {
 		go func() {
 			conf := goftp.Config{
 				User:     authUser,
@@ -47,7 +48,8 @@ func TestConcurrency(t *testing.T) {
 
 func TestDOS(t *testing.T) {
 	server := NewTestServer(t, true)
-	conn, err := net.DialTimeout("tcp", server.Addr(), 5*time.Second)
+	dialer := &net.Dialer{Timeout: 5 * time.Second}
+	conn, err := dialer.DialContext(t.Context(), "tcp", server.Addr())
 	require.NoError(t, err)
 
 	defer func() {
@@ -149,7 +151,8 @@ func TestConnectionNotAllowed(t *testing.T) {
 	}
 	s := NewTestServerWithTestDriver(t, driver)
 
-	conn, err := net.DialTimeout("tcp", s.Addr(), 5*time.Second)
+	dialer := &net.Dialer{Timeout: 5 * time.Second}
+	conn, err := dialer.DialContext(t.Context(), "tcp", s.Addr())
 	require.NoError(t, err)
 
 	defer func() {
@@ -315,13 +318,7 @@ second line
 }
 
 func isStringInSlice(s string, list []string) bool {
-	for _, c := range list {
-		if s == c {
-			return true
-		}
-	}
-
-	return false
+	return slices.Contains(list, s)
 }
 
 func TestUnknownCommand(t *testing.T) {
@@ -486,7 +483,7 @@ func TestExtraData(t *testing.T) {
 	require.Len(t, info, 1)
 
 	for k, v := range info {
-		ccInfo, ok := v.(map[string]interface{})
+		ccInfo, ok := v.(map[string]any)
 		require.True(t, ok)
 		extra, ok := ccInfo["extra"].(uint32)
 		require.True(t, ok)

handle_auth.go

@@ -11,15 +11,15 @@ func (c *clientHandler) handleUSER(user string) error {
 		err := verifier.PreAuthUser(c, user)
 		if err != nil {
 			c.writeMessage(StatusNotLoggedIn, fmt.Sprintf("User rejected: %v", err))
-			c.disconnect()
+			_ = c.disconnect()
 
 			return nil
 		}
 	}
 
 	if c.isTLSRequired() && !c.HasTLSForControl() {
 		c.writeMessage(StatusServiceNotAvailable, "TLS is required")
-		c.disconnect()
+		_ = c.disconnect()
 
 		return nil
 	}
@@ -52,7 +52,7 @@ 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()
+		_ = c.disconnect()
 
 		return true
 	}
@@ -82,14 +82,14 @@ func (c *clientHandler) handlePASS(param string) error {
 	switch {
 	case err == nil && c.driver == nil:
 		c.writeMessage(StatusNotLoggedIn, "Unexpected exception (driver is nil)")
-		c.disconnect()
+		_ = c.disconnect()
 	case err != nil:
 		if msg == "" {
 			msg = fmt.Sprintf("Authentication error: %v", err)
 		}
 
 		c.writeMessage(StatusNotLoggedIn, msg)
-		c.disconnect()
+		_ = c.disconnect()
 	default: // err == nil && c.driver != nil
 		if msg == "" {
 			msg = "Password ok, continue"

handle_auth_test.go

@@ -19,7 +19,8 @@ func panicOnError(err error) {
 func TestLoginSuccess(t *testing.T) {
 	server := NewTestServer(t, false)
 	// send a NOOP before the login, this doesn't seems possible using secsy/goftp so use the old way ...
-	conn, err := net.DialTimeout("tcp", server.Addr(), 5*time.Second)
+	dialer := &net.Dialer{Timeout: 5 * time.Second}
+	conn, err := dialer.DialContext(t.Context(), "tcp", server.Addr())
 	require.NoError(t, err)
 
 	defer func() {

handle_dirs_test.go

@@ -460,7 +460,8 @@ func TestPerClientTLSTransfer(t *testing.T) {
 
 func TestDirListingBeforeLogin(t *testing.T) {
 	s := NewTestServer(t, false)
-	conn, err := net.DialTimeout("tcp", s.Addr(), 5*time.Second)
+	dialer := &net.Dialer{Timeout: 5 * time.Second}
+	conn, err := dialer.DialContext(t.Context(), "tcp", s.Addr())
 	require.NoError(t, err)
 
 	defer func() {

handle_files.go

@@ -689,7 +689,7 @@ func (c *clientHandler) computeHashForFile(filePath string, algo HASHAlgo, start
 	case HASHAlgoSHA512:
 		chosenHashAlgo = sha512.New()
 	default:
-		return "", errUnknowHash
+		return "", errUnknownHash
 	}
 
 	file, err = c.getFileHandle(filePath, os.O_RDONLY, start)

handle_misc.go

@@ -10,7 +10,7 @@ import (
 	"time"
 )
 
-var errUnknowHash = errors.New("unknown hash algorithm")
+var errUnknownHash = errors.New("unknown hash algorithm")
 
 func (c *clientHandler) handleAUTH(_ string) error {
 	if tlsConfig, err := c.server.driver.GetTLSConfig(); err == nil {
@@ -316,7 +316,7 @@ func (c *clientHandler) handleQUIT(_ string) error {
 	}
 
 	c.writeMessage(StatusClosingControlConn, msg)
-	c.disconnect()
+	_ = c.disconnect()
 	c.reader = nil
 
 	return nil

server.go

@@ -2,6 +2,7 @@
 package ftpserver
 
 import (
+	"context"
 	"crypto/tls"
 	"errors"
 	"fmt"
@@ -208,7 +209,8 @@ func (server *FtpServer) Listen() error {
 }
 
 func (server *FtpServer) createListener() (net.Listener, error) {
-	listener, err := net.Listen("tcp", server.settings.ListenAddr)
+	lc := &net.ListenConfig{}
+	listener, err := lc.Listen(context.Background(), "tcp", server.settings.ListenAddr)
 	if err != nil {
 		server.Logger.Error("cannot listen on main port", "err", err, "listenAddr", server.settings.ListenAddr)
 
@@ -266,8 +268,6 @@ func (server *FtpServer) Serve() error {
 // It returns a boolean indicating if the error should stop the server and the error itself or none if it's a standard
 // scenario (e.g. a closed listener)
 func (server *FtpServer) handleAcceptError(err error, tempDelay *time.Duration) (bool, error) {
-	server.Logger.Error("Serve error", "err", err)
-
 	if errOp := (&net.OpError{}); errors.As(err, &errOp) {
 		// This means we just closed the connection and it's OK
 		if errOp.Err.Error() == "use of closed network connection" {
@@ -277,6 +277,8 @@ func (server *FtpServer) handleAcceptError(err error, tempDelay *time.Duration)
 		}
 	}
 
+	server.Logger.Error("Serve error", "err", err)
+
 	// see https://github.com/golang/go/blob/4aa1efed4853ea067d665a952eee77c52faac774/src/net/http/server.go#L3046
 	// & https://github.com/fclairamb/ftpserverlib/pull/352#pullrequestreview-1077459896
 	// The temporaryError method should replace net.Error.Temporary() when the go team

server_stop_test.go

@@ -0,0 +1,90 @@
+package ftpserver
+
+import (
+	"sync"
+	"testing"
+	"time"
+
+	log "github.com/fclairamb/go-log"
+	"github.com/stretchr/testify/require"
+)
+
+// TestServerStopDoesNotLogError tests that stopping a server doesn't log an error
+// when the listener is closed as expected
+func TestServerStopDoesNotLogError(t *testing.T) {
+	req := require.New(t)
+
+	// Create a server with a test driver
+	server := NewFtpServer(&TestServerDriver{
+		Settings: &Settings{
+			ListenAddr: "127.0.0.1:0", // Use dynamic port
+		},
+	})
+
+	// Use a custom logger that tracks error logs
+	mockLogger := &MockLogger{}
+	server.Logger = mockLogger
+
+	// Start listening
+	err := server.Listen()
+	req.NoError(err)
+
+	// Start serving in a goroutine
+	var serveErr error
+	var waitGroup sync.WaitGroup
+	waitGroup.Add(1)
+
+	go func() {
+		defer waitGroup.Done()
+		serveErr = server.Serve()
+	}()
+
+	// Give the server a moment to start accepting connections
+	time.Sleep(100 * time.Millisecond)
+
+	// Stop the server
+	err = server.Stop()
+	req.NoError(err)
+
+	// Wait for the Serve goroutine to finish
+	waitGroup.Wait()
+
+	// Serve should return nil (no error) when stopped normally
+	req.NoError(serveErr)
+
+	// Check that no error was logged for the "use of closed network connection"
+	// The mock logger should not have received any error logs
+	req.Empty(mockLogger.ErrorLogs, "Expected no error logs when stopping server, but got: %v", mockLogger.ErrorLogs)
+}
+
+// MockLogger captures log calls to verify behavior
+type MockLogger struct {
+	ErrorLogs []string
+	WarnLogs  []string
+	InfoLogs  []string
+	DebugLogs []string
+}
+
+func (m *MockLogger) Debug(message string, _ ...any) {
+	m.DebugLogs = append(m.DebugLogs, message)
+}
+
+func (m *MockLogger) Info(message string, _ ...any) {
+	m.InfoLogs = append(m.InfoLogs, message)
+}
+
+func (m *MockLogger) Warn(message string, _ ...any) {
+	m.WarnLogs = append(m.WarnLogs, message)
+}
+
+func (m *MockLogger) Error(message string, _ ...any) {
+	m.ErrorLogs = append(m.ErrorLogs, message)
+}
+
+func (m *MockLogger) Panic(message string, _ ...any) {
+	panic(message)
+}
+
+func (m *MockLogger) With(_ ...any) log.Logger {
+	return m
+}

server_test.go

@@ -84,7 +84,8 @@ func newFakeListener(err error) net.Listener {
 func TestCannotListen(t *testing.T) {
 	req := require.New(t)
 
-	portBlockerListener, err := net.Listen("tcp", "127.0.0.1:0")
+	lc := &net.ListenConfig{}
+	portBlockerListener, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0")
 	req.NoError(err)
 
 	defer func() { req.NoError(portBlockerListener.Close()) }()
@@ -107,7 +108,8 @@ func TestCannotListen(t *testing.T) {
 func TestListenWithBadTLSSettings(t *testing.T) {
 	req := require.New(t)
 
-	portBlockerListener, err := net.Listen("tcp", "127.0.0.1:0")
+	lc := &net.ListenConfig{}
+	portBlockerListener, err := lc.Listen(t.Context(), "tcp", "127.0.0.1:0")
 	req.NoError(err)
 
 	defer func() { req.NoError(portBlockerListener.Close()) }()
@@ -173,7 +175,6 @@ func TestQuoteDoubling(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			require.Equal(t, tt.want, quoteDoubling(tt.args.s))
 		})

transfer_active_test.go

@@ -26,7 +26,8 @@ func TestRemoteAddrFormat(t *testing.T) {
 }
 
 func TestActiveTransferFromPort20(t *testing.T) {
-	listener, err := net.Listen("tcp", ":20") //nolint:gosec
+	lc := &net.ListenConfig{}
+	listener, err := lc.Listen(t.Context(), "tcp", ":20")
 	if err != nil {
 		t.Skipf("Binding on port 20 is not supported here: %v", err)
 	}

transfer_test.go

@@ -1104,7 +1104,8 @@ func TestPASVIPMatch(t *testing.T) {
 
 	server := NewTestServer(t, false)
 
-	conn, err := net.DialTimeout("tcp", server.Addr(), 5*time.Second)
+	dialer := &net.Dialer{Timeout: 5 * time.Second}
+	conn, err := dialer.DialContext(t.Context(), "tcp", server.Addr())
 	require.NoError(t, err)
 
 	defer func() {
@@ -1187,7 +1188,7 @@ func TestPassivePortExhaustion(t *testing.T) {
 
 	defer func() { require.NoError(t, raw.Close()) }()
 
-	for i := 0; i < 20; i++ {
+	for range 20 {
 		rc, message, err := raw.SendCommand("PASV")
 		require.NoError(t, err)
 		require.Equal(t, StatusEnteringPASV, rc, message)