diff --git a/README.md b/README.md index cfb9eb1..eda7e7d 100644 --- a/README.md +++ b/README.md @@ -2667,6 +2667,9 @@ IRC_LISTEN_ADDR: "" An optional `PASS` before registration sets the session password (minimum 8 characters). Registering creates a session without a hashcash stamp; closing the connection ends the session, as `QUIT` does. +- **PING**: a connection that sends nothing for 90 seconds is sent a `PING`, and + is closed if it sends nothing in the 30 seconds after that; any line, such as + the client's `PONG`, starts both waits again. - **CAP negotiation**: `CAP LS` gets an empty capability list and `CAP END` is ignored, for compatibility with modern clients. - **Channel prefixes**: Channels must start with `#`. `JOIN` prepends it if diff --git a/internal/ircserver/conn.go b/internal/ircserver/conn.go index 8fc9224..e97feaa 100644 --- a/internal/ircserver/conn.go +++ b/internal/ircserver/conn.go @@ -20,7 +20,6 @@ import ( const ( maxLineLen = 512 - readTimeout = 5 * time.Minute writeTimeout = 30 * time.Second killWriteWindow = 2 * time.Second dnsTimeout = 3 * time.Second @@ -46,6 +45,11 @@ type Conn struct { serverSfx string commands map[string]cmdHandler + // pingInterval and pongDeadline are the constants of the + // same names, except in tests, which shorten them. + pingInterval time.Duration + pongDeadline time.Duration + // writeMu serializes writes to conn, which come from // serve(), from the relay goroutine and, for KILL, from // Disconnect. @@ -96,6 +100,9 @@ func newConn( serverSfx: srvName, remoteIP: host, hostname: resolveHost(ctx, host), + + pingInterval: pingInterval, + pongDeadline: pongDeadline, } conn.commands = conn.buildCommandMap() @@ -214,17 +221,28 @@ func resolveHost(ctx context.Context, addr string) string { // serve is the main loop for a single IRC client connection. // Cancelling ctx when it returns stops the relay goroutine. +// When nothing has been read for pingInterval the client is +// sent a PING, and when nothing is read within pongDeadline +// after that the connection is closed. func (c *Conn) serve(ctx context.Context) { ctx, cancel := context.WithCancel(ctx) defer cancel() defer c.cleanup(ctx) + ping := time.AfterFunc(c.pingInterval, func() { + c.send("PING :" + c.serverSfx) + }) + defer ping.Stop() + scanner := bufio.NewScanner(c.conn) scanner.Buffer(make([]byte, maxLineLen), maxLineLen) for { + // Every line read starts both waits again. + ping.Reset(c.pingInterval) + _ = c.conn.SetReadDeadline( - time.Now().Add(readTimeout), + time.Now().Add(c.pingInterval + c.pongDeadline), ) if !scanner.Scan() { @@ -366,7 +384,7 @@ func (c *Conn) handleMessage( msg *Message, ) { // Before registration, only NICK, USER, PASS, PING, - // QUIT, and CAP are accepted. + // PONG, QUIT, and CAP are accepted. if !c.registered { c.handlePreRegistration(ctx, msg) @@ -452,6 +470,10 @@ func (c *Conn) handlePreRegistration( case irc.CmdPing: c.handlePing(msg) + return + case irc.CmdPong: + // The answer to serve's PING, which unregistered + // clients are sent too. return case irc.CmdQuit: c.handleQuit(msg) diff --git a/internal/ircserver/export_test.go b/internal/ircserver/export_test.go index 60d039b..9281491 100644 --- a/internal/ircserver/export_test.go +++ b/internal/ircserver/export_test.go @@ -64,6 +64,23 @@ const KillWriteWindow = killWriteWindow // output queue when no broker notification arrives. const PollInterval = pollInterval +// ServeConn serves tcpConn as s serves each connection it +// accepts, except that interval and deadline replace +// pingInterval and pongDeadline. +func (s *Server) ServeConn( + tcpConn net.Conn, + interval, deadline time.Duration, +) { + client := newConn( + context.Background(), tcpConn, s.log, + s.database, s.brk, s.cfg, s.svc, + ) + client.pingInterval = interval + client.pongDeadline = deadline + + go client.serve(context.Background()) +} + // NewTestConn returns a Conn for nick on tcpConn, without a // server, database or service behind it. func NewTestConn( diff --git a/internal/ircserver/server_test.go b/internal/ircserver/server_test.go index 6aba3a8..c7d28b0 100644 --- a/internal/ircserver/server_test.go +++ b/internal/ircserver/server_test.go @@ -9,6 +9,7 @@ import ( "net" "os" "runtime" + "slices" "strings" "testing" "time" @@ -174,6 +175,42 @@ func (env *testEnv) dial(t *testing.T) *testClient { } } +// The ping interval and PONG deadline for dialWithPingTimes. +// They are equal, so that three answered PINGs take longer +// than a client that answers none stays connected. +const ( + testPingInterval = 300 * time.Millisecond + testPongDeadline = 300 * time.Millisecond +) + +// dialWithPingTimes connects a client to the test server +// whose connection is sent a PING after testPingInterval with +// nothing read, and closed when nothing is read within +// testPongDeadline after that. +func (env *testEnv) dialWithPingTimes(t *testing.T) *testClient { + t.Helper() + + serverSide, clientSide := net.Pipe() + + t.Cleanup(func() { _ = clientSide.Close() }) + + env.srv.ServeConn( + serverSide, testPingInterval, testPongDeadline, + ) + + return &testClient{ + t: t, + conn: clientSide, + scanner: bufio.NewScanner(clientSide), + } +} + +// isServerPing reports whether line is the PING the test +// server sends a quiet client. +func isServerPing(line string) bool { + return line == "PING :"+testServerName +} + // testClient wraps a raw TCP connection with helpers. type testClient struct { t *testing.T @@ -388,6 +425,57 @@ func TestPingPong(t *testing.T) { assertContains(t, lines, "PONG", "PONG response") } +// TestQuietClientAnsweringPingStaysConnected checks that a +// registered client that sends nothing is sent a PING each +// time the ping interval passes, and stays connected as long +// as it answers each one. +func TestQuietClientAnsweringPingStaysConnected(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + client := env.dialWithPingTimes(t) + client.register("answers") + + for range 3 { + client.readUntil(isServerPing) + client.send("PONG :" + testServerName) + } + + client.sendAndExpect("PING :still here", "PONG") +} + +// TestQuietClientNotAnsweringPingIsClosed checks that a +// registered client that does not answer the PING is +// disconnected. +func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + client := env.dialWithPingTimes(t) + client.register("silent") + + lines := client.readUntilClosed() + if !slices.ContainsFunc(lines, isServerPing) { + t.Errorf("closed without a PING, got lines %q", lines) + } +} + +// TestUnregisteredClientAnswersPing checks that a PONG before +// registration, the answer to the PING unregistered clients +// are sent too, gets no ERR_NOTREGISTERED. +func TestUnregisteredClientAnswersPing(t *testing.T) { + t.Parallel() + + env := newTestEnv(t) + client := env.dialWithPingTimes(t) + + client.readUntil(isServerPing) + client.send("PONG :" + testServerName) + + lines := client.sendAndExpect("PING :after", "PONG") + assertNotContains(t, lines, " 451 ", "ERR_NOTREGISTERED") +} + // TestIdleCleanupKeepsActiveUser checks that a user who // sends commands is kept by the idle cleanup, and a user who // sends nothing is not.