From aa56b8b63c7152e66e317f449b7107e571f8da6a Mon Sep 17 00:00:00 2001 From: clawbot Date: Tue, 20 Oct 2026 16:12:36 +0000 Subject: [PATCH] PING a quiet IRC listener client before closing it (closes #155) serve closed a connection after five minutes with nothing read and never sent a PING, so a quiet registered client lost its session. It now sends PING : after pingInterval with nothing read, and closes the connection when nothing is read within pongDeadline after that. Every line read starts both waits again. readTimeout is removed. Unregistered connections are sent the PING too, so a PONG before registration is now ignored instead of answered with ERR_NOTREGISTERED. Model: opus-5-5 --- README.md | 3 ++ internal/ircserver/conn.go | 28 ++++++++-- internal/ircserver/export_test.go | 17 ++++++ internal/ircserver/server_test.go | 88 +++++++++++++++++++++++++++++++ 4 files changed, 133 insertions(+), 3 deletions(-) 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.