ftpserverlib icon indicating copy to clipboard operation
ftpserverlib copied to clipboard

Double close of client connection upon when a client gracefully QUIT

Open folays opened this issue 1 year ago • 2 comments

Hello,

Tested with netcat client to the FTPd port, the client just does USER + PASS + QUIT, and I am polite on the client side (I keep the netcat connection opened after sending the QUIT command).

There is a double c.Conn.Close() On the ftpserverlib server when a client send a gracefully QUIT message :

  1. func (c *clientHandler) handleQUIT(_ string) error { in https://github.com/fclairamb/ftpserverlib/blob/v0.24.1/handle_misc.go#L298 calls c.disconnect() which in func (c *clientHandler) disconnect() { at https://github.com/fclairamb/ftpserverlib/blob/v0.24.1/client_handler.go#L134 does a FIRST (*clientHandler).conn.Close()

  2. then X nanoseconds later func (c *clientHandler) HandleCommands() { in https://github.com/fclairamb/ftpserverlib/blob/v0.24.1/client_handler.go#L401 has a :

func (c *clientHandler) HandleCommands() {
	defer c.end() // <----------------- here a problematic defer ?
        [...]
	for {
		if c.readCommand() {
			return
		}
	}

defer which calls c.conn.Close() a second time at https://github.com/fclairamb/ftpserverlib/blob/v0.24.1/client_handler.go#L375

func (c *clientHandler) end() {
	c.server.driver.ClientDisconnected(c)
	c.server.clientDeparture(c)

	if err := c.conn.Close(); err != nil { // <------------- second close
		c.logger.Debug(
			"Problem closing control connection",
			"err", err,
		)
	}

SYMPTOM :

Here on a Linux, at least on a loopback TCP connection, superfluous messages appear in the logs :

Problem closing control connection | clientId="1" err="close tcp [::1]:21->[::1]:34446: use of closed network connection"

This is probably related to https://github.com/fclairamb/ftpserver/issues/710

folays avatar Oct 14 '24 08:10 folays

is affect file transfer,or just log

xiaoweihong avatar May 26 '25 12:05 xiaoweihong

is affect file transfer,or just log

Sorry I wasn't clear when I said superfluous message ; It is only cosmetic, functionality is not affected, it's just a confusing log message.

folays avatar Jun 05 '25 01:06 folays

This has been fixed with #560.

fclairamb avatar Sep 01 '25 22:09 fclairamb